search

scmbreeze / scm_breeze

Adds numbered shortcuts to the output git status, and much more
https://madebynathan.com/2011/10/19/git-shortcuts-like-youve-never-seen-before/
MIT License
2.82k stars 192 forks source link

Fix parsing of escaped shell command list #283

Closed kesyog closed 4 months ago

kesyog commented 5 years ago

A change made in a6eeebf broke parsing of scmb_wrapped_shell_commands. Rather than iterating over each command in the space-separated list, the entire block of commands was treated as a single command.

HaleTom commented 5 years ago

Thanks for catching and reverting this.

I suspect there is an issue with $IFS which has this behave in a non-standard way.

Would you be willing to write a failing test that this fixes?

ghthor commented 5 years ago

Yeah, I wonder if this can be fixed by just explicitly setting a local IFS=' ' here?

On Tue, Oct 30, 2018 at 12:39 PM Tom Hale notifications@github.com wrote:

Thanks for catching and reverting this.

I suspect there is an issue with $IFS which has this behave in a non-standard way.

Would you be willing to write a failing test that this fixes?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/scmbreeze/scm_breeze/pull/283#issuecomment-434375116, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJyKny9EqgUES7Bqf3zfvpePyuJw1v1ks5uqIDIgaJpZM4YB-VH .

kesyog commented 5 years ago

@HaleTom Yeah, I'll look into adding a test and will poke around to see if this can be fixed by setting IFS appropriately

HaleTom commented 5 years ago

@kesyog have you seen #284?

Can you report when using an array instead, and perhaps show an minimal illustrative example of the issue?

kesyog commented 4 years ago

@HaleTom sorry for the super late reply. I'm using zsh and using an array does work properly in my .git.scmbrc: e.g. scmb_wrapped_shell_commands=(vim emacs gedit cat rm cp mv ln cd ls less subl code)

I couldn't figure out how to make a failing test case in ZSH, but then realized that shwordsplit is set in the test file: https://github.com/scmbreeze/scm_breeze/blob/master/test/lib/git/shell_shortcuts_test.sh#L13-L15.

While this fix works for me, but https://github.com/scmbreeze/scm_breeze/issues/297#issuecomment-551137282 is probably the better option.