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

command <someCommand> missing in several places throughout the shell code #3458

Closed
5 of 10 tasks
calestyo opened this issue Sep 28, 2023 · 3 comments
Closed
5 of 10 tasks

Comments

@calestyo
Copy link
Contributor

  • I have read through the manual page (man fzf)
  • I have the latest version of fzf
  • I have searched through the existing issues

Info

  • OS
    • Linux
    • Mac OS X
    • Windows
    • Etc.
  • Shell
    • bash
    • zsh
    • fish

Problem / Steps to reproduce

Hey.

AFAIU, you use command <someCommand> in the sourced shell files to prevent undesired alias subsitution, which would likely break everything - just as e.g. bash-completion does as well and which of course is a good thing.

Looking at the shell code (completion.bash and key-bindings.bash at least), there are however numerous places where this isn't done, e.g.:

echo "$1"

while read -r l; do

eval "base=$base"

fzf/shell/completion.bash

Lines 284 to 287 in a3ff49a

command cat <(command tail -n +1 ~/.ssh/config ~/.ssh/config.d/* /etc/ssh/ssh_config 2> /dev/null | command grep -i '^\s*host\(name\)\? ' | awk '{for (i = 2; i <= NF; i++) print $1 " " $i}' | command grep -v '[*?%]') \
<(command grep -oE '^[[a-z0-9.,:-]+' ~/.ssh/known_hosts | tr ',' '\n' | tr -d '[' | awk '{ print $1 " " $1 }') \
<(command grep -v '^\s*\(#\|$\)' /etc/hosts | command grep -Fv '0.0.0.0') |
awk -v "user=$1" '{if (length($2) > 0) {print user $2}}' | sort -u

I could try to write a patch that adds command, but:

  • only after my other PRs have been either merged or rejected (otherwise it gets to messy)
  • we need to decide on what we protect via command

I think for cases like awk or tr it makes sense to use command. I also have seen cases, where people alias echo/printf. Not sure about things like read, break or eval. I'd tend to perhaps use command for read but not for any of the special built-ins.

Also, function names like in:

matches=$(eval "$1 $(printf %q "$dir")" | FZF_DEFAULT_OPTS="--height ${FZF_TMUX_HEIGHT:-40%} --reverse --scheme=path --bind=ctrl-z:ignore ${FZF_DEFAULT_OPTS-} ${FZF_COMPLETION_OPTS-} $2" __fzf_comprun "$4" -q "$leftover" | while read -r item; do

may be aliased, too.

We could in principle avoid alias substitution in such cases by quoting the function name, not sure whether this should be done though.

Cheers,
Chris.

@junegunn
Copy link
Owner

  • we need to decide on what we protect via command

That's the hard part. An easy solution is to put command only when someone actually complains and raises an issue. That's how we've been handling the issue so far.

I also have seen cases, where people alias echo/printf.

Wow.

@calestyo
Copy link
Contributor Author

That's the hard part.

What if we use command on any program/utility that is not a special built-in command? And, to increase acceptance ;-), also exclude echo/printf for now.

@calestyo
Copy link
Contributor Author

Well sadly it probably makes sense now to close all my open issues as wontfix, but this one was actually fixed, thus closing.

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