Slow startup in pantheon-terminal

[egmont]

In pantheon-terminal it takes 10 seconds for mc to start up.

This is because pantheon-terminal sets

PROMPT_COMMAND='dbus-send [blahblahblah];'

with a semicolon at the end, and mc appends to PROMPT_COMMAND using semicolon as separator. However, two semicolons (even if separated by spaces) is a syntax error in bash.

One possible solution is to use a newline separator instead, see patch.

Another approach could be to strip off trailing whitespaces and then a trailing semicolon, it's really cumbersome.

Yet another approach could be to prepend to PROMPT_COMMAND and then use ';' as separator. This would be okay because a command beginning with semicolon is a syntax error, so we'd only get error by two consecutive semicolons if the command was broken to begin with. However I'm not sure if the "kill -STOP $$" has to be at the very end, and it's safer not to tamper with that.

[and]

Hi,
i think this will lead to an error when origin PROMPT_COMMAND not ending with ';' separator

current:

$ PROMPT_COMMAND=
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+$PROMPT_COMMAND; }pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
pwd>&%d;kill -STOP 12477
$ PROMPT_COMMAND="history -a;history -c; history -r"
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+$PROMPT_COMMAND; }pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
history -a;history -c; history -r; pwd>&%d;kill -STOP 12477
$ PROMPT_COMMAND="history -a;history -c; history -r;"
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+$PROMPT_COMMAND; }pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
history -a;history -c; history -r;; pwd>&%d;kill -STOP 12477

Yes, known problem when origin PROMPT_COMMAND already ended with ';' separator

with patch:

$ PROMPT_COMMAND=
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+$PROMPT_COMMAND}pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
pwd>&%d;kill -STOP 12477
$ PROMPT_COMMAND="history -a;history -c; history -r"
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+$PROMPT_COMMAND}pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
history -a;history -c; history -rpwd>&%d;kill -STOP 12477
$ PROMPT_COMMAND="history -a;history -c; history -r;"
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+$PROMPT_COMMAND}pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
history -a;history -c; history -r;pwd>&%d;kill -STOP 12477

Now we have a problem when origin PROMPT_COMMAND ended _without_ ';' separator

We need to deal with selective ';' separator adding, how about this:

$ PROMPT_COMMAND=
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+${PROMPT_COMMAND%;};}pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
pwd>&%d;kill -STOP 12477
$ PROMPT_COMMAND="history -a;history -c; history -r"
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+${PROMPT_COMMAND%;};}pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
history -a;history -c; history -r;pwd>&%d;kill -STOP 12477
$ PROMPT_COMMAND="history -a;history -c; history -r;"
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+${PROMPT_COMMAND%;};}pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
history -a;history -c; history -r;pwd>&%d;kill -STOP 12477

but it will not catch trailing whitespaces after last ';' separator

$ PROMPT_COMMAND="history -a;history -c; history -r;  "
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+${PROMPT_COMMAND%;};}pwd>&%d;kill -STOP $$" ; echo $PROMPT_COMMAND_MCSUBSHELL
history -a;history -c; history -r; ;pwd>&%d;kill -STOP 12477

Sadly we cannot nest ${var} expressions to catch trailing whitespaces too.

[egmont]

I don't get you. You're not testing what my patch does. You simply removed adding the semicolon, which is of course broken. My patch changes it to a newline.

