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

Ignore aliases when using ls command #4743

Merged
merged 3 commits into from
Jul 25, 2019
Merged

Conversation

skunkworker
Copy link
Contributor

This is an addendum on #4282 for ZSH and possibly other shells.
The current version works fine on bash when alias ls='something-not-ls' but on zsh alias ls='something-not-ls' is called it currently fails.

It also fixes another ls command invocation in is_gem_installed().

Where /bin/ls is installed, this is used instead of the terminals ls short command. Otherwise it defaults to the shell sessions ls alias if it doesn't find it.

This can be helpful to make sure the default command doesn't have extra arguments.

This was my first attempt and I was able to get rid of the bug, but I think it might be worth it to move the _ls_command command out of the function.

…herwise use the alias on the system.

This prevents shells like ZSH having slightly different alias behavior from interferring with ls when another program is aliased to ls.
Copy link
Member

@pkuczynski pkuczynski left a comment

Choose a reason for hiding this comment

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

We write wrappers over system commands in scripts/functions/support. In this the best case would be to define it in __rvm_setup_utils_functions_common():

__rvm_ls() { \command \ls "$@" || return $?; }

This definition should ignore aliases. Then you should be able to use _rvm_ls in functions/gemset. Can you give it a try please?

@skunkworker
Copy link
Contributor Author

We write wrappers over system commands in scripts/functions/support. In this the best case would be to define it in __rvm_setup_utils_functions_common():

__rvm_ls() { \command \ls "$@" || return $?; }

This definition should ignore aliases. Then you should be able to use _rvm_ls in functions/gemset. Can you give it a try please?

Thanks @pkuczynski this seemed to work on my ZSH environment just fine with the aliasing.

@pkuczynski
Copy link
Member

Looks better! @mpapis what you think?

We are only missing CHANGELOG entry...

@pkuczynski pkuczynski changed the title Fix for zsh alias of ls command Ignore aliases when using ls command Jul 24, 2019
@pkuczynski pkuczynski added this to the rvm-1.29.10 milestone Jul 24, 2019
mpapis
mpapis previously approved these changes Jul 25, 2019
@pkuczynski pkuczynski merged commit f865bd5 into rvm:master Jul 25, 2019
@skunkworker skunkworker deleted the zsh_ls_fix branch July 26, 2019 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants