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

Allow comments in .nvmrc config file #3336

Open
JohnAlbin opened this issue Apr 18, 2024 · 5 comments
Open

Allow comments in .nvmrc config file #3336

JohnAlbin opened this issue Apr 18, 2024 · 5 comments

Comments

@JohnAlbin
Copy link

PR #2288 has a discussion about comments in .nvmrc and I'm moving this to a proper issue so it doesn't get buried in the opinions about the code in that PR.

Why support comments?

Projects should, when necessary, have comments in their configuration files like .nvmrc.

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.

Current .nvmrc specification

I was looking at the code in NVM and noticed this line in the 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 could potentially 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.

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

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

Plan for allowing comments in .nvmrc

In this comment, the nvm maintainer said:

eventually nvm will support parsing additional content from successive lines.

If we want to officially support comments, no code changes will be needed in this version of NVM. But we'll need to be mindful of how complex the code will need to be to support code comments in future versions of NVM.

What we need to discuss:

  1. At a minimum, we need to support line comments. We need to decide if we optionally want to support inline comments and/or block comments.

    # a line comment means the entire line is ignored
    some-config-syntax # an inline comment means just the bit after the comment marker is ignored
    <!-- a block comment can span multiple lines and everything
         between the start and end comment marker is ignored -->
    
  2. Decide which comment format(s) to use. Examples:

    # hash-prefixed comments (like Perl, Python and shell comments)
    // JavaScript inline comments
    ; ini-style comments
    -- ADA/AppleScript comments
    /* JavaScript block comments */
    <!-- HTML block comments -->
    

    Wikipedia has many other examples of code comment syntax.

  3. Ensure the chosen comment format doesn't interfere with NVM's:

    • Node.js versions
    • built-in aliases for Node.js versions
    • custom aliases for Node.js versions
    • the syntax for planned, but not-yet-implemented features in the future (I'm unsure what those requirements will be).

    For example: /* js-style */ block comments are INCOMPATIBLE with NVM's built-in alias, lts/*, so we can't use that comment syntax.

  4. Document how comments work by adding it to the .nvmrc section of README.md

Both 1 and 2 will affect how complicated the code will need to be when we need to implement comment parsing. It's clear the NVM maintainer wants very low complexity and maintenance costs. totally understandable.

@JohnAlbin
Copy link
Author

FYI, I prefer line and inline #-prefixed comment syntax because:

  • it matches most of the other dot file comment syntaxes
  • it should be simple to add parsing for it
  • it doesn't appear to conflict with Node.js versions and alias syntax. Although, we would need to explicitly state that # is not allowed in custom Node.js aliases.

If it turns out that # conflicts with the syntax of planned features (of which I am not aware of), then I'd favor only allowing line comments where the first character in the line is a #. In other words, a line MUST start with a # to be a comment and the entire line is ignored when determining the syntax of the rest of the file.

@JohnAlbin
Copy link
Author

JohnAlbin commented Apr 18, 2024

As a work-around, devs can add comments anywhere after the first line of .nvmrc. It's possible those comments will break NVM in future versions and you'll have to remove them. Fortunately, my .nvmrc comments explain why the comment may break NVM. 😁

This is what one of my projects' .nvmrc file looks like:

v18.19

# Node.js v18.20 and v20.10 (and later) have a bug that causes Jest code
# coverage to randomly fail.
# TODO: Switch to "v18" or "v20" when this bug is fixed.
# https://github.com/nodejs/node/issues/51251

# Note: This is a non-standard .nvmrc comment. If these comments cause an error,
# see https://github.com/nvm-sh/nvm/issues/3336

@ljharb
Copy link
Member

ljharb commented Apr 18, 2024

Additional lines will be used in the future for additional configuration, so if you put comments there things will break for you - don’t do that.

@JohnAlbin
Copy link
Author

Additional lines will be used in the future for additional configuration, so if you put comments there things will break for you - don’t do that.

That's why I called it a work-around while comments are still not officially supported.

@ljharb
Copy link
Member

ljharb commented Apr 18, 2024

Regarding the OP, it's an rc file, so the only two options that would make sense are ; or # - ie, ini or shell.

I think "shell" makes the most sense for nvm, so i'd say that "lines starting with #", at a minimum, would be skipped, and thus would support comments. Whether it's necessary to add in "inline" comments is worth discussing, but I'm not sure it's particularly useful, and it adds extra complexity in case that character is ever actually needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants