cantino / mcfly

Fly through your shell history. Great Scott!
MIT License
6.85k stars 178 forks source link

Clean PROMPT_COMMAND before joining with a semicolon #160

Closed cantino closed 3 years ago

cantino commented 3 years ago

Should fix #159.

cantino commented 3 years ago

@SafariMonkey, does this seem okay?

SafariMonkey commented 3 years ago

Personally, I would double quote the inner $PROMPT_COMMAND, because otherwise you're relying on echo putting the arguments back together correctly - probably safe, but preferably bypassed altogether.

I tried putting it into shellcheck.net:

#!/bin/bash
PROMPT_COMMAND="mcfly_prompt_command;$(echo $PROMPT_COMMAND | sed 's/^;//')"

And got out:

$ shellcheck myscript

Line 2:
PROMPT_COMMAND="mcfly_prompt_command;$(echo $PROMPT_COMMAND | sed 's/^;//')"
                                       ^-- SC2001: See if you can use ${variable//search/replace} instead.
                                            ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: (apply this, apply all SC2086)
PROMPT_COMMAND="mcfly_prompt_command;$(echo "$PROMPT_COMMAND" | sed 's/^;//')"

$

It recommends the quoting of the variable, as I mentioned. It also suggests trying to use the native substitution capabilities built into bash instead of using sed. Looking at that a little more, I'm pretty sure the following is equivalent to your command:

PROMPT_COMMAND="mcfly_prompt_command;${PROMPT_COMMAND#;}"

Shellcheck is happy with that.

cantino commented 3 years ago

Thanks @SafariMonkey! My bash skill level is fairly low, so I really appreciate another set of eyes.

Without the substitution:

PROMPT_COMMAND=";echo hi" ./dev.bash
bash: PROMPT_COMMAND: line 0: syntax error near unexpected token `;;'
bash: PROMPT_COMMAND: line 0: `mcfly_prompt_command;;echo hi'

With the substitution:

PROMPT_COMMAND="echo hi" ./dev.bash
hi
bash-3.2$ echo $PROMPT_COMMAND
mcfly_prompt_command;echo hi
PROMPT_COMMAND=";echo hi" ./dev.bash
hi
bash-3.2$ echo $PROMPT_COMMAND
mcfly_prompt_command;echo hi
cantino commented 3 years ago

This should fix your issue @alexxbb.

cantino commented 3 years ago

For future reference: https://tldp.org/LDP/abs/html/string-manipulation.html

alexxbb commented 3 years ago

Thank you @cantino . I will try it as soon as I can.