Added in comments in .nvmrc

[Yash-Singh1]

Overview

I added in support for comments in the .nvmrc file.

List of Updates

• Modified the nvm_rc_version to strip comments
• Removed support for spaces and #'s inside nvm aliases

[ljharb]

I think before deciding what code changes are needed, to this PR or nvm itself, we should have nonzero discussion about what the goal is, and why this feature is needed.

[ljharb]

Might as well talk about it here :-)

[sladyn98]

@ljharb Do we want to take action on this one with the intention of getting it merged ?

[ljharb]

@sladyn98 I'm still not convinced it's worth the complexity.

[sladyn98]

@ljharb if you plan to not merge in the future don't you think we should close it

[ljharb]

I'm undecided, which is why it's left open. I don't believe in closing a PR unless it's certain to never land.

[penx]

Would be very useful for "make sure you update the <docker container/CI config/package.json engines> too!" style comments

[Yash-Singh1]

Do you mean allowing a .nvmrc, like this?:

“some comment”
node

The reason that this shouldn’t be added is because it removes support for aliases with double quotes and anyways, the following works:

# some comment
node

[penx]

No, I mean the feature (as you propose it) would be useful for adding comments to the .nvmrc to remind people to change values elsewhere.

e.g.

# this should be kept in sync with the <docker container/CI config/package.json engines/readme/wiki>
14

This was partially in response to

why this feature is needed

further up.

[ljharb]

@penx isn't a CI job that fails when those aren't in sync much more effective than a comment?

[penx]

@ljharb yes it would, but it may not always be feasible or convenient

e.g.

• the place to keep in sync is not within the repository, such as a wiki or external repository
• the format to keep in sync is hard to parse e.g. numbers contained in paragraphs in a markdown readme where there may be multiple mentions and the wording could change ("prerequisites: node >= 12 and <= 14.*, npm 7")
• the contents of .nvmrc doesn't exactly match what you're trying to sync with (e.g. semver, ranges)
• the management of the CI pipelines are done by a different team/company and it is not trivial to add new ones or modify existing ones

[jasonkarns]

Another useful example of a useful comment (and one that isn't particularly suited for git commit messages, IMO):

14.0.0

# symlinked as .nvmrc/.node-version to support nvm, nodenv and asdf with a single file
# until such time as https://github.com/nodejs/version-management/issues/13 is resolved

[ljharb]

@jasonkarns i mean, the symlink part is already self-evident from "the actual symlinks in the filesystem"; it's just the issue URL that has no place to go. but fair, that's a good use case.

[jasonkarns]

@ljharb if we assume that it being a symlink is even evident at all.

Some ides/editors don't distinguish that a file is a symlink; and even those that do, don't necessarily make it -obvious-.

Add insult to injury, these are hidden files so many project views wouldn't show them in a listing.

All that to say: I -had- this exact comment in a symlink in a project recently and had to remove it for compatibility. :)

[JohnAlbin]

First, about why you want comments in the .nvmrc file:

git log is horrible for file type discovery. git log .nvmrc is going to show "Initial commit" for 100% of my projects.

If you are explaining what an .nvmrc file is for, wouldn't that be better put in a docs/DEVELOPMENT.md file? Your project readme files should explain what tools and commands are needed for development and how they are used. Having new developers poke around in all the files and to try to figure out what is important and why is not a great onboarding strategy. However, having an intern make notes about project files (even if the notes are "???") and then tasking them with expanding a project's developer docs is a GREAT intern task, @danielrob.

Several people have pointed this out above, so I'd like to reiterate the main reason I have comments in configuration files is to document bugs that force me to use a specific configuration option, .e.g. "Bug X causes Y so we set our config to Z as a work-around. [link to issue]" That kind of information needs to be right next to the configuration or someone (maybe even me!) will "fix it" later not realizing it causes a bug, e.g. "Huh, this file is out-of-date. The value of v14.13.1 should be updated to the latest v14.19.1. Oops. I broke something."

IMO, I think all configuration files should be able to have comments in them. Don't get me started on .json files for configuration.

Lastly, I was looking at the code this PR changes and noticed this line in the original nvm.sh file:

NVM_RC_VERSION="$(command head -n 1 "${NVMRC_PATH}" | command tr -d '\r')" || command printf ''

Only the first line in .nvmrc is ever used to determine the value of NVM_RC_VERSION. That means all the other lines in .nvmrc can be used for comments without any code changes. I tested this by creating a .nvmrc file with an intentionally blank first line:

v14

And I got an error: Warning: empty .nvmrc file found

And when I changed the file to:

v14
# This is a comment
// And so is this
; And so is this
And so is this

The nvm use command reported Found '.nvmrc' with version <14> and ignored all other lines except the first.

So, all we really need to do is make this officially supported in docs; no code changes, afaics.

[ljharb]

@JohnAlbin however, if you do that, it's guaranteed to break in the future, because eventually nvm will support parsing additional content from successive lines.

[JohnAlbin]

@ljharb said:

if you do that, it's guaranteed to break in the future, because eventually nvm will support parsing additional content from successive lines.

Agreed.

From my previous comment:

…all we really need to do is make this officially supported in docs… (emphasis added)

Which is why I said we need to decide how to document how comments should work.

Right now, afaics, the entire .nvmrc syntax specification is:

• The first line must contain a Node.js version number; see https://github.com/nvm-sh/nvm#nvmrc
• Other lines are currently ignored but are reserved for future usage.

To add the official ability to comment the file we need to decide what format comments are allowed to use. And whether we want to reserve the second or third lines for forward compatibility with future features.

FYI, I prefer the # comment syntax as it matches most of the other dot file comment syntaxes.

[r-moore]

Several people have pointed this out above, so I'd like to reiterate the main reason I have comments in configuration files is to document bugs that force me to use a specific configuration option, .e.g. "Bug X causes Y so we set our config to Z as a work-around. [link to issue]" That kind of information needs to be right next to the configuration or someone (maybe even me!) will "fix it" later not realizing it causes a bug, e.g. "Huh, this file is out-of-date. The value of v14.13.1 should be updated to the latest v14.19.1. Oops. I broke something."

This is our exact use case

We pin a project to a specific node version and I would like to leave a comment in nvmrc to say why we are using that version (e.g. "don't upgrade to v16 yet because X breaks")

Yes we could put that comment somewhere else in documentation, but this seems like the best place to leave a reminder (so that the person bumping the version will see it)

[JohnAlbin]

I created a new issue so we can discuss adding comments to .nvmrc without mixing that discussion up with our opinions about the code in this PR.

#3336 Allow comments in .nvmrc config file