- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MC clobbers the PROMPT_COMMAND shell variable #2027
Comments
|
Use |
Sorry, need to use |
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. |
|
Replying to doktornotor:
Well, here it doesn't (Slackware 12.1, bash 3.1.017, mc 4.7.0.1). |
@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. |
Replying to doktornotor:
Reeeally? ;-)
Just what I had in mind, yes. ;-)
And which bash version exhibits this problem?
So it certainly looks like a very broken bash.
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.
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. |
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. |
Replying to doktornotor:
Please leave emotions aside when discussing technical stuff. And remember - "correlation is not causation".
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?) |
Replying to wjaguar:
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.
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. |
Replying to doktornotor:
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.
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. |
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. |
Replying to storchaka:
this seems more elegant :)
Replying to doktornotor:
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. |
|
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). |
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) |
Branch: 2027_bash_prompt |
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. |
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. |
That branch works great. Thanks! Btw. Is there a way to get the prompt colors show up also in MC? |
Replying to pronobis:
Unfortunately no. It is not supported, аll control sequences in the prompt are ignored. |
|
|
|
Important
This issue was migrated from Trac:
wjaguar
(@wjaguar)egmont@….com
(@egmontkob),pahan@….info
(@Hubbitus)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.
Note
Original attachments:
wjaguar
(@wjaguar) onFeb 14, 2010 at 13:23 UTC
The text was updated successfully, but these errors were encountered: