dsifford / yarn-completion

Bash completion for Yarn
MIT License
277 stars 25 forks source link

Allow colon in e.g. package fields script #19

Closed joma74 closed 5 years ago

joma74 commented 6 years ago

I often do

"scripts": {
    "test:once": "...",
    "test:watch": "...",
  }

in my package script. Which are never completed properly.

joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn run 
build       code_frmt   dev         env         test:once   test:watch  
joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn run test:
config/       dist/         .git/         node_modules/ src/          types/        .vscode/

So i investigated that colon is a special, see http://tiswww.case.edu/php/chet/bash/FAQ on the E13 case. Looked through your script and saw COMP_WORDBREAKS="\"'><=;|&(: ". Which explicitly includes : as a completion word break character. Which might be kind of the opposite this issue is for.

However, if you accept a PR is see one viable solutions. If a am not missing why else colon should explicitly be included as a wordbreak.

Solution A: Add the wordbreak exclusion parameter to the shell completion builtin calls

COMPREPLY=()
        if command -v _init_completion >/dev/null; then
                _init_completion -n :
        else
                if command -v _get_comp_words_by_ref >/dev/null; then
                        _get_comp_words_by_ref -n : cur prev words cword
                fi
        fi
...
__ltrim_colon_completions "$cur"

giving

joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn run 
build       code_frmt   dev         env         test:once   test:watch  
joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn run test:
once   watch  
joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn run test:once

P.S. I am right now of the opinion that changing COMP_WORDBREAKS as local var in you script never worked.

joma74 commented 6 years ago

Hm, just saw that - having patched it - it does not complete in the not-run case

joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn te
team        test:once   test:watch  
joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn test:

Hm, i don't get it.

Ok, in this case this is happening. _yarn_test: should not be taken as next cmd. Ends in filedir completion which shoud not.

joma@edison:~/entwicklung/nodews/angular-ultimate-fundamentals$ yarn test:+ __yarn_get_command
+ cmd=yarn
+ [[ 1 -lt 2 ]]
+ case "${words[$counter]}" in
+ cmd=test:
+ break
+ declare completions_func=_yarn_test:
+ declare -F _yarn_test:
+ declare -F _yarn_test:
+ __yarn_filedir
+ [[ test: == @* ]]
+ COMPREPLY=($(compgen -f -- "$cur"))
++ compgen -f -- test:
+ compopt -o nospace
+ set +x
dsifford commented 6 years ago

Not sure why you're having these issues and unfortunately, I'm too pressed for time to look into it this week.

One thing I will say though is that I've never had issues with run scripts containing colons completing in any situation (using run, or not). So an easier fix might just be within your own dotfiles.

ikornienko commented 6 years ago

I confirm that I always had issues with completion for scripts like test:watch where colon is there (just upgraded to the latest version, nothing changed). It's interesting that if I type yarn test and press Tab I see all possible options, but once I have yarn test: typed then pressing Tab has no effect.

dsifford commented 6 years ago

What's the output of the following command for you @ikornienko

$ echo "bash: ${BASH_VERSINFO[*]}" ::: "bash-completion: ${BASH_COMPLETION_VERSINFO[*]}"

Also, of note, I just started last night actually slowly putting together a basic test suite for this project. Should be a bit safer and more reliable when I get that finished.

End goal is to also fully support workspaces.

ikornienko commented 6 years ago

@dsifford first of all, thank you for looking into it!

$ echo "bash: ${BASH_VERSINFO[*]}" ::: "bash-completion: ${BASH_COMPLETION_VERSINFO[*]}"
bash: 4 4 19 1 release x86_64-apple-darwin17.3.0 ::: bash-completion:

I'm not an expert and cannot tell right away why ${BASH_COMPLETION_VERSINFO[*]} is empty. I have bash-completion version 1.3_3 installed via homebrew.

I have another entry point that may (or may not) help. In package.json scripts I have framerx:build and framerx:bootstrap. When I type yarn framerx completion shows options that include both of them, when I type yarn framerx: completion doesn't show any options (same as I described before), but when I type yarn framerx:b it completes it with yarn framerx:build even though I'd expect it to offer me both options, since it's a common prefix for both of them (but it selects build... maybe because it's defined before bootstrap in my package.json).

dsifford commented 6 years ago

Noted, thanks for the info.

Can't offer any sound advice until I get a moment to check into this, but one thing I will mention right off the rip is that I'd recommend first checking to be sure you have the correct bash-completion installed from homebrew.

Using bash ^4.x, the correct one is bash-completion@2, not bash-completion.

Other than that, sit tight until maybe next weekend and I should be able to investigate further. :+1:

ikornienko commented 6 years ago

Thanks for letting me know about bash-completion2, @dsifford, somehow I overlooked it. Ok, now I have

$ echo "bash: ${BASH_VERSINFO[*]}" ::: "bash-completion: ${BASH_COMPLETION_VERSINFO[*]}"
bash: 4 4 23 1 release x86_64-apple-darwin17.5.0 ::: bash-completion: 2 8

Hm, you know, I re-read @joma74's comment which says

Ok, in this case this is happening. _yarn_test: should not be taken as next cmd. Ends in filedir completion which shoud not.

Since I have build dir, I nuked it... and now after typing yarn framerx:b completion doesn't offer anything. So the fact that it completed yarn framerx:build before was misleading, it actually was inserting dir name.

dsifford commented 5 years ago

