dsifford / yarn-completion

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

Breaks when there is }, in scripts #10

Closed kachkaev closed 6 years ago

kachkaev commented 6 years ago

Having }, inside one of the scripts breaks autocompletion. MWE:

{
  "scripts": {
    "hello1": "echo 'hello1'",
    "hello2": "echo 'hello2'",
    "hello3": "echo 'hello3'",
    "hello4": "echo 'hello4'",
    "hello5": "echo 'hello5'"
  }
}

yarn run hello →tab hello1 hello2 hello3 hello4 hello5 (ok) npm run hello →tab hello1 hello2 hello3 hello4 hello5 (ok)


{
  "scripts": {
    "hello1": "echo 'hello1'",
    "hello2": "echo '},'",
    "hello3": "echo 'hello3'",
    "hello4": "echo 'hello4'",
    "hello5": "echo 'hello5'"
  }
}

yarn run hello →tab hello1 hello2 (broken) npm run hello →tab hello1 hello2 hello3 hello4 hello5 (still ok)


A real-world script that broke yarn completion was:

    `"format": "prettier --write --ignore-path .gitignore \"{.,src/**,test/**}/{*.{j,t}s{x,},*.md,ts*.json}\"",

Could the reason be in this line? https://github.com/dsifford/yarn-completion/blob/689daceca5a3b9d3dea157bd30b59bd86fa3d09b/yarn-completion.bash#L42

dsifford commented 6 years ago

Very likely.

The way the JSON is parsed out for script names is pretty archaic since the goal is to keep it portable across platforms.

Happy to take a PR on it if you feel like making the parsing with sed work better

kachkaev commented 6 years ago

I'm pretty sure I won't be able to crack it in any reasonable time because my level in bash script editing in 0.01 out of 1 😅

Speaking of a a simple fix, could a RegExp that searches for the end of "scripts" look like so?

This still won't be as clean as parsing json in full, but the likelihood of bugs should get smaller.

dsifford commented 6 years ago

@kachkaev This should be fixed now in 0.6.0.

kachkaev commented 6 years ago

Thanks for trying to fix this @dsifford! It could be that I've done something wrong, but after updating to 0.6.0 script detection broke completely. I still see it working as it was in the pre-update terminal tabs, but in all new tabs I get:

yarn run →tab yarn run env yarn →tab all standard yarn commands, no helloX commands

npm run still shows hello1 ... hello5 as it should.

I tested 0.6.0 on a couple of real-world packages in addition to the MWE above and the problem reproduced in all cases.

What could be causing this in brew's bash 4.4.19(1)-release (x86_64-apple-darwin17.3.0) with yarn-completion 0.6.0 also installed via homebrew?

dsifford commented 6 years ago

Not sure. I tried all the examples you provided when I made the change and it worked for me.

And actually, the change I made to the sed command made it even more POSIX compliant than the earlier one (the earlier one used extended regexp).

Did you try logging out completely and logging back in to make sure you weren't using a stale file or something?

dsifford commented 6 years ago

I'll try in on my macbook here in the next hour or so and see if it's a mac quirk or something.

kachkaev commented 6 years ago

Same behaviour even after a full reboot. Curious to know what your mac test tells you! Thanks a lot for looking into this problem 🚀

dsifford commented 6 years ago

Thanks for checking.

Do you have a repo that has a non-parsable package.json file that I can clone when I test on my macbook? I want to be certain that I'm reproducing this correctly.

dsifford commented 6 years ago

@kachkaev Cant reproduce on my macbook.

Tried it using the package.json in this repo and it worked just fine for me: https://github.com/callthemonline/client-nextjs

dsifford commented 6 years ago

Also, I just tried it again using both GNU and BSD sed and it still worked for me.

I assume you have the default BSD sed installed on your system? Can you try installing GNU sed and see if that works?

dsifford commented 6 years ago

@kachkaev what's the output when you enter shopt -p | sort?

kachkaev commented 6 years ago
screen shot 2018-02-24 at 07 54 27

Downgrading to 0.5.1 by running

curl -L https://raw.githubusercontent.com/dsifford/yarn-completion/c40137136d8b803407f5f2fb99c1ae92af594b07/yarn-completion.bash > `brew --prefix`/etc/bash_completion.d/yarn

returns yarn-completion to its old behaviour described in this issue (after Terminal is relaunched, of course). This means that my setup is not broken as such and it's not the case of me having using two conflicting yarn completions somehow.

Upgrading one commit up to 3b5a4d7336fef8ac9342509d3637eadc8521111f makes things as broken as on current master (0.6.0), i.e. I only see yarn runrun yarn env. The behaviour is the same both for https://github.com/callthemonline/client-nextjs and a dummy project where package.json just contains hello1...5 scripts.

dsifford commented 6 years ago

@kachkaev I have no idea why this isn't working for you. I just matched all my shell options to the ones you have enabled and it still works perfectly fine on my end.

The only thing I can think of that it might still be is that you're using BSD sed. If you can confirm that the issue is still persisting even using GNU sed, then I'm out of ideas and will have to unfortunately wait until more people report this issue.

kachkaev commented 6 years ago

Thanks for for your hint about GNU sed – turns out this is exactly what caused trouble! Originally, even after installing GNU bash, man sed was starting like:

screen shot 2018-02-24 at 19 11 57

  

After I did brew install gnu-sed --with-default-names and restarted the Terminal, it changed to:

screen shot 2018-02-24 at 19 11 42

Consequently, yarn autocompletion 0.6.0 began to work! 🎉

I'm a frequent user of the command line, but am no way an expert in bash versions and flavours of other standard tools. All this is pretty foggy to me, so thanks for your help with sorting things out! To summarise, this is what I had to do in order to get yarn-completion on macOS High Sierra working:

brew install bash
brew install gnu-sed --with-default-names

brew install bash-completion
## + copy a few commands to ~/.bash_profile as instructed by brew after bash-completion setup

curl -L https://raw.githubusercontent.com/dsifford/yarn-completion/master/yarn-completion.bash > `brew --prefix`/etc/bash_completion.d/yarn

## restart Terminal

Hope this helps someone else in a similar situation!

dsifford commented 6 years ago

Urrrrgh! BSD utils are going to be the death of me, I swear....

Glad to hear we've found the culprit. I'll try and get this compatible with BSD as soon as feasible. Thanks for your patience.

kachkaev commented 6 years ago

Feel free to supplement README with my setup commands. I can easily imagine someone else getting into a similar trouble, so it’ll be great if we can save their time!

dsifford commented 6 years ago

Open a PR with the edits and I'll be happy to merge

dsifford commented 6 years ago

Version 0.6.1 should work in BSD sed now.

kachkaev commented 6 years ago

Thank you! 0.6.1 worked for me even after uninstalling gnu-sed 🎉