MC clobbers the PROMPT_COMMAND shell variable

[wjaguar]

When starting a Bash subshell, MC overwrites the PROMPT_COMMAND shell variable, instead of adding to it. Thus user's settings from ~/mc/bashrc are lost.
This breaks use-cases like the one described here:
​http://forum.ducea.com/discussion/23/bash-history-for-multiple-session/

Patch is attached.

[storchaka]

Use
"PROMPT_COMMAND=\"$PROMPT_COMMAND${PROMPT_COMMAND:+ }\"'pwd>&%d;kill -STOP $$'\n"

[storchaka]

Sorry, need to use
"PROMPT_COMMAND=\"$PROMPT_COMMAND${PROMPT_COMMAND:+;}\"'pwd>&%d;kill -STOP $$'\n"

[doktornotor]

Well, this patch causes "bad descriptor" and some other weird error stuff occasionally, at least here. Tested with PROMPT_COMMAND='history -a; history -n' from the link above.

[wjaguar]

Replying to doktornotor:

Well, this patch causes "bad descriptor" and some other weird error stuff occasionally, at least here.

Well, here it doesn't (Slackware 12.1, bash 3.1.017, mc 4.7.0.1).
Which is one reason why I run Slackware; a stable system which doesn't throw random weird errors at me, makes it much easier to find and fix real bugs. ;-)

[doktornotor]

@wjaguar: Well, the reason it's throwing such stuff is not a broken system. Particularly, working with Gentoo portage (emerge and such) which makes heavy and extensive use of bash features seemed to produce the bad descriptor issue quite often with this patch.

Also, wrt the use case described in the issue description, well, that certainly didn't work for me as described, plus caused ignoredups to be essentially ignored. Doesn't look like a very viable use case to me.

[wjaguar]

Replying to doktornotor:

@wjaguar: Well, the reason it's throwing such stuff is not a broken system.

Reeeally? ;-)
Well, if you can offer _any_ viable mechanism by which this patch could cause something which is NOT broken to misbehave, I'm all ears. Otherwise... well, in my experience, _unexplainable_ glitches are just a sign that one is using too-unstable stuff.

Particularly, working with Gentoo portage

Just what I had in mind, yes. ;-)

(emerge and such) which makes heavy and extensive use of bash features seemed to produce the bad descriptor issue quite often with this patch.

And which bash version exhibits this problem?
BTW - autoconf-generated configure scripts aren't exactly leaving bash idle, either; but nothing untoward does ever happen on my system after the patch. Same with SlackBuild scripts.

Also, wrt the use case described in the issue description, well, that certainly didn't work for me as described,

So it certainly looks like a very broken bash.
Because, see, if it hadn't been working just as advertised on MY system, I would have no reason in the world to patch mc to prevent it interfering, now would I? ;-)

plus caused ignoredups to be essentially ignored.

Again, ignoredups works for me - inside each separate bash instance. And the rare cross-instance dups are taken care of with a Perl one-liner.

Doesn't look like a very viable use case to me.

Myself, I don't care either way. I have made a SlackBuild with the patches I need, and intend to expand it further when another "use case" of some never-fixed mc bug becomes too annoying to me.

But still I think it common courtesy to report the bugs I've found and fixed in others' projects. Because I myself dislike it when users of _my_ software don't report the bugs they've encountered, leaving me no chance to fix them.

[doktornotor]

Please take this distro-based politics crap outside of this bug. I'm merely reporting that your patch causes problems. Period. And thanks, my bash is just fine.

[wjaguar]

Replying to doktornotor:

Please take this distro-based politics crap outside of this bug. I'm merely reporting that your patch causes problems. Period.

Please leave emotions aside when discussing technical stuff. And remember - "correlation is not causation".

And thanks, my bash is just fine.

And am I to believe the patch is "causing problems" BY MAGIC, then? (I gather if you had any realistic explanation, you would have told me of it, yes?)

[doktornotor]

Replying to wjaguar:

Please leave emotions aside when discussing technical stuff. And remember - "correlation is not causation".

You are not discussing any technical stuff here, you are just bashing a distro you don't use because obviously it must be "very broken" and "too unstable stuff" just because you don't have problems with your patch but other people have.

And am I to believe the patch is "causing problems" BY MAGIC, then?
(I gather if you had any realistic explanation, you would have told me of it, yes?)

I don't have time to dig into this. A command reads its input from stdin, prints normal output to stdout and error ouput to stderr. If one of the 3 fd's isn't open, then you get "bad file descriptor". It isn't a problem until you start appending stuff you PROMPT_COMMAND like you do in your patch - at least here. Once you do that, you get the problem I've described. Why? Your patch, you take care of explanations.

Your patch is causing problem that wasn't there before until you tried to fix the issue described in this bug. This patch breaks more than it fixes for me, so I'm commenting here b/c I'm obviously not interested in getting a patch committed that breaks more than it fixes (i.e. the illusive history synchronization b/w multiple sessions.

[wjaguar]

