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

[Fix] bash_completion: be robust when cd is overridden #2585

Merged
merged 1 commit into from
Sep 13, 2021
Merged

[Fix] bash_completion: be robust when cd is overridden #2585

merged 1 commit into from
Sep 13, 2021

Conversation

giladbarnea
Copy link

Bug:

If cd is patched (overridden) and prints something to stdout/stderr, completing nvm use with tab-tab erroneously displays this output alongside the completion options.

Steps to reproduce:

# Patch cd to print arbitrary text:
function cd(){ echo EXTRA INFORMATION IN MY FACE; command cd "$@"; }
nvm use <TAB> <TAB>

Completion words should be e.g
default EXTRA FACE IN INFORMATION iojs MY node stable unstable v14.17.6

Fix:

This is fixed if function __nvm_aliases in bash_completion uses command cd instead of bare cd.

@ljharb ljharb added the shell alias clobbering Anything dealing with users shadowing builtins with aliases or functions. label Sep 13, 2021
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, seems great to me.

@ljharb ljharb changed the title Bugfix: bash completion for nvm aliases is broken when cd is overridden [Fix] bash_completion: be robust when cd is overridden Sep 13, 2021
@ljharb ljharb merged commit d91087c into nvm-sh:master Sep 13, 2021
@giladbarnea
Copy link
Author

Thanks, seems great to me.

Should I delete the branch now?

@ljharb
Copy link
Member

ljharb commented Sep 14, 2021

That’s up to you, it’s your fork - but yes, typically the branches are deleted after the PR is merged.

@giladbarnea giladbarnea deleted the fix_bash_completion_nvm_aliases branch September 14, 2021 06:56
@giladbarnea
Copy link
Author

That’s up to you, it’s your fork - but yes, typically the branches are deleted after the PR is merged.

Alright, deleted. Thanks for nvm-sh!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shell alias clobbering Anything dealing with users shadowing builtins with aliases or functions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants