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

"help" intercepts commands run by exec and run #3126

Open
cseitz opened this issue Jun 6, 2023 · 9 comments
Open

"help" intercepts commands run by exec and run #3126

cseitz opened this issue Jun 6, 2023 · 9 comments
Labels
informational pull request wanted This is a great way to contribute! Help us out :-D

Comments

@cseitz
Copy link

cseitz commented Jun 6, 2023

Command

# Run node --help with the specified version
nvm exec lts/hydrogen node --help

What happened?

NVM outputs its help command.

What did you expect to happen?

I expected the --help flag to be passed to node

Potential Solution

I understand why the help command takes precedense since its useful for just plopping it at the end of a command to get help on said command. However, this really ruins any situations where one might be using nvm exec as a replacement to node, effectively making it a pain to run anything else's help command.

My primary usage is to use nvm behind-the-scenes as an environment manager, so any time someone specifies "help" in my use-case, it is intended for node, npm, or whatever the command they are running is and not NVM. The user would never be running nvm directly, but rather a custom CLI that passes args through NVM when necessary to run them at the correct version without updating the default NVM version that's in use elsewhere on the user's machine.

Thus, I propose NVM_NO_HELP=1.

...
  # nvm.sh, line 2820
  case $i in
    --) break ;;
    '-h'|'help'|'--help')
    NVM_NO_COLORS=""
+   if [[ "$NVM_NO_HELP" -eq 1 ]]; then
+     break;
+   fi
    for j in "$@"; do
...

This flag, if specified, will instruct NVM to not respond to help requests.

I'm not very skilled with bash so I imagine there's a better solution out there; but the code snippet above works on my local installation of nvm and lets me bypass the help command.

NVM_NO_HELP=1 nvm exec lts/hydrogen node --help
# correctly outputs node v18's help command!

Conclusion

Please let me know if you think this is something that is worth fixing in future versions of NVM. It would certainly help my use-case and allow me to avoid modifying nvm.sh just to escape help commands.

Thanks.

@ljharb
Copy link
Member

ljharb commented Jun 6, 2023

The solution here is to use -- to tell nvm to stop processing dashed commands, which is a universal shell convention.

@ljharb
Copy link
Member

ljharb commented Jun 6, 2023

(In the future you may want to hold off on a PR until you're sure the maintainer is interested)

@cseitz
Copy link
Author

cseitz commented Jun 6, 2023

Oh, thank you! I tried adding -- but apparently I did it in the wrong place thus lead me down this path.

nvm exec lts/hydrogen -- node --help

Good to know this is already implemented. Thanks!

Is this something we should add to the docs?

@ljharb
Copy link
Member

ljharb commented Jun 6, 2023

Sure, let's update your PR to change the docs instead :-) It should be able to appear anywhere.

@cseitz
Copy link
Author

cseitz commented Jun 6, 2023

# ❌ - shows NVM help
nvm -- exec lts/hydrogen node --help
# ❌ - error in nvm-exec line 15
nvm exec -- lts/hydrogen node --help
# ✅ - works perfectly
nvm exec lts/hydrogen -- node --help
# ❌ - nodejs: module not found "--help"
nvm exec lts/hydrogen node -- --help

My initial attempt was the 4th one, but this results in Node.js MODULE_NOT_FOUND: cannot find module '--help'

@ljharb
Copy link
Member

ljharb commented Jun 6, 2023

it certainly has to go after exec lts/hydrogen, but the last one should certainly work - I'd consider that a bug.

@shadowspawn
Copy link

For interest, an alternative approach is to have run and exec pass through following arguments (including options). I got used to this convenience with docker:

% docker run ubuntu ls --help
Usage: ls [OPTION]... [FILE]...
List information about the FILEs (the current directory by default).
Sort entries alphabetically if none of -cftuvSUX nor --sort is specified.
...

n follows this pattern:

% n exec hydrogen node --version
v18.16.1

@ljharb
Copy link
Member

ljharb commented Aug 5, 2023

@shadowspawn i worry that would cause problems if there's nvm-specific options passed to run and exec.

I certainly think that anything that's not used by nvm should be passed through, though.

@ljharb
Copy link
Member

ljharb commented Aug 5, 2023

Given that nvm exec lts/hydrogen -- node --help works, i think it actually does make sense to not remove a -- inside the executed command - iow, there's no guarantee it's node, and the -- could be meaningful. I'm not sure there's anything to do here except perhaps update the docs?

@ljharb ljharb added informational pull request wanted This is a great way to contribute! Help us out :-D labels Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
informational pull request wanted This is a great way to contribute! Help us out :-D
Projects
None yet
Development

No branches or pull requests

3 participants