Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add $HOME/.config/nodejs with an option for deduping default repl history #11907

Closed
Fishrock123 opened this issue Mar 17, 2017 · 13 comments
Closed
Labels
feature request Issues that request new features to be added to Node.js. repl Issues and PRs related to the REPL subsystem.

Comments

@Fishrock123
Copy link
Contributor

Fishrock123 commented Mar 17, 2017

I just landed 5bda5fa which allows readlines to dedupe their history automatically when an option is set. I originally thought this would also be nice to have in the repl, but @DannyNemer made some good points in #2982 (comment) about why we shouldn't have that on by default.

I personally think there could be room for an env variable for this, and I'd personally kinda like to have it.

@Fishrock123 Fishrock123 added the repl Issues and PRs related to the REPL subsystem. label Mar 17, 2017
@Fishrock123 Fishrock123 changed the title Add ENV var for deduping repl history by default Add ENV var for deduping default repl history Mar 17, 2017
@jasnell
Copy link
Member

jasnell commented Mar 17, 2017

hmm... I'd say -0 on it. Wouldn't object but not overly favorable as I just don't see it as being that important.

@silverwind
Copy link
Contributor

I think we should really moderate ourselves with new environment variables. Maybe a ~/.noderc file would be better to control REPL options.

@bnoordhuis
Copy link
Member

I agree with @silverwind that we should be conservative with adding new environment variables.

The comment you link to mentions multi-line input, like a function definition. Deduping individual lines in multi-line input seems broken to me, I would expect it to either filter everything or nothing.

@Fishrock123
Copy link
Contributor Author

If we feel ~/.noderc would be reasonable that sounds fine to me.

@Fishrock123 Fishrock123 changed the title Add ENV var for deduping default repl history Add ~/.noderc with an option for deduping default repl history Mar 18, 2017
@Fishrock123 Fishrock123 added the feature request Issues that request new features to be added to Node.js. label Mar 18, 2017
@ChALkeR
Copy link
Member

ChALkeR commented Mar 18, 2017

~/.noderc? Please don't add yet another file directly in the users home dir, that was outdated more than a decade ago (in 2003, I guess).

The correct place is $XDG_CONFIG_HOME/noderc now, defaulting to $HOME/.config/noderc if $XDG_CONFIG_HOME is not set, and the configuration should also be searched in $XDG_CONFIG_DIRS directories. (Reference).

Note: we might prefer to put things into a directory (e.g. $XDG_CONFIG_HOME/Node.js/settings instead of $XDG_CONFIG_HOME/noderc) so it would be possible to add more files in the future.

That said, I am not convinced if we should store the configuration at all, so that's -1 on ~/.noderc, and -0 on storing it in the adequate place (though I might reconsider the latter, I'm just not convinced yet).

@Fishrock123 Fishrock123 changed the title Add ~/.noderc with an option for deduping default repl history Add $HOME/.config/nodejs with an option for deduping default repl history Mar 19, 2017
@Fishrock123
Copy link
Contributor Author

I honestly don't really care where it is, but it would be nice to store repl options somewhere outside cli flags.

@Fishrock123
Copy link
Contributor Author

Maybe this can be specifically for repl options then. Would people prefer that?

@ChALkeR
Copy link
Member

ChALkeR commented Mar 19, 2017

@Fishrock123 I saw a Node.js config file being mentioned in nodejs/node-eps#39.

/cc @bmeck and @michael-ciniawsky, perhaps.

@jasnell
Copy link
Member

jasnell commented Mar 19, 2017

I cannot say that I'm a big fan of having a .noderc type config file. Yes, there are a number a ways in which it could be used but the added benefit is minimal in my opinion and there's plenty of room for issues (such as conflicting config requirements for multiple node.js apps). Not going to -1 this right now, tho, just not a fan.

@michael-ciniawsky
Copy link

michael-ciniawsky commented Mar 20, 2017

.noderc

{
   "module": true 
   "harmony": {...V8Options}
   "debug": {...options}
}
node --module index => node index

.noderc && NODE_ENV

{
   "production": { // === process.env.NODE_ENV
      "module": false
      "harmony": false
      "debug/inspect": false
   },
   "development": { // === process.env.NODE_ENV
      "module": true
      "harmony": {...options}
      "debug/inspect": {...options}
   },
    "test": { // === process.env.NODE_ENV
      "module": true,
      "harmony": true,
      "debug/inspect": true
   }
}
node --debug/inspect --module --harmony '...' index => NODE_ENV=test node index

Why NODE_ENV ?

.noderc

{
   "production": {
       "no_deprecations": true
    } 
}

Deprecation Warnings => Error { default: false }

Additional

enforce_package

.noderc

{
   "production": {
       "enforce_package": true
    } 
}
git clone https://github.com/user/myAPI && NODE_ENV=production node
pkg_enforced && (pkg && !node_modules)
   ? cp.spawn('npm', [ 'i', process.env.NODE_ENV === 'production' ? '--production' : '' ]) 
   : throw new Error(`
       No package.json found in ${process.cwd()}
       Running in ${process.env.NODE_ENV} mode failed!
     `)

@bmeck
Copy link
Member

bmeck commented Mar 20, 2017

Please note that any talk of "module" in a global config won't affect any default for packages since all packages are currently written for CJS, it would only act as if the CLI flag for the entry point was set.

My talks in other issues about similar global files have often resulted in problems with keeping default behavior set in expected ways. I fear that having this config functionality will remove barriers to using features that are not intended to be used in non-testing scenarios (--harmony*), problems with configuration (would need a CLI flag to configure where the .noderc is), or applications using it for configuration (like npm_config_* ENV flags). I think the idea is interesting for an out of band configuration but am not sold on it without very constrained capabilities.

@michael-ciniawsky
Copy link

michael-ciniawsky commented Mar 20, 2017

Yep agreed with @bmeck, also module is basically obsolete if .mjs will be the way to go (likely). I just tried 'summed' it up for discussion proposes and this needs to be given way more thought in terms of concrete design and skeptical investigation if really needed.

@Trott
Copy link
Member

Trott commented Jul 30, 2017

Closing as this seems inactive and with no clear consensus on how to approach it. However, if I'm wrong about that, or even if we just want it open as a placeholder/reminder, feel free to re-open. Just tidying up, no super-strong opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

8 participants