Replying to doktornotor:

You are not discussing any technical stuff here, you are just bashing a distro you don't use because obviously it must be "very broken" and "too unstable stuff" just because you don't have problems with your patch but other people have.

No, I'm just pointing to the obvious fact that trivial modifications of one program triggering unrelated random errors in another, is a thing which very rarely happens with stable software.
Besides, isn't "I don't have problems but other people have" the very definition of the difference between stable system and unstable? ;-)

It isn't a problem until you start appending stuff you PROMPT_COMMAND like you do in your patch - at least here. Once you do that, you get the problem I've described. Why? Your patch, you take care of explanations.

Why should I? The patch is trivial, and it is for mc, it doesn't touch bash's code in any way. PROMPT_COMMAND is a documented variable of bash; mc's clobbering it is an obvious bug (not a documented behaviour, at the very least). I use the variable in a documented way to pass documented commands with documented options to bash. Which, on my system, works beautifully and solves a real problem.

And even if I did pass "rm -rf /" instead, what business is that of yours? Your system is yours to configure, and leaving global PROMPT_COMMAND unset is entirely within your power. As is debugging your copy of bash and/or filing a bugreport in an appropriate place.

[doktornotor]

Outta here... I'll revert the patch for myself if needed if it gets commited. Other than that - I wanted to comment on a patch causing problems with (frankly, already pretty hackish) subshell stuff in mc, NOT to be preached by some freaking Slackware evangelist.

[ossi]

Replying to storchaka:

Sorry, need to use
"PROMPT_COMMAND=\"$PROMPT_COMMAND${PROMPT_COMMAND:+;}\"'pwd>&%d;kill -STOP $$'\n"

this seems more elegant :)

"PROMPT_COMMAND=${PROMPT_COMMAND:+$PROMPT_COMMAND; }'pwd>&%d;kill -STOP $$'\n"

Replying to doktornotor:

Well, this patch causes "bad descriptor" and some other weird error stuff [...]

this kinda makes no sense unless any of the built-in commands decide to close the pipe's file descriptor. play some with lsof.

btw, a cleaner solution than inheriting pipe file descriptors would be creating a randomly named fifo and having the prompt command write into it. that way random programs started from the shell would not inherit the handle.

[kecsap]

I just would like to ask to include this one or similar patch to the subshell creation. This problem really breaks the custom prompts which is very useful in development with git. Currently, the prompt variable is overwritten and if I source the prompt modification script in the subshell, the Midnight Commander does not follow the directory changes in the subshell when switching with Ctrl+O back and forth.

I read some of the previous comments, please do not waste energies on flames, but fixing the problem. Even a new command line option/some kind of configuration or shell variable can be fine to have the possibility to override the current behavior of the mc.

Feedback: the proposed patch works fine for me with bash 4.2, mc 4.7.0.9 on Ubuntu 11.04 (Natty).

[egmont]

Friendly ping, dear developers...

I was about to report this bug now. How come it's been unfixed for 5 years!? :( Obviously overwriting PROMPT_COMMAND is harmful, mc should definitely prepend/append to it, as it does for zsh.

This would probably also make ticket #3088 unnecessary.

(Similar bugreport in another product: ​https://bugzilla.gnome.org/show_bug.cgi?id=704960)

[andrew_b]

Branch: 2027_bash_prompt
changeset:ff827ad6578cf69c95ebb99aed79194c59848f17

[egmont]

Thanks!

Fedora + Gnome-terminal are working on a feature that you get notified if a command in a non-visible terminal tab completes (​https://bugzilla.gnome.org/show_bug.cgi?id=711059). This will be technically achieved by emitting a special escape sequence from PROMPT_COMMAND. By appending rather than replacing, this will also work when the command is launched from mc.

[egmont]

As for #3088 (gnome-terminal opening the new tab in the same dir as the current tab – this is also achieved by emitting an escape sequence from PROMPT_COMMAND telling the cwd to the terminal):

So far it didn't work at all from mc. With this patch, it sometimes works. If you execute a command from a directory, after its completion the new tab will open from that dir (since the prompt is printed and hence PROMPT_COMMAND is executed when the commad finishes). However, it doesn't work after a simple directory change, PROMPT_COMMAND is not executed and hence the relevant sequence is not emitted in this case.

The best would probably be if mc could send a cd command to the subshell each time the directory changes. I'm not familiar with the subshell code to tell if this can be implemented without undesirable side effects.

Other approaches could be manually duplicating executing the shell's pre-prompt hooks (but this sounds fragile in the long run), or independently of this ticket apply 3088's patch.

[pronobis]

That branch works great. Thanks! Btw. Is there a way to get the prompt colors show up also in MC?

[andrew_b]

Replying to pronobis:

Is there a way to get the prompt colors show up also in MC?

Unfortunately no. It is not supported, аll control sequences in the prompt are ignored.

[andrew_b]

Merged to master: [1f38576741ad9e11343f3f9c9aca909d0fc51fe3].