[Fix] Ignore commented profile source lines - #3860
Conversation
Signed-off-by: Vaibhav Cheruvu <vaibhavsharma.c@gmail.com>
|
Hi @ljharb , can you please look at this pr, when you have time, it is a small and focused fix. |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
ljharb
left a comment
There was a problem hiding this comment.
Thanks! This will need regression tests, though.
| BASH_OR_ZSH=true | ||
| fi | ||
| if ! command grep -qc '/nvm.sh' "$NVM_PROFILE"; then | ||
| if ! command grep -qc '^[[:space:]]*[^#].*/nvm\.sh' "$NVM_PROFILE"; then |
There was a problem hiding this comment.
This still matches indented comments - [[:space:]]* backtracks, so [^#] can itself match a space: a line like # [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" (or a tab-indented one) is still treated as active, and the #1889 repro persists for it.
Excluding whitespace from the guard class fixes it without breaking indented active lines:
'^[[:space:]]*[^#[:space:]].*/nvm\.sh'(the $NVM_DIR/bash_completion pattern below has the same issue)
The regression tests should include an indented-comment case, since that is the case that distinguishes this pattern from the correct one.
There was a problem hiding this comment.
Thanks @ljharb will work on the regex fix and will add regression tests.
Fixes: #1889
This updates the install script so commented-out nvm profile lines are not treated as active setup lines.
Before this change,
install.shchecked for/nvm.shand$NVM_DIR/bash_completionanywhere in the profile file. This also matched commented-out lines as well.As a result, reinstalling nvm skipped appending active setup lines. After opening a new bash shell with that profile,
nvmwas still unavailable:$ nvm --version bash: nvm: command not foundAfter this change, the installer ignores commented-out setup lines and appends active nvm and bash_completion lines:
Hence, nvm is available as expected: