pacak / bpaf

Command line parser with applicative interface
337 stars 17 forks source link

Completions broken in zsh if completing with a path containing `~` or an environment variable #315

Closed VorpalBlade closed 5 months ago

VorpalBlade commented 10 months ago

This is a weird one:

$ ~/src/chmm/target/debug/chezmoi_modify_manager -s a<TAB> _chezmoi_modify_manager:2: no such file or directory: ~/src/chmm/target/debug/chezmoi_modify_manager

$ ls ~/src/chmm/target/debug/chezmoi_modify_manager
/home/arvid/src/chmm/target/debug/chezmoi_modify_manager

$ ~/src/chmm/target/debug/chezmoi_modify_manager --help
[binary works, and outputs help...]

$ chezmoi_modify_manager -s a<TAB>
[Lists files starting with a]

$ /usr/bin/chezmoi_modify_manager -s a<TAB>
[Lists files starting with a]

The binary in /usr/bin is the one in path. And it is the same version as from my source dir (except built with Arch Linux AUR as a system package). Huh?

_chezmoi_modify_manager is in /usr/share/zsh/site-functions/_chezmoi_modify_manager (installed by Arch Linux package manager as part of AUR package).

Relevant system info:

VorpalBlade commented 10 months ago

Oh! I think I may have spotted it:

$ ~/../../usr/bin/chezmoi_modify_manager  -s a<tab>  _chezmoi_modify_manager:2: no such file or directory: ~/../../usr/bin/chezmoi_modify_manager
$ $HOME/src/chmm/target/debug/chezmoi_modify_manager -s a _chezmoi_modify_manager:2: no such file or directory: $HOME/src/chmm/target/debug/chezmoi_modify_manager

Probably it doesn't like the ~. Or $HOME. But relative and absolute paths work.

pacak commented 10 months ago

Some zsh shenanigans I suspect. You can see what exactly zsh is trying to execute if you press Ctrl-X followed by ? instead of TAB while trying to complete something. Then Up and Enter to get the exact code. Things like ~ and $HOME are usually handled by shell. Can you check if there's anything unusual is going on there?

VorpalBlade commented 10 months ago

First time I have read that output. It is rather long at 195 very very long lines.

But from the the looks of it, the issue that somehow we end up with single quotes around things:

   +_dispatch:46> str=~/src/chmm/target/debug/chezmoi_modify_manager
   +_dispatch:47> [[ -n '~/src/chmm/target/debug/chezmoi_modify_manager' ]]
   +_dispatch:51> str='~/src/chmm/target/debug/chezmoi_modify_manager' 
   +_dispatch:52> name='~/src/chmm/target/debug/chezmoi_modify_manager' 
   +_dispatch:53> comp='' 
   +_dispatch:54> service='~/src/chmm/target/debug/chezmoi_modify_manager' 
   +_dispatch:56> [[ -z '' ]]
   +_dispatch:46> str=chezmoi_modify_manager
   +_dispatch:47> [[ -n chezmoi_modify_manager ]]
   +_dispatch:51> str=chezmoi_modify_manager 
   +_dispatch:52> name=chezmoi_modify_manager 
   +_dispatch:53> comp=_chezmoi_modify_manager 
   +_dispatch:54> service=chezmoi_modify_manager 
   +_dispatch:56> [[ -z _chezmoi_modify_manager ]]
   +_dispatch:56> break
   +_dispatch:61> [[ -n _chezmoi_modify_manager && chezmoi_modify_manager != -default- ]]
   +_dispatch:62> _compskip=patterns 
   +_dispatch:63> eval _chezmoi_modify_manager
    +(eval):1> _chezmoi_modify_manager
     +_chezmoi_modify_manager:2> source /proc/self/fd/21
     +_chezmoi_modify_manager:2> '~/src/chmm/target/debug/chezmoi_modify_manager' '--bpaf-complete-rev=7' -s a
_chezmoi_modify_manager:2: no such file or directory: ~/src/chmm/target/debug/chezmoi_modify_manager
   +_dispatch:63> ret=0 

Which is odd...

$ type -f _chezmoi_modify_manager                                                                                                                                                      27m 54s
_chezmoi_modify_manager () {
    source <( "${words[1]}" --bpaf-complete-rev=7 "${words[@]:1}" )
}

It could be something with the zsh framework/addons etc I use (if you can't reproduce this). Unfortunately the one I use (zsh4humans) has effectively stopped being maintained in recent years. As it suits my needs perfectly and has very good latency I have yet to try to switch to something else.

Worth noting is that commands other than things using bpaf seem unaffected. Even those that call the binary to generate completions dynamically (such as chezmoi, written in Go) work when called via $HOME/../../bin/.... It's script is way more complicated though (at 155 lines)

VorpalBlade commented 10 months ago

For what it is worth, the completions for chezmoi does seem to call chezmoi differently. This seems like the relevant part:

    # Prepare the command to obtain completions
    requestComp="${words[1]} __complete ${words[2,-1]}"
    if [ "${lastChar}" = "" ]; then
        # If the last parameter is complete (there is a space following it)
        # We add an extra empty parameter so we can indicate this to the go completion code.
        __chezmoi_debug "Adding extra empty parameter"
        requestComp="${requestComp} \"\""
    fi

    __chezmoi_debug "About to call: eval ${requestComp}"

    # Use eval to handle any environment variables and such
    out=$(eval ${requestComp} 2>/dev/null)

That comment above the eval seem extremely relevant here.

I believe (from a previous bug report last year or possibly before) that chezmoi also uses a library similar to bpaf/clap etc that does the actual completions and arg parsing. I don't remember the name of it, as I don't know Go.

pacak commented 10 months ago

What you pasted is basically a fix :)

I made this change to a script from cargo-show-asm that at the moment uses older version and it works with both relative and absolute paths. You should be able to make a similar change in your script to have it working. I'll clean it up and release in a new version at some point in the future.

line="${words[1]} --bpaf-complete-rev=4 ${words[2,-1]}"
if [[ ${words[-1]} == "" ]]; then  
    line="${line} \"\""                                                    
fi                                                                                          
IFS=$'\n' lines=$(eval ${line})                       
VorpalBlade commented 10 months ago

I'm not keen on the use of eval though! There might be some expansion modifier that does what you want... Zsh has a ton of those.

Also check license of where I took that from (some random go library). I don't know how prevalent copyleft licences are in that ecosystem.

VorpalBlade commented 7 months ago

Just a question: did you ever incorporate the fix for this?

pacak commented 7 months ago

Just a question: did you ever incorporate the fix for this?

Got distracted by some other stuff. I can add this along with some other changes I'm working if that's a problem for you.

VorpalBlade commented 7 months ago

Sounds good, though issue #303 is also a problem for me (and that one is probably harder to fix)

pacak commented 7 months ago

Ack. Will try to give them more priority now that I have better tests (not merged yet) in place.