Append to PROMPT_COMMAND for prompts utilizing $? #90

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

notwa commented May 3, 2013

my bad about the commit summary being too long

Owner

rupa commented May 3, 2013

As written this breaks when you have an existing PROMPT_COMMAND and ended it with a semicolon, or none at all (leading semicolon is no bueno). The reason I prepend is cause it's the least amount of code that is reliable no matter what you've already done. That said, I'm for this, but we need to account for:

  • no PROMPT_COMMAND
  • existing PROMPT_COMMAND with trailing semicolon
  • existing PROMPT_COMMAND without trailing semicolon

and ideally, we account for all those with brevity and panache.

Contributor

notwa commented May 4, 2013

good observation, I'll think about that. it might be better to store and restore $? for this specific case.

Owner

rupa commented May 4, 2013

nah I think prepending is a good idea by the principle of least surprise,
its just harder, we have to account for all the possible cases

On Friday, May 3, 2013, notwa wrote:

good observation, I'll think about that. it might be better to store and
restore $? for this specific case.


Reply to this email directly or view it on GitHubhttps://github.com/rupa/z/pull/90#issuecomment-17425399
.

Contributor

notwa commented May 9, 2013

Note that it converts "; " to ":;". Not terse enough methinks so I'm looking into using BASH_REMATCH.

Contributor

notwa commented May 11, 2013

don't know what else to do

Owner

rupa commented May 17, 2013

Thanks for the work ... I'm going to take a look at what you have when I get a chance, and either use yours or make my own implementation. i do agree with the intent.

My pull request above is for an alternate approach to fixing this problem, and it's quite a bit simpler than the regex-based approach that notwa came up with. I think it scores better on the "brevity and panache" scale without sacrificing any correctness or flexibility.

Owner

rupa commented May 27, 2013

hmm, nope, not yet,

Owner

rupa commented May 27, 2013

OK, that should do it:

x=''
echo ${x%;}${x:+;}'cmd2;'
x='cmd'
echo ${x%;}${x:+;}'cmd2;'
x='cmd;'
echo ${x%;}${x:+;}'cmd2;'

all works.

Contributor

notwa commented May 27, 2013

not with x=' ' but it's an odd case. yours is probably the most obvious in intent.

Owner

rupa commented Jun 15, 2013

Merged into master.

rupa closed this Jun 15, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment