jasongin / nvs

Node Version Switcher - A cross-platform tool for switching between versions and forks of Node.js
Other
2.74k stars 210 forks source link

Add support for comments in version files like .nvmrc #175

Open atl-mk opened 4 years ago

atl-mk commented 4 years ago

I recently came across a project with a .nvmrc file with a comment in it which is useful to tell maintainers that unfortunately it's not the single source of truth (due to it being duplicated in a pom.xml that uses frontend-maven-plugin). Could support be added to ignore anything after a version number that comes at the start of the file?

cat .nvmrc 12.18.0 # note to update the pom.xml node version too

jasongin commented 4 years ago

Does nvm support comments in .nvmrc?

There is a long discussion at https://github.com/nodejs/version-management/issues/13 about agreeing on a standard format for a file that specifies the node version for a project, however there was no consensus reached. Without agreement there, I would hesitate to extend support for more options in that file.

ljharb commented 4 years ago

Absolutely nvm does not support comments; the only thing it can contain is an nvm-style "version-ish", with perhaps a trailing newline.

atl-mk commented 4 years ago

I've noticed that neither fnm nor nvm have issues processing this particular file. Perhaps that's just due to the implementation details. I understand why you're hesitant to officially support this behaviour.

What about settling for an unofficial support via less strict parsing?

jasongin commented 4 years ago

Yes, it would be make sense to ignore any following lines after the first. (No need for a setting.)

ljharb commented 4 years ago

fwiw I'm far more likely to change nvm to error on extra lines being present, in order to preserve the design space - iow, to make the parsing stricter.

atl-mk commented 4 years ago

@ljharb How about warning for now and then do a breaking change to error (probably at the same time as the rest of the file standard is defined) since the contract isn't currently defined?

I'm imagining this would mean that current users won't have a broken experience as soon as they upgrade nvm, but they can fix the warning by removing whatever other non-standard lines. IMO this gives consumers some advanced notice and thus a better experience, but I'm keen to hear your thoughts.

I do appreciate this adds a little bit of complexity in the mean time.

@jasongin As for how nvs should handle this; aside from eventually reaching consistency in the contract with the version files across all node version manager/switcher tools being important (IMO), I obviously have vested interest here, but unless the nvm's behaviour will actually change soon I don't see any reason to be relatively more strict.

mtariqsajid commented 11 months ago

please add support for comments i don't get it why it was not done in the early stage. its like very basic requirement of any config file

ljharb commented 11 months ago

The core config file of the node ecosystem has never had comments, so i don’t think it’s as basic as you’re claiming.

atl-mk commented 11 months ago

The core config file of the node ecosystem has never had comments, so i don’t think it’s as basic as you’re claiming.

It's often called out as a missing feature. The trend within the ecosystem seems to be moving towards more expressive configuration with .json -> .js

mtariqsajid commented 11 months ago

@ljharb @atl-mk but every config file has comments also known as dotfile

because i need to tell other developer that 18 means is nodejs version 18 its very confusing to just put 18 in .nvmrc file

ljharb commented 11 months ago

i hear you. I’m just saying that package.json doesn’t have comments and it’s fine.

i don’t personally find it confusing - what else would 18 and “nvm” mean - but that’s subjective.

mtariqsajid commented 11 months ago

i hear you. I’m just saying that package.json doesn’t have comments and it’s fine.

i don’t personally find it confusing - what else would 18 and “nvm” mean - but that’s subjective.

junior developer

korpiq-tiketti commented 9 months ago

I would like to add a reason for sticking to a specific node version. In our case, we need the experimental-specifier-resolution option that was removed in node 19, so we're stuck with node 18. I'd love to be able to state this reason where the setting is instead of the README. Can do without; would like to have the comment at the setting it explains.

DavidBruchmann commented 7 months ago

There are also options to define the version more general than just with a distinct version number.
Explaining the options or the meaning of the used option might be useful.
Some completely unrelated config files come with lengthy explanations as comments, so I don't see a reason to exclude this option from the .nvmrc file format.