Indeed it doesn't make too much sense to replace, especially next to the zsh version which appends.

A bit more repetative (spelling the var name 3 times) but gives nicer formatted result:
PROMPT_COMMAND="${PROMPT_COMMAND}${PROMPT_COMMAND:+; }__vte_prompt_command"

If previous value is "foobar", new value becomes "foobar; __vte_prompt_command".

If previous value is "" or unset, new value becomes "__vte_prompt_command".

*If* we have to do this (which I don't like), should use ${PROMPT_COMMAND:+$PROMPT_COMMAND;}

Similar: bug 743073

Seeing the zsh version, I'm pretty sure that if bash supported an array of prompt commads, we'd also append.  That's the behavior that makes sense, overwriting doesn't.

The way to achieve this is indeed quite ugly in bash, but that shouldn't be a reason not to do this.

Forgot to say: come here via https://bugzilla.redhat.com/show_bug.cgi?id=1183192

I proposed that in https://bugzilla.redhat.com/show_bug.cgi?id=924275#c13 but the maintainer didn't like it. Trying bash upstream seems even more hopeless.

Sure those would be way better solutions.  But if RH/bash/whatever don't like the idea, why not do the best ourselves that we can do?

Just by changing that 1 line to append rather than replace, it won't be any harder to refactor later to a proper solution :)

(In reply to comment #2)
> should use ${PROMPT_COMMAND:+$PROMPT_COMMAND;}

This was my first version but then I slightly preferred the other :) I don't care too much, so I'll go with this one.

Fixed.

As an additional complication, recently fedora has altered its default /etc/bashrc to set PROMPT_COMMAND=__vte_prompt_command. This will only happen for non-login shells for reasons of ordering, but it will mean that now with this change to vte.sh, PROMPT_COMMAND will contain __vte_prompt_command twice: first by the default /etc/bashrc script, then appended by the vte.sh script.

(The default scripts are in here: https://fedorahosted.org/releases/s/e/setup/setup-2.9.4.tar.bz2 )

(In reply to comment #8)
> PROMPT_COMMAND will contain __vte_prompt_command twice:

This is ugly, but harmless.  Still way better than harmfully wiping out any previous value that might be there.

I can't recall how many times I've had duplicate entries in my PATH under various distros due to their stupid setup, and it never caused a problem.  Similar story.

If it really bothers you, I can extend the script to append it only if it's not yet there.  (And then I'd do the same for zsh, of course.)

(In reply to comment #10)
> If it really bothers you, I can extend the script to append it only if it's not
> yet there.  

Yes, please do.

Not sure if I'll have time before today's release -- feel free to revert temporarily if the current state really bothers you.

Created attachment 295376 [details] [review]
Fix v2

Here's a version that only appends the value if it's not already present there.

Strictly speaking, for zsh I should look for exact match rather than substring.  However, that leads to either a very complicated code, or zsh-specific syntax which we cannot allow if the same script is sourced by bash.  Also, it'd need to use a temporary variable (or a temporary function which can declare that variable as local), both are ugly.

So I think substring match is good enough.

I think it would be nice to submit this patch together with bug 743073's change, so that downstream distros have to adapt only once.  If that's in the next cycle, I'm fine with it.

*** Bug 749615 has been marked as a duplicate of this bug. ***

Appending a semicolon and then the new command is not perfect. It's legal to terminate a command with a semicolon, but it's a syntax error to have two semicolons next to each other (even with whitespaces in between). With the "Fix v2" patch, if the original PROMPT_COMMAND ends in a semicolon, we break it.

Possible solutions:

- Strip off trailing whitespaces and semicolon (really cumbersome);

- Use newline as separator;

- Prepend instead of append (it's a syntax error to begin with a semicolon, so if this one would result in two semicolons next to each other then the original PROMPT_COMMAND was broken to begin with);

- Prepend and use newline as separator; being the most robust because we perform whatever we want to perform even if the original PROMPT_COMMAND had a syntax error, perhaps an unterminated quote, a leading semicolon, whatever.

Another solution that could work safely, I haven't verified yet, is something like

PROMPT_COMMAND_VTESAVE="$PROMPT_COMMAND"
PROMPT_COMMAND='eval "$PROMPT_COMMAND_VTESAVE"; __vte_prompt_command'

(See also http://www.midnight-commander.org/ticket/3534#comment:10)

But so far prepending seems to be the cleanest to me.

Dear GNOME developers,

You are _breaking_ system ENV variable, please fix it!

Something like following should work and is suggested by autojump developer [1].

--- /etc/profile.d/vte.sh	2016-07-01 19:52:13.000000000 +0200
+++ /etc/profile.d/vte.sh	2016-07-19 17:48:41.665043681 +0200
@@ -59,7 +59,7 @@ 
 
 case "$TERM" in
   xterm*|vte*)
-    [ -n "$BASH_VERSION" ] && PROMPT_COMMAND="__vte_prompt_command"
+    [ -n "$BASH_VERSION" ] && PROMPT_COMMAND="${PROMPT_COMMAND:+$PROMPT_COMMAND; }__vte_prompt_command"
     [ -n "$ZSH_VERSION"  ] && precmd_functions+=(__vte_osc7)
     ;;
 esac


[1] https://github.com/wting/autojump/blob/master/docs/body.md

About: vte-profile-0.48.4-1.fc26.x86_64

I've been patching this script across many machines multiplied by countless times when system was upgraded to a newer version.

Is this even necessary to run `__vte_prompt_command` when user _already_ made conscious effort to decide its own `PROMPT_COMMAND`?

Why not completely _avoid_ overriding `PROMPT_COMMAND` if it is set?

Why not simply document somewhere that _only_ in cases when user sets its own `PROMPT_COMMAND`, it should also call `__vte_prompt_command` for "some" reasons?

Perhaps leaving user's custom `PROMPT_COMMAND` alone is the most adequate solution now when any better decision is still in the making (rather than keeping this bug around for 4 years since "2013-07-26"). Sometimes better may never come.

What is so critical in `__vte_prompt_command` anyway?

(In reply to Alexey Pakseykin from comment #18)

> What is so critical in `__vte_prompt_command` anyway?

Seems to me that you keep patching vte.sh without understanding what it actually does?? Follow the links from this bugreport to get to the answer.

This attitude is exactly why I wouldn't want to leave alone PROMPT_COMMAND for users who set it. It's not feasible for folks who decide to set PROMPT_COMMAND (based on their findings in bash's manual or some snippet from some forum) to require to be familiar with VTE's requirements and adjust accordingly. It's VTE's job to do it. Plus, I don't want to see tons of bugreports (here) and forum questions (elsewhere) about g-t's tabs not inheriting the current directory.

> when any better decision is still in the making

The change we're about to make will require distributions to adapt their scripts. Distributions are really (like, reaaalllly) reluctant to make such changes. We can't piss them off by making multiple subsequent changes. Whenever we fix it, we need to fix in a way that's pretty likely to be good enough for everyone (including you, of course) for many years to come. And yup, distros will be pissed off even by having to adjust their scripts once.

I know this set of bugs have been sitting here idle for 2-3 years now. I never had the motivation to finish the work, and I can't even remember if we had a consensus on what should be implemented; whereas on the other hand I truly wish this issue was properly resolved finally. Until then, I believe not touching the script and not messing with distributions is the least harm we can do.

Hi,
https://github.com/GNOME/vte/commit/b307d45e8b2dd27a3881852c29f4a6f0443d5348

In my opinion that commit should have never got into the code, let alone into distributions. What should had to be done is to work with upstream (bash, zsh) and implement this properly.

What you could do now as a workaround is to append your commands to the variable. If users are knowledgeable enough to modify system environment variables, then users are knowledgeable enough to fix broken g-t (meaning gnome-terminal?).

Me personally could not care less for stuff you've mentioned, what I care about is the overwritten system environment variable and the attitude towards it like it is not a big deal.

(In reply to srakitnican from comment #20)

> What should had to be done is to work with upstream
> (bash, zsh) and implement this properly.

zsh supports an array here and we use that (precmd_functions).

As far as I recall, the issue has been brought up with bash developers and they refused to implement it.

Also distributions are quire reluctant to introduce /etc/bashrc.d which we would need here.

It's not easy to move forward where you'd ideally need buy-in from N+1 parties (bash and N distributions) and none of them cares.

Any help is highly appreaciated here!

> What you could do now as a workaround is to append your commands to the
> variable.

As far as I remember that's the direction we started to go, the work just never got finished.

> If users are knowledgeable enough to modify system environment
> variables, then users are knowledgeable enough to fix broken g-t

IMO it's not being "knowledgeable" that matters. It's not okay if a user who wants to do X and looks up forums, documentation etc. and does X, breaks in turn a pretty much independent Y which (s)he doesn't care about, didn't want to touch, and now has to spend time to look up, learn more about Y and manually fix that too (hoping that that won't break Z...).

> g-t (meaning gnome-terminal?).

Yup, sorry for the abbreviation.

> and the attitude towards it like it is not a big deal.

I'm sorry if that's how it came across, I never meant that and never intended to say that.

Thanks for the prompt response!
Let's sort this out.

> Seems to me that you keep patching vte.sh without understanding what it actually does??

Not quite. I did look at `__vte_prompt_command` and it does nothing important for me.

Why does it overwrite mine? This was the missing understanding.
Asking it here was my last resort.

The rest is very clear. The `PROMPT_COMMAND` envvar is _my custom user settings_ for my shell - it is supposed to be untouchable if a user sets it.

> It's not feasible for folks who decide to set PROMPT_COMMAND (based on their findings in bash's manual or some snippet from some forum) to require to be familiar with VTE's requirements and adjust accordingly. It's VTE's job to do it. Plus, I don't want to see tons of bug reports (here) and forum questions (elsewhere) about g-t's tabs not inheriting the current directory.

I disagree. VTE's job is to stay out of the way when a user sets its preferences.

If some folks are already "adventurous" enough to mess with _system-wide_ defaults (`PROMPT_COMMAND` in this case), they are in risk to break things on a substantially larger scale anyway. Let them search for "VTE's requirements and adjust accordingly" instead if they actually use and break it themselves.

Whom are we trying to protect? Unreliable minorities?

Solution "override only when `PROMPT_COMMAND` is not set" would totally work:
- [A] for all innocent users who haven't messed up their settings themselves;
- [B] for those innocent who set their own VTE-independent defaults;
- [C] for those who read "VTE's requirements and adjust accordingly".

More importantly, such solution will work right now... before we all get old and die searching for perfection.

I can keep on patching this bug in script forever automatically.
I'm just trying to save time for the majority.

== ONE MORE SOLUTION ==

If "override only when `PROMPT_COMMAND` is not set" is not good enough, another solution could be an envvar like `VTE_DISABLE_PROMPT_COMMAND`.

If set, `vte.sh` does not modify `PROMPT_COMMAND` anyhow.

The default is not set.

This would still require me setting `VTE_DISABLE_PROMPT_COMMAND` (together with `PROMPT_COMMAND`), but system upgrades won't affect it.

======

I even thought of uninstalling the package, but it sits deep in the dependencies for lots of useful stuff.

(In reply to Alexey Pakseykin from comment #22)

> Why does it overwrite mine? This was the missing understanding.

It's indeed bad that it overwrites PROMPT_COMMAND. I agree it's a bug and should be fixed. There's not much point asking "why" that bug is there, it's just simply there.

However:

- Is there anything that prevents you from setting your preferred PROMPT_COMMAND *after* vte-2.91.sh does its stuff, rather than before? (I'm asking seriously.)

- Would prepending or appending to PROMPT_COMMAND, which is the current state of the patches as far as I recall, cause any problems to you?

> The rest is very clear. The `PROMPT_COMMAND` envvar is _my custom user
> settings_ for my shell - it is supposed to be untouchable if a user sets it.

I don't agree with this. Why do you insist on the exact value to be left untouched? Would you say it's also the case with $PATH and some similar variables, and absolutely no 3rd party app ever should be allowed to append their stuff there once you touch it?

I absolutely buy and agree that VTE shouldn't tamper with the _functionality_ that you define in your PROMPT_COMMAND. But to meet that goal, prepending/appending should still be okay, shouldn't it?

I still don't buy your argument that whoever decides to put something in PROMPT_COMMAND should then immediately study and understand the entire story of who else puts what there and why and what are the consequences of those no longer being there.

By the way, I'm aware of at least one terminal-based app, unrelated to VTE, that modifies PROMPT_COMMAND (appends to that). I'm deliberately not saying what it is because I don't want a bugreport flying in about it. (It's not hard to figure out what it is if you really care, I'm also active in that project. But unless the _functionality_ is broken, modifying the value is not a bug.)

For the bigger picture (and maybe another sibling bug would be a better place for this comment) another idea occurred to me today.

The script that we ship under /etc/profile.d basically only defines functions and env variables. In bash, functions can also be exported (technically they become env variables).

What if we removed this script, and hardwired into VTE to set up these env variables (functions) in advance for bash? The shell would launch up with __vte_osc7, PROMPT_COMMAND and others already being there.

With that approach, VTE would be the very first in the chain doing some stuff there, everyone else would follow. Probably not the best overall design, but definitely the one where it's always someone else to blame, not us :-D

Biggest advantage, seriously: We'd no longer rely on /etc/bashrc.d which distros don't have. Even non-login shells would automatically have the required stuff for everyone.

Some disadvantages:

Apparently bash-only, doesn't work with zsh.

The interface seems to be undocumented, subject to change at any time (as it did change after Shellshock).

Is there any point continuing thinking in this direction?

If you need to know the working directory of the running shell when you open a new tab, perhaps you can look at /proc/PID/cwd and/or a BSD equivalent?

Of course, this isn't so simple if the current shell isn't the immediate child, but perhaps there is a way to find the right PID to use in this case, too.

It used to be this way, and then changed for good reasons (see e.g. bug 675987). We're not going to revert.

(In reply to Egmont Koblinger from comment #23)

> Why do you insist on the exact value to be left untouched?

Would I insist if this was not an open discussion for years?

I simply assume that we are beyond accepting that fact.

Comment #22 carefully weights why not to touch it _at least when it is already set_. How was that?

> Would you say it's also the case with $PATH and some similar
> variables, and absolutely no 3rd party app ever should be allowed to append
> their stuff there once you touch it?

1. `PATH` with colon `:` has no issues like `PROMPT_COMMAND` with semi-colon `;` explained in comment #15.
2. Most system packages do not mess with _system-wide_ `PATH` at all (especially _deeply_ _rooted_ once which cannot be uninstalled). Instead, their executables are installed into predefined directories listed in `PATH`. There are irrelevant _local_ `PATH` changes in some launch scripts - this never leaks into _system-wide_ `PATH`. Path items and their order in the default _system-wide_ `PATH` are almost standard (chosen with great care to avoid conflicts).


So, if a standard package overrides default _system-wide_ `PATH` completely, it is undoubtedly reverted as a bug.

(In reply to Alexey Pakseykin from comment #27)

> > Why do you insist on the exact value to be left untouched?
> 
> Would I insist if this was not an open discussion for years?

I don't know; how shall I know whether you'd insist if you joined this conversation earlier? How is it any relevant whether we had capacity to work on this issue recently or not?

> I simply assume that we are beyond accepting that fact.

What fact??

> Comment #22 carefully weights why not to touch it _at least when it is
> already set_. How was that?

No, comment 22 pragmatically states that it should be untouched without evaluating the possibility of appending to it, just because.

> 1. `PATH` with colon `:` has no issues like `PROMPT_COMMAND` with semi-colon
> `;` explained in comment #15.

And as said there, PROMPT_COMMAND doesn't have such an issue either if we go for '\n' as the separator. So absolutely no technical problem here.

> 2. Most system packages do not mess with _system-wide_ `PATH` at all

I asked about _3rd_ party apps. Like, you know, the ones that don't come from your distribution's packages, and install typically into /opt/name_of_that_software and add that to $PATH. This is not what you answered (sorry if the question wasn't clear).

I'm sorry but this thread smells like it's just about to turn into a flamewar. At least, your recent comment added absolutely no value to the discussion in my eyes. If you keep commenting, please provide clear and objective answers to my questions, in particular the two bulleted questions from comment 23. Thanks!

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vte/-/issues/2010.