Selectively removing the trailing semicolon is complicated (as you said you don't remove trailing whitespaces which you should), and is very risky, e.g. what if it was a backslahsed literal semicolon?

[and]

When using a newline character as control operator and delimiter for origin PROMPT_COMMAND and MC subshell cwd shell code,
it looks not so intuitive when dealing/echoing $PROMPT_COMMAND inside of a mc subshell.

normal shell# echo $PROMPT_COMMAND
history -a;history -c; history -r;
mc subshell # echo $PROMPT_COMMAND
history -a;history -c; history -r; pwd>&6;kill -STOP $$

normal shell# echo $PROMPT_COMMAND
history -a;history -c; history -r
mc subshell # echo $PROMPT_COMMAND
history -a;history -c; history -r pwd>&6;kill -STOP $$

When using selective ';' delimiter adding (depends on last character of origin PROMPT_COMMAND)

--- a/src/subshell.c
+++ b/src/subshell.c
@@ -878,7 +878,7 @@
     {
     case BASH:
         g_snprintf (precmd, sizeof (precmd),
-                    " PROMPT_COMMAND=${PROMPT_COMMAND:+$PROMPT_COMMAND; }'pwd>&%d;kill -STOP $$'\n",
+                    " PROMPT_COMMAND=${PROMPT_COMMAND:+${PROMPT_COMMAND%;};}'pwd>&%d;kill -STOP $$'\n",
                     subshell_pipe[WRITE]);
         break;

its looks more happier to me.

normal shell$ echo $PROMPT_COMMAND
history -a;history -c; history -r;
mc subshell $ echo $PROMPT_COMMAND
history -a;history -c; history -r;pwd>&6;kill -STOP $$

normal shell$ echo $PROMPT_COMMAND
history -a;history -c; history -r
mc subshell $ echo $PROMPT_COMMAND
history -a;history -c; history -r;pwd>&6;kill -STOP $$

[egmont]

We're talking about source code. It doesn't have to be intuitive. How intuitive is "pwd>&6;kill -STOP $$" anyway? I've no clue what/how/why it does.

Using echo $PROMPT_COMMAND without quotes is an error on your side. It does what you ask it to do. You should do an echo "$PROMPT_COMMAND" with double quotes to see the actual value without mangling the spaces and newlines. It's not mc's fault if you make such a mistake.

That being said, PROMPT_COMMAND could be modified to have $'\n' (literal 5 chars: dollar-apostrophe-backslash-n-apostrophe) after the % sign instead of a literal newline (LF as a single char).

$ PROMPT_COMMAND="history -a;history -c; history -r"
$ PROMPT_COMMAND_MCSUBSHELL="${PROMPT_COMMAND:+$PROMPT_COMMAND$'\n'}pwd>&%d;kill -STOP $$" ; echo "$PROMPT_COMMAND_MCSUBSHELL"
history -a;history -c; history -r
pwd>&%d;kill -STOP 2642

The backslash of course would need to be doubled in the C source file.

g_snprintf (precmd, sizeof (precmd), 
            " PROMPT_COMMAND=${PROMPT_COMMAND:+$PROMPT_COMMAND$'\\n'}'pwd>&%d;kill -STOP $$'\n",

I really don't understand your goal, can't see what's your problem with my patch, and why you keep insisting on another solution which is known to be buggy in at least two different ways (breaks if there's a trailing space, breaks if the last semicolon is backslashed).

[mooffie]

For the record, two other users complain of this problem due to bad PROMPT_COMMAND:

• ​A user "x-yuri" had "history -a; history -c; history -r;" in his PROMPT_COMMAND. A trailing ";".

• ​A user on our own mailing list a few months back too had his Bash stumble upon a PROMPT_COMMAND-related syntax error (his strace dump says "bash: PROMPT_COMMAND: line 1: sy", the "sy" being the start of "syntax error"). Most probably he too has a triling ";", but to make sure I've just sent him an email asking him for his PROMPT_COMMAND.

(Interestingly, a ​user "jtkmt" says a "\C-j" binding in his .inputrc caused the problem. But I don't know if it's relevant to us.)

[mooffie]

So, the question is whether we care that "echo $PROMPT_COMMAND" (without quotes) might produce a somewhat confusing output. Do we?

If we do care, let's use "\n;\n" instead of just "\n".

(Yes, this will make echo produce "whatever ; ; pwd ..." if the original command already ended with ";", but why should we care?)

[egmont]

For the record, two other users complain of this problem due to bad PROMPT_COMMAND:

Thanks for these pointers!

So, the question is whether we care that "echo $PROMPT_COMMAND" (without quotes) might produce a somewhat confusing output. Do we?

IMO no, absolutely not! Why should we ever care that someone who choses to use the wrong tool for debugging is misguided because the wrong tool misguides him/her?

If we do care, let's use "\n;\n" instead of just "\n".

But, just as you show with the double semicolons, it's not any more correct, so why bother and why overcomplicate it?

In the mean time I realized that my $'\n' proposal wouldn't help; it would be the step when PROMPT_COMMAND is defined that would become more readable, but the actual new value of PROMPT_COMMAND would be the same, containing an actual LF.

[mooffie]

My apologies:

The "\n;\n" I proposed turns out to be a syntax error (because ";" can't start a line).

(It was a simplification of my original thought, to use "\n:;\n". While "\n:;\n" isn't a syntax error, it doens't at all serve the purpose of making 'echo's confusing output better.)

To sum it up:

@egmont's solution, of using a single "\n", seems to me to be the most rational/maintainable solution.

(It is a bummer that 'echo $PROMPT_COMMAND' could confuse people, though.)

[egmont]

Replying to mooffie:

(It is a bummer that 'echo $PROMPT_COMMAND' could confuse people, though.)

I agree. If there was an equally good and robust solution that doesn't suffer from this inconvenience then I'd go for that. Alas I can't think of such a solution.

[egmont]

I'm wondering if something along the lines of

    export PROMPT_COMMAND_MCBAK="$PROMPT_COMMAND"
    export PROMPT_COMMAND='eval "$PROMPT_COMMAND_MCBAK"; pwd...;kill...'

could work reliably. I'll take a closer look.

[mooffie]

Replying to egmont:

I'm wondering if something along the lines of

    export PROMPT_COMMAND_MCBAK="$PROMPT_COMMAND"
    export PROMPT_COMMAND='eval "$PROMPT_COMMAND_MCBAK"; pwd...;kill...'

This seems to me preferable over the "\n" solution. Good thinking!

(It serves the purpose: echo's output won't be misleading. Albeit a "complex" output, it isn't any more complex than it currently is (maybe a tad less comfortable to read, because of the indirection, but not more complex), and the "intended" crowd is supposed to be smart enough to grok it.)

Replying to egmont:

[if] could work reliably. I'll take a closer look.

I did some reading and couldn't find a problem with this. I'm not a Bash lawyer, though. I'd certainly want to see this solution chosen, unless of course somebody points out a problem in it (or manages to comes up with something better).

[egmont]

Here's a patch for the second approach. It's quite complicated...

• eval's first arg should be a '--' so that if old PROMPT_COMMAND begins with a dash then it's not taken as an argument to eval.

• Special ${var:-} trick is used so that it doesn't exit if the variable is undefined but "set -eu" was specified in the shell.

• Theoretically it's not recursive, as opposed to prepending you couldn't perform the same steps again to "prepend" another action. In other words, breaks if PROMPT_COMMAND_MC_SAVE was already defined and eval'd from PROMPT_COMMAND. For this a unique variable name (e.g. containing the PID) should be chosen. In practice it's not a problem because you cannot run multiple MCs nested.

• Since and & mooffie expressed it's important for them to have a PROMPT_COMMAND as readable as possible, I decided to omit eval'ing an empty variable if the old PROMPT_COMMAND is empty/nonexistent, although I'm not sure it's the good approach.

• TODO: To export or not to export? PROMPT_COMMAND may or may not be exported, and if it is, us overriding its value still keeps it exported. Then another shell run from there would try to eval a variable that's not defined, causing again problems with "set -u". Maybe PROMPT_COMMAND_MC_SAVE should always be exported??? Might be confusing in a subprocess. Or check whether PROMPT_COMMAND is exported and only export in that case??? Makes the initializer shell code in subshell.c very complex. I don't want to alter the exported/nonexported status of PROMPT_COMMAND.

With all these subtle tricky cases, I cast my vote at the first approach with the embedded newline.

[and]

We need more votes/thoughts :)

KISS should be common sense, complex solution for all theo. cases should avoid, are we agreed?

We are inside of $PROMPT_COMMAND and need to add mc-ish commands to get cwd
$PROMPT_COMMAND can be empty or predefined by user needs
So we need explicit command separation before we add our mc-ish commands

we have <newline> vs <semicolon> as command separator

Usually <newline> is used as separator within shell scripts files, <semicolon> is used _inside_ of a variable
It is unusual to use <newline> _inside_ of variables, this may/not may guide us to errors on old bash versions

We can not simple add a leading <semicolon> because predefined trailing <semicolon> carrying us to error-prone double <semicolon> case
So KISS (but not for all cases) is to add leading <semicolon> when last predefined character is _not_ a <semicolon>

this KISS can be broken by prefined trailing <space> or escaped <semicolon>

[egmont]

At this point I have absolutely no clue what you're trying to say.

KISS is not KISSB (keep it simple, stupid and buggy). As long as there is a solution that contains no known bugs (and there is), all solutions that contain a known bug are out of question.

I don't care about buzzwords. I care about robust solutions what won't have to be reworked over and over again as it's still not working for some people.

[mooffie]

(Sorry for not chiming in earlier: I was not aware there was activity here (I only recently learned of mc-bugs @ groups.google, but I forgot to visit it).)

Replying to egmont:

With all these subtle tricky cases, I cast my vote at the first approach with the embedded newline.

You mentioned at least three "tricky cases" (and solutions) I was oblivious to, so you've won me over: we should pick the newline solution. Considering our situation (lack of dev manpower; inadequate feedback from users), we don't have the "luxury" of entertaining a solution we don't know is always 100% correct.

Additionally: the urgency of this issue (see my next comment) should override sentiments about echo's confusing output, I believe. We can always revisit this decision in the future, if we get real indications (in contrast to guesses) that many people get confused or at all care.

As long as there is a solution that contains no known bugs (and there is), all solutions that [...]
[...] I care about robust solutions what won't have to be reworked over and over again as it's still not working for some people.

Agreed.

[mooffie]

Last week I accidentally stumbled upon two users who got hit by this trailing-semicolon bug:

• ​A user who has PROMPT_COMMAND="update_terminal_cwd; $PROMPT_COMMAND" in his /etc/bashrc. It seems that this line ​is, or was, the standard in OS X.

• A user who had export PROMPT_COMMAND="history -a; history -c; history -r; $PROMPT_COMMAND" somewhere. He recognized this as the cause of his ​I cannot use a sub-shell problem. (He reported this to me in a private email; that's how I know.)

Interestingly, both are OS X users.

So I hereby change the priority to "major". It seems that many users fall victim to this bug but we aren't aware of it because they don't report the problem.

[mooffie]

Replying to and:

this KISS can be broken by prefined trailing <space> or escaped <semicolon>

There's a trailing <space> in the two cases in my previous comment.

(I think it'd be nice if you changed your nick. Addressing an "and" can cause confusion. It's even more confusing than echo's output!)

[zaytsev]

Sounds like there is no consensus on this issue :-( probably not for 4.8.15 then, as I'd need at least a day to attentively read your discussion, check the code and take sides. D'oh...

[egmont]

Hi,

Could you please apply the first patch then, and forget that any of the followup discussion ever happened?

It's still better than no patch at all. We can revise later and change to the other one.

As mooffie pointed out, this bug does indeed cause problem to quite a few people.

I wouldn't care if we had frequent releases, but given the current tendency the next release is going to be in another at least half a year if not more. Why wait that long? It's basically a one-character change, from a semicolon to a newline.

[ossi]

i endorse egmont's original patch.

note that this is a command that is piped directly to bash, so no-one will ever see it (as long as they have HISTCONTROL=ignorespace). therefore it makes no sense to beautify it, like avoiding the literal linebreak in the command.

i also don't buy that the embedded linebreak in the variable is a problem of any kind. bash's 'set' will automatically display it as $'\n' (compare: $IFS). and echo'ing it is really no use case to be concerned about.

also, going through indirections would only make things *harder* to follow.

[zaytsev]

+echo 'Sources not indented'
Sources not indented
+git ls-files --modified
src/subshell.c

[slavazanko]

Created branch 3534_fix_prompt_command (initial changeset:5d1944ee824ebbc1fbb428a76a262d8979e262be)

And the branch has been merged into master:

git log --pretty=oneline 7c15794..5d1944e

[slavazanko]

​https://www.midnight-commander.org/wiki/NEWS-4.8.15?action=diff&version=31&old_version=30

[slavazanko]

Sources not indented

fixed

[ossi]

well, thanks for ignoring pretty much everything i said and committing the totally overengineered, cryptic solution. ;)

[and]

yep, overengineered solution.
majority is for '\n' solution, so please take first patch suggestion.

[slavazanko]

Ops, sorry, guys, my fail :(

First patch was applied:

git log --pretty=oneline 2ac9250..aa3ec5b