@ikornienko @joma74 Are you both still having issues with this? I have a little bit of time this weekend to maybe look into getting a fix out for this.

@ikornienko if you're still having issues, can you provide me with a minimal package.json to test out?

Thanks

ikornienko commented 5 years ago

@dsifford thanks for taking your time to look into it! The repo is here https://github.com/ikornienko/yarn-completion-bug (I've put a description of the behavior I observe on my two machines into the README.md file).

dsifford commented 5 years ago

Awesome, thanks. I'll chime back with what I find when I get a chance to dig into it.

mrmckeb commented 5 years ago

I'm also having this issue on bash (Ubuntu 18.04 in WSL). It ships with the latest version of bash-completion (v2.8).

Once I hit a colon, tabbing no longer does anything.

dsifford commented 5 years ago

Fix should be out sometime later this week. Working on a big overhaul that tackles several longstanding issues

dsifford commented 5 years ago

@mrmckeb @ikornienko Can you gentlemen give the updated completions a try and let me know if it now works for you? There's a lot more improvements peppered everywhere, but just focus on the colon thing for now.

https://github.com/dsifford/yarn-completion/blob/a1b02af98ac68296b3eaac0db892cf8812a66a0c/yarn-completion.bash

mrmckeb commented 5 years ago

@dsifford I have just tried, and it doesn't seem to work still. I replaced the contents of ~/.yarn-completion with this new file, and restarted my terminal.

dsifford commented 5 years ago

What system are you running? Can you go through the same steps as @ikornienko if you’re on MacOS?

dsifford commented 5 years ago

Or better yet, just confirm that you have indeed done the steps outlined in the updated readme (if you're running mac os)

mrmckeb commented 5 years ago

Hi @dsifford, I'm on Windows 10, using WSL. So it's Ubuntu 18.04 with Bash.

I removed all traces of the previous script, and then I ran this:

curl -o /etc/bash_completion.d/yarn-completion.bash https://raw.githubusercontent.com/dsifford/yarn-completion/a1b02af98ac68296b3eaac0db892cf8812a66a0c/yarn-completion.bash

It works the same as before, not better, not worse. I still get no autocomplete once I hit a colon.

I'm running bash-completion@2.8 and bash@4.4.19.

dsifford commented 5 years ago

Might be a silly question, but after downloading the file to your computer, you did either source it or log out and log back in, correct?

ikornienko commented 5 years ago

Same for me as for @mrmckeb , nothing really changed after I replaced the old file. And I did it in this way: killed old yarn completion, restarted the terminal and checked that yarn completion stopped working, then got https://raw.githubusercontent.com/dsifford/yarn-completion/master/yarn-completion.bash, restarted the terminal and checked that yarn completion works, but the bug is still there.

P.S. BTW, when I followed README instructions I got

$ curl -o "${BASH_COMPLETION_USER_DIR:-${XDG_DATA_HOME:-$HOME/.local/share/bash-completion}}/completions/yarn" https://raw.githubusercontent.com/dsifford/yarn-completion/master/yarn-completion.bash
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:--  0:00:03 --:--:--     0Warning: Failed to create the file 
Warning: /Users/ikornienko/.local/share/bash-completion/completions/yarn: No 
Warning: such file or directory
 12 15278   12  1916    0     0    499      0  0:00:30  0:00:03  0:00:27   500
curl: (23) Failed writing body (0 != 1916)

Which is true, I don't have the corresponding directory. So instead I just put it into /usr/local/share/bash-completion/completions.

dsifford commented 5 years ago

Hm, well that's disappointing..

I can't for the life of me figure out why it works for me on both my desktop (Arch Linux) and my Laptop (Macbook Pro) and not for you guys.

I guess the only other thing I can point to is the following lines from the bash-completion README...

Q. Completion goes awry when I try to complete on something that contains a colon.

A. This is actually a 'feature' of bash. bash recognises a colon as starting a new completion token, which is often what you want when completing something like a PATH variable:

export PATH=/bin:/sbin:/usr Without the special treatment of the colon, the above wouldn't work without programmable completion, so it has long been a feature of the shell.

Unfortunately, you don't want the colon to be treated as a special case when doing something like:

man File::B Here, the colons make bash think that it's completing a new token that begins with 'B'.

Unfortunately, there's no way to turn this off. The only thing you can do is escape the colons with a backslash.

So I suppose the remedy in your cases would be to just stop using colons and instead use dashes or something.

Sorry I couldn't be more helpful.

dsifford commented 5 years ago

@ikornienko @jasonkarns I was under the assumption that you guys had menu-complete enabled this entire time.

I'm able to reproduce this using the default complete method, but why would anybody prefer that over menu-complete!

Just enable menu-complete! That should fix it for you.

dsifford commented 5 years ago

Add this to your ~/.inputrc

# Cycle through completions, rather than dumping all options
TAB:       menu-complete
"\e[Z":    menu-complete-backward
ikornienko commented 5 years ago

Thanks for the investigation, @dsifford ! Ok, so it seems like completion after : won't work because of bash-completion's limitation / feature. It does work to some degree if one escapes colon (e.g. yarn test\: and then triggers the completion through TAB). And this limitation holds for both default and menu-complete method (i.e. even with menu-complete yarn test: and pressing TAB won't do anything).

At the same time with menu-complete one can type yarn test, hit TAB and iterate through all the possible options including options with colon, which almost entirely remediates the problem! Thank you for all the research and suggestions!

mrmckeb commented 5 years ago

Yes, thanks for the effort @dsifford!