After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 711059 - Notifications for long-running commands
Notifications for long-running commands
Status: RESOLVED OBSOLETE
Product: gnome-terminal
Classification: Core
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: GNOME Terminal Maintainers
GNOME Terminal Maintainers
: 738267 (view as bug list)
Depends on: 711060
Blocks: 745626
 
 
Reported: 2013-10-29 09:25 UTC by Allan Day
Modified: 2021-06-10 20:36 UTC
See Also:
GNOME target: ---
GNOME version: 3.11/3.12


Attachments
emulation: Add sequences and signals for desktop notification (9.17 KB, patch)
2015-01-28 12:15 UTC, Debarshi Ray
none Details | Review
Support desktop notifications from OSC 777 (6.11 KB, patch)
2015-01-28 12:17 UTC, Debarshi Ray
none Details | Review
Make notifications based on org.gtk.Notification work (8.58 KB, patch)
2015-01-28 12:18 UTC, Debarshi Ray
none Details | Review
Support desktop notifications from OSC 777 (10.55 KB, patch)
2015-01-28 18:06 UTC, Debarshi Ray
none Details | Review
Sprinkle debug messages for notifications (2.79 KB, patch)
2015-01-29 12:05 UTC, Debarshi Ray
none Details | Review
vte.sh: Emit OSC 777 from PROMPT_COMMAND (818 bytes, patch)
2015-01-29 12:11 UTC, Debarshi Ray
none Details | Review
vte.sh: Emit OSC 777 from PROMPT_COMMAND (870 bytes, patch)
2015-01-30 17:18 UTC, Debarshi Ray
none Details | Review
Support desktop notifications from OSC 777 (10.80 KB, patch)
2015-01-30 17:41 UTC, Debarshi Ray
none Details | Review
vte.sh: Emit OSC 777 from PROMPT_COMMAND (913 bytes, patch)
2015-02-03 12:23 UTC, Debarshi Ray
none Details | Review
Support desktop notifications from OSC 777 (10.28 KB, patch)
2015-02-05 13:43 UTC, Debarshi Ray
none Details | Review
Support desktop notifications from OSC 777 (11.52 KB, patch)
2015-04-15 10:04 UTC, Debarshi Ray
none Details | Review
[gnome-terminal] Notify when a long-running foreground process terminates (11.02 KB, patch)
2018-05-10 17:25 UTC, Debarshi Ray
none Details | Review

Description Allan Day 2013-10-29 09:25:25 UTC
If I start a command that will take a long time to complete, it would be really useful to get a notification when it has completed. Also, it would be fantastic if I was notified if it halts for any reason.

We shouldn't show notifications for focused windows though.
Comment 1 Florian Müllner 2013-10-29 11:52:33 UTC
(In reply to comment #0)
> We shouldn't show notifications for focused windows though.

Windows or tabs?
Comment 2 Allan Day 2013-10-29 12:02:51 UTC
(In reply to comment #1)
...
> Windows or tabs?

A tab should be able to communicate a state change if you are looking at the window it belongs to - so windows.
Comment 3 Christian Persch 2013-11-13 20:33:36 UTC
Related to bug 73273, bug 132173, bug 557593 and dups thereof.
Comment 4 Debarshi Ray 2014-10-10 04:45:41 UTC
*** Bug 738267 has been marked as a duplicate of this bug. ***
Comment 5 Debarshi Ray 2014-11-05 13:18:58 UTC
Since we are not going to notify for the current focused window, I think we can hook into PROMPT_COMMAND like we do for setting the title. The first part is important because it means we don't have to bother about short-lived commands.
Comment 6 Debarshi Ray 2015-01-28 12:15:28 UTC
Created attachment 295652 [details] [review]
emulation: Add sequences and signals for desktop notification

This adds OSC 777 to vte, based on Enlightenment's Terminology:
https://phab.enlightenment.org/T1765
Comment 7 Debarshi Ray 2015-01-28 12:17:55 UTC
Created attachment 295653 [details] [review]
Support desktop notifications from OSC 777

We need to find a way to notify for tabs in the active window.
Comment 8 Debarshi Ray 2015-01-28 12:18:25 UTC
Created attachment 295654 [details] [review]
Make notifications based on org.gtk.Notification work
Comment 9 Christian Persch 2015-01-28 17:40:49 UTC
This seems to have gone from 'notification of long-running commands' to 'terminal apps should be allowed and able to show notifications in gnome-shell', somehow‽ I thought this would add some way for the terminal app to (via escape sequence) show its busy/idle/done status, and then gnome-terminal would do the notification (and I didn't imagine using gnome-shell notifications).

Also, to prevent a repeat of bug 743185, any new escape sequences need to be coordinated with xterm upstream.

Since there is no shortage of OSC sequences, why try to multiplex everything through 777 (why that number?) with a command argument ("notify") ?
Comment 10 Christian Persch 2015-01-28 17:44:49 UTC
Comment on attachment 295654 [details] [review]
Make notifications based on org.gtk.Notification work

When doing renames, should use git mv, not do an remove/add.

Has the issue with backward compatibility on desktop file rename been solved?

Also, gnome-terminal-server is not an activatable gapplication, so why the need to rename the desktop file?
Comment 11 Allan Day 2015-01-28 17:57:55 UTC
(In reply to comment #9)
> This seems to have gone from 'notification of long-running commands' to
> 'terminal apps should be allowed and able to show notifications in
> gnome-shell', somehow‽

I thought that "notification" was commonly understood as shell notifications (or libnotify/GNotifications). Apologies if that wasn't clear.
Comment 12 Debarshi Ray 2015-01-28 18:02:22 UTC
(In reply to comment #9)
> This seems to have gone from 'notification of long-running commands' to
> 'terminal apps should be allowed and able to show notifications in
> gnome-shell', somehow‽ I thought this would add some way for the terminal app
> to (via escape sequence) show its busy/idle/done status, and then
> gnome-terminal would do the notification (and I didn't imagine using
> gnome-shell notifications).

We use desktop wide notifications only when the terminal sending it is not part of the active window. For those terminals that are part of the active window (think tabs), it is gnome-terminal who handles the notification. Currently an icon added to the tab label, but I think that can be improved.

Regarding, busy/idle/done status, isn't it up to the terminal applications themselves to emit the escape sequence?

> Also, to prevent a repeat of bug 743185, any new escape sequences need to be
> coordinated with xterm upstream.
> 
> Since there is no shortage of OSC sequences, why try to multiplex everything
> through 777 (why that number?) with a command argument ("notify") ?

Egmont suggested OSC 777 because Enlightenment has adopted it. I have no clue why they use "notify" like that. I asked him the same the question. (see email discussion)
Comment 13 Debarshi Ray 2015-01-28 18:04:11 UTC
(In reply to comment #10)
> (From update of attachment 295654 [details] [review])
> When doing renames, should use git mv, not do an remove/add.

I did use git mv. Did I mess it up?

> Has the issue with backward compatibility on desktop file rename been solved?

Umm... which issue?

> Also, gnome-terminal-server is not an activatable gapplication, so why the need
> to rename the desktop file?

The name of the desktop file needs to match the application-id for org.gtk.Notifications to work.
Comment 14 Debarshi Ray 2015-01-28 18:06:02 UTC
Created attachment 295691 [details] [review]
Support desktop notifications from OSC 777
Comment 15 Allan Day 2015-01-29 10:14:30 UTC
(In reply to comment #9)
> This seems to have gone from 'notification of long-running commands' to
> 'terminal apps should be allowed and able to show notifications in
> gnome-shell', somehow‽ I thought this would add some way for the terminal app
> to (via escape sequence) show its busy/idle/done status, and then
> gnome-terminal would do the notification (and I didn't imagine using
> gnome-shell notifications).
...

A few more notes/comments about this:

The major goal for this bug, as far as I understand it, is to inform the user about changes in a terminal when they aren't looking directly at it. There are two possible scenarios here:

1. The terminal of interest is in an unfocused tab.
2. It is in an unfocused/hidden window.

For 1, the tab can inform the user that the terminal has changed. For 2, using libnotify/gnotification to generate a desktop notification is really the only way to inform the user. (Indeed, this is the primary purpose of these notifications - to tell the user about events in windows that they currently aren't using.)
Comment 16 Debarshi Ray 2015-01-29 12:05:14 UTC
Created attachment 295748 [details] [review]
Sprinkle debug messages for notifications
Comment 17 Debarshi Ray 2015-01-29 12:11:40 UTC
Created attachment 295749 [details] [review]
vte.sh: Emit OSC 777 from PROMPT_COMMAND

We should sanitize the arguments. Otherwise if the last command had a semi-colon in it then it would cause problems.
Comment 18 Debarshi Ray 2015-01-30 17:18:05 UTC
Created attachment 295814 [details] [review]
vte.sh: Emit OSC 777 from PROMPT_COMMAND
Comment 19 Debarshi Ray 2015-01-30 17:41:10 UTC
Created attachment 295816 [details] [review]
Support desktop notifications from OSC 777

Add a little tooltip for the in-tab notification icon.
Comment 20 Egmont Koblinger 2015-02-01 22:28:20 UTC
The OSC 777 originates from urxvt whereas it's the generic code to reach out to its plugin scripts which can execute whatever you want. It seems to be a standard there for the first word to somehow denote the kind of action. Somebody suggested that the word "notify" would be used for notification, but it's not part of urxvt, it's just one possible way to extend it. Then Terminology adopted and hardcoded this 777+"notify". I'm not sure if this is the right approach for us. I have no further info on what urxvt is doing on 777, we should study it.

Escaping special chars like ';' (comment 17) is indeed an important issue, and it's crucial that all the terminal emulators here agree on something. Reusing the same escape sequence with different escaping syntax is a giant no-go. (To make it more complicated, based on previous personal experiences I'm not willing to talk to urxvt's maintainer, and can't recommend to anyone who prefers to keep their inner peace.)

-----

I still can't see what's the ultimate goal with this notification thing.

Is there planned to be a desktop-wide notification, like the one generated by the "notify-send" command? I would HATE to see this hooked up to the shell prompt by default, it would just be waaaaaay toooo annoying. As if Firefox notified me each time it finishes loading a non-active tab. I'm fine with an escape sequence that I can put into the end of really-long-running scripts to notify me, though. But if it's intended for long-running scripts only, I'd prefer to have variants to either notify me unconditionally, or notify me only if the tab is not active, based on my choice. Maybe 777+"notify" and 777+"notify-is-nonactive", perhaps even other variants. However, if this feature is not hooked up to the prompt then it'd have very limited use, it's not clear if it's worth it to implement it at all. After all, placing a notify-send command at the end of my script _almost_ takes me there. (Sure, it'd work over ssh etc. so it'd be cool, but probably only very few people would use it, and I'm happy to live without it.)

-----

Re comment 15: If we're about to ever behave differently on focused vs unfocused window, we're blocked by bug 677329 which seems to be very complicated and nobody eager to work on. Behaving differently on hidden vs. nonhidden window is IMO a definite no-go, you can't draw a clear borderline when a partially hidden window should behave as a hidden or as a nonhidden one, it'd cause unpredictable and hence confusing behavior. That leaves us with only one criterium at this moment: active vs. nonactive tab within the window.

-----

When bash starts up, with the current vte.sh it immediately emits a 777+notify with the latest entry in .bash_history that was executed by a previous bash. This is bad (and I can't see how to fix it), even if this notification doesn't actually appear on the UI most of the time. The newly started shell is usually (but not always) inside the active tab and then this faulty notification doesn't get shown, but sometimes it would appear.

-----

According to bug 704960 and bug 743073, I'll refactor vte.sh to only contain the necessary stuff for opening the new tab in the previous tab's working directory. This is the only feature that I believe everyone's happy with. Setting the window title is a user preference which doesn't belong here, and I believe neither does hooking up desktop notifications to the prompt. If a distro decides to set it up, they should do it, leaving an easy way for the user to opt out.

-----

I really don't like the current approach, as it only addresses a small subset of when someone might want to see a notification:

[+] A foreground command launched from bash/zsh finishes.
[-] A background task launched from bash/zsh prints something.
[-] A foreground command launched from mc finishes.
[-] mc finishes copying tons of files, or finishes with any time-consuming task.
[-] An IDE finishes with a time consuming task (e.g. compilation launched from emacs).
[-] An IM client (e.g. irssi) that's idle most of the time receives a message.
[-] The user ssh's to a remote machine and does any of the above there (incl. when foreground command from bash/zsh finishes).

Out of these example uses cases, the current approach only gives me a notification in the 1st, and not in any other of them. If you come up with more examples, they'll also fall in the negative category.

This makes me believe we're completely on the wrong track.

-----

What I'd prefer to see is what I'd been using in iTerm2 for many years. As far as I remember, it had 3 possible colors for the title:

- Default color1 for the active tab, and for tabs where nothing changed since I switched away;
- Color2 for inactive tabs where there was activity since I switched away, including some activity within the last 10 seconds (i.e. unseen content and keeps changing);
- Color3 for inactive tabs where there was activity since I switched away, but not in the last 10 seconds (i.e. unseen content, but idle).

This was implemented without any escape sequences, just by watching whether the contents changed. It was more than good enough, and presented useful notification for all the cases mentioned above. This approach is also a better fit to the world of terminals where it's just input and output bytes, not caring about the apps who send them or the semantics of what's happening inside.

This solution could be further improved by something similar to 777+notify, hooked up the shell prompt. It would have the semantics "the terminal almost sure will be idle until further user action", would eliminate the 10-second delay in switching from Color2 to Color3, and could also provide some more user info (the command that finished; although it's less useful on the tab and more useful for desktop notification which, again, I'd hate to see hooked up by default). That is, 777+notify would not be the core of notification, but rather a small optional improvement to that.

The core would be implicitly watching what's happening inside the terminal (which we can already do reliably and covers almost all use cases where I'd want to see notification), rather than expecting explicit instructions (which needs to be hooked up at all the desired places, but will probably only happen for bash/zsh prompt which is a very small subset).
Comment 21 Matthias Clasen 2015-02-01 23:31:45 UTC
> Out of these example uses cases, the current approach only gives me a
> notification in the 1st, and not in any other of them. If you come up with more
> examples, they'll also fall in the negative category.
>
> This makes me believe we're completely on the wrong track.

The goal for this effort is stated in the title of this bug:

Notify the user when a long-running command in a non-active tab finishes. There's two variations to this: if the command is running in a window that is not the active window, we need to generate a system-wide notification (since the window may be buried or on a different workspace, etc). If the command is running in a non-active tab in the currently active window, we can improve on this by avoiding the system-wide notification that would would that the users attention away from the window, and instead just show the notification inside the window.

This is pretty much exactly what Debarshi's vte and gnome-terminal branches achieve, taken together.

The way this is implemented (copying the escape sequence approach from other terminal implementation) was chosen simply because it is basically impossible to get any improvements past your and chpe's veto.

We are pretty much tired of this, and will carry these patches downstream in Fedora.

I think it is time that we have a discussion about finding a different way to evolve the terminal code base. It is clear that you and Christian are not interested in adding most of the things the gnome designers, me, Debarshi, and others want to see in GNOME's terminal application.
Comment 22 Egmont Koblinger 2015-02-02 00:18:35 UTC
(In reply to comment #21)

> The goal for this effort is stated in the title of this bug:

Sorry, I didn't mean the "goal", I stated in incorrectly. I meant to say I don't see how the complete user experience would look like.

> Notify the user when a long-running command in a non-active tab finishes.
> There's two variations to this: if the command is running in a window that is
> not the active window,

To have the concept of "active window", please help fix bug 677329.

> we need to generate a system-wide notification

Which is a user preference whether this is useful or terribly annoying, me belonging to the second group. That's why I'd prefer a solution where this might be a possibility, but is not forced on users. In my opinion, just coloring the tab titles is good enough, even if the given window is not visible. YMMV.

The initial comment also says "it would be fantastic if I was notified if it halts for any reason". No sign whatsoever of supporting this in the current design.

> The way this is implemented (copying the escape sequence approach from other
> terminal implementation) was chosen simply because it is basically impossible
> to get any improvements past your and chpe's veto.

I'm not vetoing, I'm constructively criticizing the approach because of the aforementioned issues, offering an alternative that I believe would be much better. I have previously written those in private mail to ChPe and Debarshi, and received no comments on them. I'm yet to hear why anyone thinks the currently proposed approach (which only handles a small subset of relevant cases) is good, because I disagree.

I'm not against the idea of notification at all, in fact I do support it, I just believe (and support with arguments) that we should take a completely different approach.

> It is clear that you and Christian are not
> interested in adding most of the things the gnome designers, me, Debarshi, and
> others want to see in GNOME's terminal application.

What makes you believe this? In this thread I've made 1 comment shortly after the thread went viral, explaining why I think it's the wrong approach and what I think the right approach would be. Does that really sound like not being interested??

What are those other "most of the things" you're referring to??

(One personal note to clarify: Even though I received git access and at some point someone made the word "developer" appear next to my name here in bugzilla, I consider myself a random occasional contributor. I've never made any commitment to the project. I'm not the main developer and not the one having the final word. I'm expressing my own opinion on the topic, not necessarily Christian's or anyone else's or the team's overall opinion.)
Comment 23 Egmont Koblinger 2015-02-02 00:40:04 UTC
(In reply to comment #21)

> The way this is implemented (copying the escape sequence approach from other
> terminal implementation) was chosen simply because it is basically impossible
> to get any improvements past your and chpe's veto.

According to my best knowledge it was chpe who recommended to Debarshi to use escape sequences. And it was me pointing them to terminology's then-brand-new 777+notify.

> We are pretty much tired of this, and will carry these patches downstream in
> Fedora.

I hope you realize that the patches in their current state (at least the git *wip* branch, that's what I tried) are nice proof of concepts, but not production ready (they're really "wip"). Debarshi pointed out an escaping problem. I pointed out another bug in vte.sh. The title that becomes bold never becomes normal again. There's a popup dialog instead of desktop notification. Probably there are more issues... Just an FYI before you decide to include in RedHat.
Comment 24 Egmont Koblinger 2015-02-02 01:06:22 UTC
One tricky but important technical detail with any in-tab-label notification:

Any approach that doesn't notify in the active tab (and we won't notify there because it's annoying and there's no tab label for a single tab) is subject to race condition if we take the user's perception into the game, which we have to.

It is always possible that the command finishes just a glimpse before you switch away to another tab.

In this case the prompt appearing (or any other action that triggers the notificaition) might
 - not even make it into a VTE display update cycle;
 - make it into a VTE update but not into X11 pixels (is it possible?);
 - make it to the video card, but not into visible pixels because of the monitor's physical refresh rate;
 - make it to actual pixels for such a short time that the user doesn't notice;
 - or if it's shown a bit longer, the user might be totally uncertain whether it flashed up for a very short time, making him switch back to check it, and then switch away as he intended, but then become uncertain again and repeat endlessly.

So, practically it becomes impossible to switch away from a tab, without risking that the command just finishes and it goes unnoticed.

Of course we can't expect the user to check "ps" in another tab to prevent this race.

This means that gnome-terminal should take care of this. When switching away from a tab, if there was a previous notification event in that tab in the last ~0.2 seconds we need to show it.

It's tricky to implement, but I see no excuse, we just have to do it.

(This is the kind of issue for me where getting it right makes software developmet an interesting and fun thing.)
Comment 25 Matthias Clasen 2015-02-02 03:07:25 UTC
(In reply to comment #22)

> To have the concept of "active window", please help fix bug 677329.

Not sure why you think that bug in any way blocks your ability to use the concept of 'active window'. We use that concept every time a window goes to or from backdrop, and every time we do or don't show a blinking cursor in entries. There may be bugs in tracking it, but it works well enough.

And the worst case if we get it wrong here is that you get a system notification instead of an in-app one, or vice versa...


> > we need to generate a system-wide notification
> 
> Which is a user preference whether this is useful or terribly annoying, me
> belonging to the second group. That's why I'd prefer a solution where this
> might be a possibility, but is not forced on users. In my opinion, just
> coloring the tab titles is good enough, even if the given window is not
> visible. YMMV.

The notification panel lets you disable system-wide notifications on a per-application basis. So yes, this is already a user preference.
Comment 26 Egmont Koblinger 2015-02-02 09:53:53 UTC
(In reply to comment #25)

> And the worst case if we get it wrong here is that you get a system
> notification instead of an in-app one, or vice versa...

It'd be the vice versa case, since for a non-active window we mistakenly think it's active. Which means that the user would expect to see a desktop notification, yet that notification would occasionally not arrive. Such an expected but not presented notification might easily cause tens of minutes wasted for the user, e.g. browsing the net instead of returning to work. It's not the same severity as a cursor mistakenly blinking.

I won't support adding a new feature with such a known issue, even if technically the bug is not in gnome-terminal itself.

(Totally independently of the gnome-terminal notification: not being able to reliably tell if the window is focused is a shame and that bug effects many other apps/users too, so IMO it should be fixed with higher-than-average priority anyway.)
Comment 27 Allan Day 2015-02-02 10:08:14 UTC
(In reply to comment #20)
...
> I still can't see what's the ultimate goal with this notification thing.

Comments are below. I hope they help to clear things up.

> Is there planned to be a desktop-wide notification, like the one generated by
> the "notify-send" command? 

"notify-send" shows a system notification, which is what you get through libnotify/gnotification. I think I explained how those are intended to be used in comment #15.

> I would HATE to see this hooked up to the shell
> prompt by default, it would just be waaaaaay toooo annoying. As if Firefox
> notified me each time it finishes loading a non-active tab.

The intention is to only show a notification for long-running commands. I've left the definition of that somewhat fuzzy for now, to give space for testing and fine-tuning. But I definitely agree that we will need to ensure that the notifications are only shown when needed, and don't become annoying.

...
> it's
> not clear if it's worth it to implement it at all. 
...

People run long-running commands all the time, and end up forgetting about them, or having to manually check if they've finished. It's frustrating and means that people waste a lot of time. Eliminating those situations would be a huge improvement, wouldn't it?

...
> Re comment 15: If we're about to ever behave differently on focused vs
> unfocused window, we're blocked by bug 677329 which seems to be very
> complicated and nobody eager to work on. Behaving differently on hidden vs.
> nonhidden window is IMO a definite no-go, you can't draw a clear borderline
> when a partially hidden window should behave as a hidden or as a nonhidden one,
> it'd cause unpredictable and hence confusing behavior. That leaves us with only
> one criterium at this moment: active vs. nonactive tab within the window.

I'll defer to Matthias's view here (comment #25). From a design point of view, the main thing to avoid is notifications being shown for the current tab.

...
> I really don't like the current approach, as it only addresses a small subset
> of when someone might want to see a notification:
> 
> [+] A foreground command launched from bash/zsh finishes.
> [-] A background task launched from bash/zsh prints something.
> [-] A foreground command launched from mc finishes.
> [-] mc finishes copying tons of files, or finishes with any time-consuming
> task.
> [-] An IDE finishes with a time consuming task (e.g. compilation launched from
> emacs).
> [-] An IM client (e.g. irssi) that's idle most of the time receives a message.
> [-] The user ssh's to a remote machine and does any of the above there (incl.
> when foreground command from bash/zsh finishes).
> 
> Out of these example uses cases, the current approach only gives me a
> notification in the 1st, and not in any other of them. If you come up with more
> examples, they'll also fall in the negative category.
> 
> This makes me believe we're completely on the wrong track.

I agree that it would be great if we could generate notifications for some of the other cases you've listed here. That said, producing them for the first one alone seems like positive step, and a lot of people would benefit from it.

> What I'd prefer to see is what I'd been using in iTerm2 for many years. As far
> as I remember, it had 3 possible colors for the title:
> 
> - Default color1 for the active tab, and for tabs where nothing changed since I
> switched away;
> - Color2 for inactive tabs where there was activity since I switched away,
> including some activity within the last 10 seconds (i.e. unseen content and
> keeps changing);
> - Color3 for inactive tabs where there was activity since I switched away, but
> not in the last 10 seconds (i.e. unseen content, but idle).

That sounds neat. The issue, of course, is that it doesn't notify the user if they aren't looking at the terminal window concerned, which is the really big win with the design I've been advocating for.
Comment 28 Debarshi Ray 2015-02-02 10:12:25 UTC
(In reply to comment #20)

> When bash starts up, with the current vte.sh it immediately emits a 777+notify
> with the latest entry in .bash_history that was executed by a previous bash.
> This is bad (and I can't see how to fix it), even if this notification doesn't
> actually appear on the UI most of the time. The newly started shell is usually
> (but not always) inside the active tab and then this faulty notification
> doesn't get shown, but sometimes it would appear.

Why do you say "doesn't actually appear on the UI most of the time" and "sometimes it would appear" ? It is never visible to the user when a new terminal window or tab is opened.

I think the scenario that you are describing is where the user quickly spawns a bunch of windows on slower machine and some of the windows are already inactive by the time the shell shows the prompt. This is averted by always checking whether the shell's prompt was shown at least once before doing anything with notifications. Currently this check is done by waiting for the first window-title-changed from VteTerminal because that is emitted every time the shell's prompt is shown.
 
> According to bug 704960 and bug 743073, I'll refactor vte.sh to only contain
> the necessary stuff for opening the new tab in the previous tab's working
> directory. This is the only feature that I believe everyone's happy with.
> Setting the window title is a user preference which doesn't belong here, and I
> believe neither does hooking up desktop notifications to the prompt. If a
> distro decides to set it up, they should do it, leaving an easy way for the
> user to opt out.

The easy way for the user to opt out would be a UI element in gnome-terminal's preferences. I don't expect users to mess with their environment variables for this.
Comment 29 Debarshi Ray 2015-02-02 10:18:48 UTC
(In reply to comment #22)
> I'm not vetoing, I'm constructively criticizing the approach because of the
> aforementioned issues, offering an alternative that I believe would be much
> better. I have previously written those in private mail to ChPe and Debarshi,
> and received no comments on them. I'm yet to hear why anyone thinks the
> currently proposed approach (which only handles a small subset of relevant
> cases) is good, because I disagree.

One usually thinks of technical solutions after knowing what the end goal is. Since you were advocating for a slightly different goal, my response to your email was to dump your thoughts on this bug so that others (eg., Allan) could participate. If the end goal is substantially different from just notifying when a long running command finishes, then obviously the technical solution will also be different.

Unfortunately, there was no activity on this bug since then and since the GNOME 3.16 deadlines are fast approaching, I went ahead with the initial plan.
Comment 30 Egmont Koblinger 2015-02-02 10:24:23 UTC
(In reply to comment #27)

> The intention is to only show a notification for long-running commands.

(In reply to comment #28)

> The easy way for the user to opt out would be a UI element in gnome-terminal's
> preferences.

These are the kinds of issues I was referring to when I said I don't see the overall user experience designed yet.

I guess it's time to take a step backwards and _design_ the feature (from the user's perspective) first.

I'm against designing and implementing half of the story and being happy that it's already an improvement, without having a clue how the other half will look like. It's technically a complicated story, chances are that in order for everything to play well together we'd need to later change something in the existing components. I say let's design and implement everything we'd like to have at once.

I have a feeling we'll need both what Allen+Debarshi went for, and what I had in mind.
Comment 31 Debarshi Ray 2015-02-02 10:25:06 UTC
(In reply to comment #23)
> (In reply to comment #21)
> 
> > The way this is implemented (copying the escape sequence approach from other
> > terminal implementation) was chosen simply because it is basically impossible
> > to get any improvements past your and chpe's veto.
> 
> According to my best knowledge it was chpe who recommended to Debarshi to use
> escape sequences. And it was me pointing them to terminology's then-brand-new
> 777+notify.

This is true.

> > We are pretty much tired of this, and will carry these patches downstream in
> > Fedora.
> 
> I hope you realize that the patches in their current state (at least the git
> *wip* branch, that's what I tried) are nice proof of concepts, but not
> production ready (they're really "wip"). Debarshi pointed out an escaping
> problem.

What escaping problem? You mean taking care of semi-colons? That is already done.

> I pointed out another bug in vte.sh. The title that becomes bold never
> becomes normal again.

You mean the call to terminal_tab_label_set_bold? As far as I can see, it has no affect on Adwaita, which is what I wanted to investigate. It sounds like it does have an effect on your system.

Anyway, these are just cosmetic things, which are easily fixed.

> There's a popup dialog instead of desktop notification.

Popup dialog?

> Probably there are more issues... Just an FYI before you decide to include in
> RedHat.

Red Hat
Comment 32 Egmont Koblinger 2015-02-02 10:31:20 UTC
(In reply to comment #28)

> Why do you say "doesn't actually appear on the UI most of the time" and
> "sometimes it would appear" ? It is never visible to the user when a new
> terminal window or tab is opened.
> 
> I think the scenario that you are describing is where the user quickly spawns a
> bunch of windows on slower machine and some of the windows are already inactive
> by the time the shell shows the prompt.

Maybe. Or ssh (password-less) to a remote machine which takes some time to build up the connection and I switch away in the mean time. Or a manual "sleep 5; bash" for whatever purposes.

Emitting a faulty signal, and trying to handle in g-t that the first one is presumable faulty is a terrible design.

> This is averted by always checking
> whether the shell's prompt was shown at least once before doing anything with
> notifications. Currently this check is done by waiting for the first
> window-title-changed from VteTerminal because that is emitted every time the
> shell's prompt is shown.

Please leave the title out of this business, see the related bugs I've linked above. Please don't make it any harder for those who'd prefer vte not to set the title.

Ignoring one kind of event until another totally irrelevant kind of event is encountered at least once, just because it happens to work around something with the default wiring of PROMPT_COMMAND -- thanks but I don't want to see such a "design" in g-t. Please figure out how to emit the correct notifications only. One way could be to check the history's position on startup, and only emit the signal if there's new entry in the history since then. Another, cleaner method would of course be to implement postexec in bash.
Comment 33 Egmont Koblinger 2015-02-02 10:35:21 UTC
(In reply to comment #31)

> What escaping problem? You mean taking care of semi-colons?

Yes.

> That is already done.

In a way that's compatible with urxvt and terminology?

> > I pointed out another bug in vte.sh. The title that becomes bold never
> > becomes normal again.
> 
> You mean the call to terminal_tab_label_set_bold? As far as I can see, it has
> no affect on Adwaita, which is what I wanted to investigate. It sounds like it
> does have an effect on your system.
> 
> Anyway, these are just cosmetic things, which are easily fixed.

I just looked at the user experience, not the code. I'm using Ubuntu Utopic with its default Unity desktop and default Ambiance theme, that's where I saw it. No clue about the code.

> Popup dialog?

Yup. Ubuntu, Unity. Haven't tried with gnome-shell.
Comment 34 Lars Karlitski 2015-02-02 12:52:55 UTC
> > Popup dialog?
> 
> Yup. Ubuntu, Unity. Haven't tried with gnome-shell.

Indeed. Unity's notifications don't support actions. If an app sends a notifications with actions, Unity falls back to showing a popup dialog.

This patch uses GNotification correctly. Please don't block it because of that. We'll address this issue elsewhere.
Comment 35 Egmont Koblinger 2015-02-02 13:11:45 UTC
(In reply to comment #34)

> Indeed. Unity's notifications don't support actions. [...]
> We'll address this issue elsewhere.

Okay, but could we please have a pointer here to where this issue is tracked?
Comment 36 Egmont Koblinger 2015-02-02 13:31:44 UTC
(In reply to comment #28)

> The easy way for the user to opt out would be a UI element in gnome-terminal's
> preferences. I don't expect users to mess with their environment variables for
> this.

If we implement notifications, I'd like to use them, just not every time a non-active-tab's long-running command finishes. That is, I'd remove it from PS1/PROMPT_COMMAND but not disable in g-t for myself. (Also, as said before, I'd like to extend it to allow notification for active and/or focused tabs too.)

Also note that current script adds it if VTE_VERSION >= 3405, this should be changed to check for the version where this feature gets implemented.

To make this easier to remove from PROMPT_COMMAND, vte.sh should not unconditionally emit this sequence, but should factor it out to a shell function (e.g. __vte_command_finished_notify), so that I can redefine that function to a no-op after sourcing vte.sh. I'd like to see vte.sh designed in a way that the features it implements (osc7 for cwd, notification, and title which I'd like to nuke from here) are totally independent from each other, each checking for their own required version number, and each being easy to disable without effecting the rest.

This is a quite heavy refactoring, in the other two relevant bugs which I've linked the conclusion was that we'd defer it to the next cycle. But if notification will be implemented in this cycle, we'd need to do those too now otherwise it'll be a hell.

I'm not sure if we'll have time to do the notification properly in this cycle or not. I don't want to intentionally delay it, but I find it more important that we do it right, and if it takes more time then I personally wouldn't mind delaying it. As per comment 27, the original reporter Allan says that we'll need to do fine-tuning. There've been quite a few other issues pointed out. I honestly don't believe we can deliver this feature in good quality this cycle. I find it more reasonable to keep working on the wip branch, there's still a lot to fix here, also try to fix the missing focus in/out and the Unity notification issues in the mean time, and integrate to master in the early 3.17 days.
Comment 37 Sergey "Shnatsel" Davidoff 2015-02-02 15:09:42 UTC
Just FYI, this has been implemented in Pantheon Terminal for quite a while. This is how it looks: https://www.youtube.com/watch?v=WLhTmnifAro
I suggest you to just rip off that design :)

Since I didn't have the option to patch libvte, instead of escape sequences I've implemented it via a D-bus callback from shell prompt function in BASH and the relevant hook in ZSH. For BASH I just append the following line to $PROMPT_COMMAND:

dbus-send --type=method_call --session --dest=net.launchpad.pantheon-terminal /net/launchpad/pantheon_terminal org.pantheon.terminal.ProcessFinished string:$PANTHEON_TERMINAL_ID string:"$(history 1 | cut -c 8-)";

This doesn't traverse SSH, but otherwise seems to work fine with a bit of magic to suppress the notification on BASH startup on the terminal side.
Comment 38 Egmont Koblinger 2015-02-02 15:31:01 UTC
We saw Pantheon's implementation. chpe recommended to use escape sequences rather than off-channel (e.g. dbus) communication, and I couldn't agree more with this.

I don't know what's the "magic" used in Pantheon that suppresses the notification on bash startup, but I don't like any approach that suppresses it. That faulty notification shouldn't be sent in the first place. This can be achieved by running a one-shot "history 1" when sourcing vte.sh and remembering the sequence number in some variable, and then comparing the current sequence number against it.

Nitpicking: I don't like invoking an external command, whereas bash itself could easily strip off the first few characters. Also what if there are so many entries in the history that the columns start shifting further to the right? We should do something like "${var## *[0-9]* *}".

Back to Debarshi's semicolon: Again I don't like invoking an external command for this whereas bash could do this much faster internally. I don't like replacing them by spaces: We should be able to include any character in the notification. This could be achieved by using an escape sequence format where the message is terminated by BEL (this might not be 777 after all), or by escaping somehow. We should start by investigating whether that semicolon is necessary for 777 and if so what's urxvt's and ty's escaping strategy, to decide whether 777 is able to do this or not. If not, we should go with a custom sequence we make up.
Comment 39 Egmont Koblinger 2015-02-02 15:33:49 UTC
> "${var## *[0-9]* *}"

Oops I'm afraid I mixed up shell patterns with regexps. Anyway, you get the idea...
Comment 40 Egmont Koblinger 2015-02-02 16:06:09 UTC
I've just discovered bug 743867, which is also relevant for notifications.

You can enter unprintable characters to bash's command line (e.g. by pressing ^V first and then the desired character, e.g. ^G for BEL), these are saved literally in the history and printed back when "history" command is run. These can prematurely terminate the notification's escape sequence and cause further misbehavior. We should remove these unprintable characters (at least BEL and ESC).
Comment 41 Debarshi Ray 2015-02-02 18:16:39 UTC
(In reply to comment #38)

> I don't know what's the "magic" used in Pantheon that suppresses the
> notification on bash startup,

They check if the window ever received focus.

> but I don't like any approach that suppresses it.
> That faulty notification shouldn't be sent in the first place. This can be
> achieved by running a one-shot "history 1" when sourcing vte.sh and remembering
> the sequence number in some variable, and then comparing the current sequence
> number against it.

That may not work because the sequence number is not guaranteed to increase monotonically. Bash is often set up to ignore duplicate entries in the history. If the next thing typed by the user is the same as the last one in the history, then the logic will break.
Comment 42 Egmont Koblinger 2015-02-02 21:45:36 UTC
(In reply to comment #41)

> > I don't know what's the "magic" used in Pantheon that suppresses the
> > notification on bash startup,
> 
> They check if the window ever received focus.

I don't even remotely understand this. Newly opened windows automatically receive the focus by the wm. Newly opened tabs are opened in windows that have already received focus. I can't see how the "window ever received focus" would evaluate to FALSE.

Even if it does under some circumstances, it's again two totally unrelated issues connected somehow because they accidentally happen to be correlated in some setups.

This thread is starting to become the collection of ugliest bound-to-fail workarounds I've ever seen :(

> That may not work because the sequence number is not guaranteed to increase
> monotonically. Bash is often set up to ignore duplicate entries in the history.
> If the next thing typed by the user is the same as the last one in the history,
> then the logic will break.

This is true. We'd need access to what bash makes available in PS1 as '\#'.

There's another problem with job control. Execute "sleep 100", put it in background with ^Z (maybe "bg" too), bring it back to foreground with "fg". 100 seconds later you'll get notified that the command "fg" finished running. Not good either.

Seems that bash provides no reliable method to get the most recently finished foreground job's command line, or the fact that there's been none yet.

Instead of hackish workarounds, how about
- either accepting that we can't reliably get this info and design a notification system where we don't rely on it, e.g. just present a symbol in the tab label without further details;
- or patch bash (and zsh if needed) to make this info available, and perhaps even try to get it accepted upstream?
Comment 43 Debarshi Ray 2015-02-03 08:25:47 UTC
(In reply to comment #42)
> This thread is starting to become the collection of ugliest bound-to-fail
> workarounds I've ever seen :(

Can you please stop being so alarm and disrespectful? You have been behaving like this for a while. It is a severe obstacle to having any constructive discussion.
Comment 44 Debarshi Ray 2015-02-03 12:23:40 UTC
Created attachment 296011 [details] [review]
vte.sh: Emit OSC 777 from PROMPT_COMMAND

Don't use character positions to extract the last command, and use bash's variable substitution to remove the semi-colon.
Comment 45 Egmont Koblinger 2015-02-03 18:06:39 UTC
(In reply to comment #43)

Sorry, I didn't mean to be impolite or offend anyone. My bad English probably, I don't know how to say in a polite way if what I'm think is actually that it's just too much ugly and bound-to-fail workarounds instead of proper solutions that I keep seeing in this ticket.

Also, I don't think I was the first hitting a personal and offensive tone in this bugreport. Sorry but it's hard to keep calm after that, even if it wasn't you Debarshi either.

You say it's hard to keep a constructive discussion with this tone? Well, I've constructively critized many parts of the current approach, telling why I believe they are wrong and offering something I think would be way better. Yet I hardly see any reflections on those, any counterarguments defending what I think is wrong or proving wrong what I think is right. Instead, I keep seeing the same patterns appearing that I've already expressed by bad opinion about.

I don't mean to be offensive, but try to be as consise and pragmatic as I can:

- Introducing an escape sequence that means "notify the user, unless there were no window title changes yet" - no thanks, it just doesn't make any sense. It'd be a mostly (but not 100% reliably) working _workaround_ rather than a proper fix for a totally irrevant bug.

- Introducing an escape sequence that means "notify the user, unless the window has never received focus before" - no thanks, same story again.

- Introducing an escape sequence that means "notify the user with 95% probablity if the window is not focused", building on top of an API that's known to be broken and then wash our hands and blame Gtk+ or X (we don't even know which) - no thanks!

- Emitting faulty notifications at shell startup that will be filtered out most of the time by some fragile magic - no thanks!

- Emitting a notification that the command "fg" finished running - no thanks!

- Not being able to handle a certain character in the message, and try to explain this unexplainable stupidity to the users - no thanks!

- Designing a system that notifies in only a limited set of cases by design, and be super happy about it that wow we do notify _sometimes_ - no thanks!

Exactly the same, reworded in a constructive way (note: it's not the first time I'm wrinting any of these, and it's not the first time I'm writing them in this constructive way, so I'm just repeating myself):

- _If_ we want to ever rely on the window being focused or not, let's fix the underlying bug first and then build on top of that!

- Let's only emit notification messages that do make sense! If this can only be achieved by patching bash then we should either patch bash or sadly abandon the idea.

- Let's implement notifications in a way that they have nothing to do with the window title! Also let's implement them in a way that the user can easily disable them. (Note: I believe vte/g-t should only provide the basis for this, that is the feature of being able to notify. Wiring to the bash prompt, along with the patch to bash if necessary (and patch to other apps, e.g. http://www.midnight-commander.org/ticket/2027 until upstream fixes it) doesn't belong to vte, rather it's the distribution's job to coordinate it across several apps incl. vte _if_ they decide they want to hook up to the prompt.)

- Let's study urxvt's and terminology's code, talk to their developers and to the guy who suggested the 777+notify feature to figure out how a semicolon should be escaped to make it appear literally. Come to an agreement with them and implement that. Or, if there's no agreement or if you'd prefer not to do this, let's choose a different escape sequence where we don't have this problem.

- Let's implement support for explicit notifications (which can be hooked up to bash) along with implicit notifications (based on whether there's activity in the terminal).

-----

The current patches are a nice proof of concept that on one hand show that it can be made a cool feature, on the other hand reveals problem with this approach, some of which problems probably wouldn't have occurred to us without this demo implementation. The completeness, quality (ratio of proper solution vs hackish workaround) is still very far from what I'd like to see in the final production version, but hopefully we'll get there one day!

One more issue that occurred to me right now:

Currently (on Ubuntu Utopic's popup dialog) the message only contains the command, e.g. I get a "sleep 5" message. Er... what does it mean? It makes no sense to me on its own. What would make sense to me is something like "Terminal command finished: [newline] sleep 5". And then another question is: where would this "Terminal command finished:" string come from? Hardwired to gnome-terminal? In that case you can't use notification for any other purpose. From vte.sh where PS1 is set? Then we need to do internationalize in shell script. Again I don't mean to criticize anything or offend anyone, it's just one more technical issue that needs to solved and probably nobody thought of it so far.

PS. I've just tried in gnome-shell and I don't get any notification whatsoever. Maybe Utopic's gnome-shell is broken or too old? Or pebkac? Nevermind...
Comment 46 Debarshi Ray 2015-02-03 20:07:08 UTC
(In reply to comment #42)
> There's another problem with job control. Execute "sleep 100", put it in
> background with ^Z (maybe "bg" too), bring it back to foreground with "fg". 100
> seconds later you'll get notified that the command "fg" finished running. Not
> good either.

This is a rare enough corner case that I am happy to live with.

> Seems that bash provides no reliable method to get the most recently finished
> foreground job's command line, or the fact that there's been none yet.
> 
> Instead of hackish workarounds, how about
> - either accepting that we can't reliably get this info and design a
> notification system where we don't rely on it, e.g. just present a symbol in
> the tab label without further details;

The tab label doesn't show the name of the command. You should really try to run the code or read it.

> - or patch bash (and zsh if needed) to make this info available, and perhaps
> even try to get it accepted upstream?

Are you willing to contribute your time towards convincing the bash maintainers to accept it?
Comment 47 Egmont Koblinger 2015-02-03 22:04:48 UTC
(In reply to comment #46)

> This is a rare enough corner case that I am happy to live with.

What is rare enough for you might be an actual problem for someone. I'm not happy with designing a solution where we can foresee bugreports we'll receive and have no clue about how we'd fix them. That being said, if this was the only issue, I'd be willing to accept it, but I still wouldn't be happy with it.

> The tab label doesn't show the name of the command. You should really try to
> run the code or read it.

I tried. It does show the name of the command on mouseover, as the tooltip of the lightbulb. It's also shown in the desktop notification (it's not the tab label as I said, but it's still shown and is still potentially incorrect).

If we design a system where this information is shown anywhere, let's design it in a way that it's always correct. Or decide to never show it, and then make the escape sequence not contain it either.

> Are you willing to contribute your time towards convincing the bash maintainers
> to accept it?

I'm happy to speak up in favor of it.

-----

Two new bugs I discovered with this approach:

- PROMPT_COMMAND is executed each time the prompt is printed. That is, just by pressing a lone Enter or ^C, it's executed again, causing the notification to be emitted again for a command that has finished a long time ago. This is most of the time ignored by the terminal with your design because the terminal is focused, but not always.

E.g. if I configure ssh to forward VTE_VERSION, the remote host will emit these notifications similarly to the local one (hey, _if_ I end up liking such notifications then I'd like them to work over ssh too). Over a slow or stalled network connection the response can very easily get delayed until the window or tab is no longer active, and then you'd get a faulty notification. We could work around this (along with the initial invalid notification) by looking at the history number if bash didn't have the ignoredups feature, but it does and it causes problems as you've pointed out earlier.

It's also possible to locally hit Enter multiple times while the foreground process is still running, and then switch to another window. In this case multiple identical desktop notifications will be initiated at once; how do window managers (or whoever is responsible) handle that?

- There's an ignorespace feature that makes commands not appear in history if they begin with space. I personally quite often use it, e.g. I have the muscle memory never to type "rm" but to type " rm" instead. If you launch "ls" followed by " rm -r bigdirectorytree" and switch away, you'll get notified that "ls" finished running.

So unfortunately bash's history is quite far from being a reliable source of the last finished command.

-----

Whereas I'd like to see where we're heading at (a correct design that's free of such "mostly works" hacks and works robustly) and seeing this goal might be important in every single step we take; I'm getting more and more certain that a correct responsibility split (same as in bug 743073 comment 6) is required. Implementing explicit notifications for a certain escape sequence is vte's/gnome-terminal's job. Deciding and implementing _when_ to emit this sequence (e.g. whether to notify when a command finishes) is not. It's the task of distributions, e.g. Fedora/Red Hat in our case. _If_ a distro decides to ship to users what you have in mind, it's their taks to implement it consistently, which unfortunately involves patching bash, patching mc and probably a few other apps, and shipping a modified vte.sh (or a vte-notification.sh next to mainstream's vte.sh).

If mainstream bash and zsh supported the necessary features to make it work out of the box (it's clear that bash doesn't; no clue about zsh) then I'd be okay with shipping the necessary bits in vte.sh, but not until then, since it does require a modified bash anyway, or is bound to be very buggy.

Assuming for a moment that mainstream bash will refuse our feature request: I don't see why Fedora/RH, instead of deciding to patching bash, would decide to push mainstream vte developers to ship something that should really be done in bash, that could be made to work 100% reliably there, whereas it's necessarily buggy if hacked around externally. Fedora/RH has equal power over solving it correctly at the right place, or hacking it around with multiple bugs in the wrong place. Please choose wisely and solve it at the correct place. A one step worse solution is if Fedora/RH chooses to work it around with multiple bugs at the wrong place instead, but that's none of my business. A two steps worse scenario is if Fedora/RH wants to push mainstream vte to accept this buggy-because-cant-be-done-correctly hack, to which I express my firm "no please" vote.

I think we should move forward with this responsibility split in mind. We should agree here what feature you expect from vte/gnome-terminal (how to handle a certain escape sequence), but seems we almost have a conclusion there. Vte/g-t implements this based on your patch, and wiring up to Fedora's shell prompt (that is, deciding where/how to emit this sequence) remains solely Fedora's business. How does it sound?
Comment 48 Debarshi Ray 2015-02-03 22:26:31 UTC
(In reply to comment #47)
> I tried. It does show the name of the command on mouseover, as the tooltip of
> the lightbulb.

If you read the code then you will also realize that it is trivial to change the tool tip to something like "Command completed". In fact that is how I have been running it locally for a while.
Comment 49 Debarshi Ray 2015-02-03 22:29:56 UTC
(In reply to comment #47)
> I think we should move forward with this responsibility split in mind. We
> should agree here what feature you expect from vte/gnome-terminal (how to
> handle a certain escape sequence), but seems we almost have a conclusion there.
> Vte/g-t implements this based on your patch, and wiring up to Fedora's shell
> prompt (that is, deciding where/how to emit this sequence) remains solely
> Fedora's business. How does it sound?

Are you saying that you are ready to accept the escape sequence (OSC 777 or something else) in vte, and then leave the task of emitting it to the distribution?
Comment 50 Debarshi Ray 2015-02-03 22:37:05 UTC
(In reply to comment #45)
> - Let's study urxvt's and terminology's code, talk to their developers and to
> the guy who suggested the 777+notify feature to figure out how a semicolon
> should be escaped to make it appear literally. Come to an agreement with them
> and implement that. Or, if there's no agreement or if you'd prefer not to do
> this, let's choose a different escape sequence where we don't have this
> problem.

Do you want me to talk to the urxvt and terminology developers? What about xterm? From comment 20 I got the impression that you already knew them well enough, and I was told by chpe that you can help with xterm too.

Is there a public mailing list for discussing terminal escape sequences? Or are we going to do it over private email?

> PS. I've just tried in gnome-shell and I don't get any notification whatsoever.
> Maybe Utopic's gnome-shell is broken or too old? Or pebkac? Nevermind...

The name of your desktop file should match the application ID. So if you run gnome-terminal with --app-id=foo.bar, then foo.bar.desktop should be present.
Comment 51 Egmont Koblinger 2015-02-03 22:42:50 UTC
(In reply to comment #48)

> If you read the code then you will also realize that it is trivial to change
> the tool tip to something like "Command completed". In fact that is how I have
> been running it locally for a while.

Seems to me that you didn't understand what I meant to say.

And we're back to the point where I said there were absolutely no specifications, UI mocks whatever, just a "notify somehow" feature request followed by a proof of concept implementation notifying somehow.

According to the plan (rather than the actual code), would you ever wish to show the command that completed? Or would you be happy with no desktop notification, or a generic "Command completed" desktop notification without further details?

If the answer is "we'd never under any circumstance show the command that completed" then my response is okay let's remove this parameter from the escape sequence.

If the answer is "the command that completed is shown at least sometimes at some place" then my response is okay but let's get the command in a robust reliable way which doesn't seem to be possible with unpatched bash, the current "history 1"-based approach already has like 3 or 4 known bugs.

The escape sequence we'll introduce needs to have a meaning. 
- This meaning can be "notify the user about something", in which case arbitrarily adding the "Command completed" sequence by gnome-terminal is incorrect, since it can't tell if a command was completed, or a user chose to use this sequence for a totally different purpose. (Adding this "Command completed" string to PROMPT_COMMAND is okay, but you'd need to internationalize it in the shell script.)
- The meaning could, on the other hand, be "notify the user about a command that completed". Then it's okay for g-t to add this prefix string, but then the user and apps shouldn't use it for any other purpose. E.g. if I wrote a script and wanted to notify but keep running on the first warning, I semantically couldn't use it.

I haven't thought about which approach would be the better, but we need to make a decision. The escape sequence will need to be documented, hopefully coordinated with xterm's author and made it into the "official" ctlseqs.txt. You can't say the meaning is to notify about whatever, and then arbitrarily prepend "command finished". Either one or the other, but not a mixture of the two semantics.
Comment 52 Debarshi Ray 2015-02-03 22:46:12 UTC
(In reply to comment #47)
> It's also possible to locally hit Enter multiple times while the foreground
> process is still running, and then switch to another window. In this case
> multiple identical desktop notifications will be initiated at once; how do
> window managers (or whoever is responsible) handle that?

I expect gnome-shell to only ever show one notification for a particular VteTerminal (or TerminalScreen) because we use the 'uuid' as the 'id':
https://developer.gnome.org/gio/stable/GApplication.html#g-application-send-notification
Comment 53 Egmont Koblinger 2015-02-03 23:14:37 UTC
(In reply to comment #49)

> Are you saying that you are ready to accept the escape sequence (OSC 777 or
> something else) in vte, and then leave the task of emitting it to the
> distribution?

I can see the end of the tunnel in this direction.  Once we figure out the remaining details (exact syntax: 777 or not, how many parameters, how to escape semicolon if necessary; exact semantics: is it for command completed or for generic notification; exact behavior: when lightbulb, when desktop notification) and implement the mentioned issues (the race condition of comment 24 occurs to me right now) then yep I'm up for it.  I'd like to see the implicit notifications based on activity to be implemented too, but probably it could be done independently.  Does it sound good for you too?

Note: if we decide to depend on focused vs. unfocused, I'd still like to see the relevant focus in/out bug fixed (or I'm fine with any other reliable way of figuring out if the window is focused or not; maybe it can be done reliably despite of the missing signals, I don't know.)

(In reply to comment #50)

> Do you want me to talk to the urxvt and terminology developers? What about
> xterm? From comment 20 I got the impression that you already knew them well
> enough, and I was told by chpe that you can help with xterm too.

I've talked to xterm developer quite a lot, and to terminology developers in their bugtracker.  I can take care of this, but I can't promise any timeline. If you feel like doing it, you can go ahead, or you can leave it to me, whichever you prefer.  (I would take a closer look at the code to see what they're doing exactly before talking to them.  Also for talking to xterm's author we should already have clear answer to the desired semantics, see above.)

> Is there a public mailing list for discussing terminal escape sequences? Or are
> we going to do it over private email?

Unfortunately the latter. We could cc each other, but that's all.

(In reply to comment #52)

> I expect gnome-shell to only ever show one notification for a particular
> VteTerminal (or TerminalScreen) because we use the 'uuid' as the 'id':

Errrrr... so I launch a long-running command, I get notified when it finishes, then I launch another long-running command from the same tab and then I won't get notified!?  I hope it's a bug and not your intended behavior.  Anyway, in Unity I get multiple popup boxes, unless the notifications are emitted right after each other (not sure if it's a Unity feature, or a g-t bug that it swallows one).
Comment 54 Egmont Koblinger 2015-02-03 23:41:29 UTC
I'm looking at the desktop notification spec at
    http://www.galago-project.org/specs/notification/
and I'm wondering... would it make sense to design this escape sequence as a generic wrapper around desktop notification, with many of fields definable, and the escape sequence extendable later as the spec evolves?

Something along the lines of
OSCsomething;summary=foo;body=bar baz;expire_timeout=1000\a

Which fields would be definable and which ones would be forced to a certain value?

Does this sound like a good approach?  Or am I taking this too far?
Comment 55 Matthias Clasen 2015-02-04 07:49:02 UTC
(In reply to comment #54)
> I'm looking at the desktop notification spec at
>     http://www.galago-project.org/specs/notification/
> and I'm wondering... would it make sense to design this escape sequence as a
> generic wrapper around desktop notification, with many of fields definable, and
> the escape sequence extendable later as the spec evolves?
> 
> Something along the lines of
> OSCsomething;summary=foo;body=bar baz;expire_timeout=1000\a
> 
> Which fields would be definable and which ones would be forced to a certain
> value?
> 
> Does this sound like a good approach?  Or am I taking this too far?

The GNotification api does not support all the fields that are in that spec. expiration time is an example - gnotifications don't expire. It is a good idea to keep extensibility in mind, and come up with a syntax that allows to add fields, but initially, just supporting summary and body would be fine.
Comment 56 Bastien Nocera 2015-02-04 08:51:14 UTC
(In reply to comment #54)
> I'm looking at the desktop notification spec at
>     http://www.galago-project.org/specs/notification/

That's an ancient version of the spec. This is the latest one:
https://people.gnome.org/~mccann/docs/notification-spec/notification-spec-latest.html
Comment 57 Debarshi Ray 2015-02-04 13:12:22 UTC
(In reply to comment #53)
> > I expect gnome-shell to only ever show one notification for a particular
> > VteTerminal (or TerminalScreen) because we use the 'uuid' as the 'id':
> 
> Errrrr... so I launch a long-running command, I get notified when it finishes,
> then I launch another long-running command from the same tab and then I won't
> get notified!?

No. Let me quote the documentation that I linked to:
"If a previous notification was sent with the same id , it will be replaced with notification and shown again as if it was a new notification. This works even for notifications sent from a previous execution of the application, as long as id is the same string."

So, if a series of notifications are sent from the same tab (ie. with the same 'id'), then the newer ones will replace the older notifications.

> I hope it's a bug and not your intended behavior.  Anyway, in
> Unity I get multiple popup boxes, unless the notifications are emitted right
> after each other (not sure if it's a Unity feature, or a g-t bug that it
> swallows one).

Definitely something to do with unity's support for org.gtk.Notification.
Comment 58 Egmont Koblinger 2015-02-04 13:25:16 UTC
(In reply to comment #57)

> > > I expect gnome-shell to only ever show one notification [...]

> "If a previous notification was sent with the same id , it will be replaced
> with notification and shown again as if it was a new notification.

So, it's at most one notification at a time, not one notification during the complete lifetime of the widget? Makes sense now. (Even though Unity disobeys this.)

Slightly related, quite minor issues: As said, Unity doesn't support notifications with actions. Receiving a popup which is hidden underneath the currently active window is not a nice UI and not helpful either. It seems to me that the specification allows to query if a notification with action is supported or not. I hope Unity answers "no" to this. In that case how about falling back to presenting a non-action notification? We'd lose the button that switches to the relevant tab, but at least it'd be properly presented.

Also, the two buttons could have a more descriptive text, e.g. "Close" and "Take me to that tab", rather than Cancel/OK.
Comment 59 Egmont Koblinger 2015-02-04 14:46:00 UTC
I tend to like the generic notification idea (comment 54) and I take Matthias's response as implicitly supporting it too.

When wiring up the notification to the shell, an obvious improvement could be to notify differently for succeeded vs. failed commands (based on $?). For the desktop notification, this could be achieved by different wording, e.g. "Command completed: \n sleep 5" vs "Command failed: \n ssh unreachablehost".

For the in-tab-label notification, would we want to have separate icons for them? If so, can we and should we duplicate the notification spec's bits that say >> The "app_icon" parameter [should be ...] or a name in a freedesktop.org-compliant icon theme <<?

This would mean we start off by supporting the summary, body and app_icon fields. (Or start off by summary and body, and at least plan for an improvement later on by adding the app_icon feature.)

I'm not sure what icons a freedesktop-compliant theme supplies (e.g. if there's a lightbulb, or any relevant icon that we'd be happy with). Of course we can have a default lightbulb that's not taken from there, in case this parameter is not specified.
Comment 60 Debarshi Ray 2015-02-04 15:03:22 UTC
(In reply to comment #51)
> According to the plan (rather than the actual code), would you ever wish to
> show the command that completed? Or would you be happy with no desktop
> notification, or a generic "Command completed" desktop notification without
> further details?

Showing the command in the desktop notification is a nice touch. Otherwise there is not enough context for the user.

I would like to know what Allan has to say about this.

> If the answer is "the command that completed is shown at least sometimes at
> some place" then my response is okay but let's get the command in a robust
> reliable way which doesn't seem to be possible with unpatched bash, the current
> "history 1"-based approach already has like 3 or 4 known bugs.

While I want to fix bash to get the last foreground command in a reliable way, implementing preexec is even higher in my list of things because it offers a bigger bang for buck.

Getting the command string wrong in a few edge cases is not the end of the world for me. The biggest benefit of this feature is for those who deal with big git repositories or do long-running builds. At the end of the day, we may never manage to get anything into upstream bash, and that should not mean that we never implement this feature. Asking every distribution to patch bash is also impractical, even if I would like that to happen.
Comment 61 Debarshi Ray 2015-02-05 13:43:35 UTC
Created attachment 296197 [details] [review]
Support desktop notifications from OSC 777

Simplify the hack to suppress the spurious emissions by not using window-title-changed. We could have just used notification-received for that.
Comment 62 Egmont Koblinger 2015-03-07 10:28:47 UTC
ChPe pointed us to http://iterm2.com/shell_integration.html#/section/home (in another bug), it seems relevant here, especially if bash ends up refusing our requested feature.  It seems to add special escape sequences to the prompt, so that the terminal can safely recognize the prompt as well as the command executed.  I bet it still gets confused as for which command finished in case job control is used, but apart from that, it can probably reliably tell when a command finishes.  Still significantly more complicated than explicit support from bash would be, but definitely something we should look into.
Comment 63 Debarshi Ray 2015-03-08 00:42:47 UTC
(In reply to Egmont Koblinger from comment #62)
> ChPe pointed us to http://iterm2.com/shell_integration.html#/section/home
> (in another bug), it seems relevant here, especially if bash ends up
> refusing our requested feature.  It seems to add special escape sequences to
> the prompt, so that the terminal can safely recognize the prompt as well as
> the command executed.  I bet it still gets confused as for which command
> finished in case job control is used, but apart from that, it can probably
> reliably tell when a command finishes.  Still significantly more complicated
> than explicit support from bash would be, but definitely something we should
> look into.

How is that fundamentally different from putting a custom escape sequence in PROMPT_COMMAND ?
Comment 64 Egmont Koblinger 2015-03-08 01:42:53 UTC
(In reply to Debarshi Ray from comment #63)

> How is that fundamentally different from putting a custom escape sequence in
> PROMPT_COMMAND ?

What do you mean exactly?  I'm sorry I don't get your question, but try to answer it anyway, perhaps I succeed.  If not then please clarify the question.

The main question is not whether there's an escape sequence or not; the main question is: what does that escape sequence do?

So far we were considering implementing escape sequences for emitting notifications, but not for figuring out what those notifications should be.  We were thinking about using "history 1", or preferably a more reliable method provided by bash, neither of them relying on escape sequences.  Then we'd construct the message we wanted to show to the users, and emit it via escape sequences.

The new idea would be to use escape sequences for a totally different purpose: for understanding what's happening inside the terminal.  An escape sequence would tell the terminal "here's where the prompt begins" and another would signal "here's where the prompt ends".  Then the terminal on its own could figure out that anywhing from the "here's where the prompt ends" point up to the next explicit newline is presumably the command running.  And the next occurrence of a "here's where the prompt begins" most likely signals that the command has finished.  And from these the terminal could emit the notification.

This is a more fragile and harder-to-implement approach and not as generic as explicit support from bash would be (still erroneous when job control is used; totally wrong if multiline editing mode is disabled; wrong if ^C aborts editing a command over a slow ssh channel and you switch away from that tab in the mean time; not possible to emit custom messages from other apps etc.), but would offer other independent advantages (see bug 721689 commet 10 onwards) which might be nice to have, even if not for notifications.

My preferred solution is still the previous plan, with a patched bash.  Should bash refuse that new feature, relying on these new escape sequences that tell us where the prompt is _might_ be a plan B - I'm not sure if this is the runner-up design or not.
Comment 65 Debarshi Ray 2015-03-10 13:06:05 UTC
(In reply to Egmont Koblinger from comment #64)
> (In reply to Debarshi Ray from comment #63)
> 
> > How is that fundamentally different from putting a custom escape sequence in
> > PROMPT_COMMAND ?
> 
> [...]
>
> The new idea would be to use escape sequences for a totally different
> purpose: for understanding what's happening inside the terminal.  An escape
> sequence would tell the terminal "here's where the prompt begins" and
> another would signal "here's where the prompt ends".  Then the terminal on
> its own could figure out that anywhing from the "here's where the prompt
> ends" point up to the next explicit newline is presumably the command
> running.  And the next occurrence of a "here's where the prompt begins" most
> likely signals that the command has finished.  And from these the terminal
> could emit the notification.

I see.

> This is a more fragile and harder-to-implement approach and not as generic
> as explicit support from bash would be (still erroneous when job control is
> used; totally wrong if multiline editing mode is disabled; wrong if ^C
> aborts editing a command over a slow ssh channel and you switch away from
> that tab in the mean time; not possible to emit custom messages from other
> apps etc.), but would offer other independent advantages (see bug 721689
> commet 10 onwards) which might be nice to have, even if not for
> notifications.
> 
> My preferred solution is still the previous plan, with a patched bash. 
> Should bash refuse that new feature, relying on these new escape sequences
> that tell us where the prompt is _might_ be a plan B - I'm not sure if this
> is the runner-up design or not.

I don't like this option either. Taking PROMPT_COMMAND away from the user because the terminal widget uses it to do its own thing is bad enough. Touching PS1 would be much worse.
Comment 66 Debarshi Ray 2015-04-15 10:02:25 UTC
(In reply to Christian Persch from comment #10)
> Comment on attachment 295654 [details] [review] [review]
> Has the issue with backward compatibility on desktop file rename been solved?

gnome-shell has a mapping from old names to new ones
Comment 67 Debarshi Ray 2015-04-15 10:04:27 UTC
Created attachment 301610 [details] [review]
Support desktop notifications from OSC 777

Use focus-in to hide the notification.
Comment 68 Bastien Nocera 2015-04-15 10:20:21 UTC
(In reply to Debarshi Ray from comment #66)
> (In reply to Christian Persch from comment #10)
> > Comment on attachment 295654 [details] [review] [review] [review]
> > Has the issue with backward compatibility on desktop file rename been solved?
> 
> gnome-shell has a mapping from old names to new ones

In bug 745626.
Comment 69 Christian Persch 2015-04-17 19:38:49 UTC
Comment on attachment 295654 [details] [review]
Make notifications based on org.gtk.Notification work

So let's commit this part now. But please make sure that this will record as a git mv, not rm/add.
Comment 70 Debarshi Ray 2016-02-19 17:19:47 UTC
Now that Carlos has fixed the missing focus-in/out events in gtk+ (bug 677329), what else is left to be done here?
Comment 71 alsoijw 2016-03-26 12:13:45 UTC
1) If the last command is not recorded in history, the notice on execution will be the last command recorded in history.
2) the Notification does not work for fish shell
Comment 72 Debarshi Ray 2016-03-29 15:22:18 UTC
(In reply to alsoijw from comment #71)
> 1) If the last command is not recorded in history, the notice on execution
> will be the last command recorded in history.

How did you disable the history? set +o history?

> 2) the Notification does not work for fish shell

These patches only target bash, which is the most well integrated shell in gnome-terminal. For example, on zsh, opening a new terminal won't open it in the current working directory. I can consider supporting other more popular shells but I want to get it work well with bash first.
Comment 73 alsoijw 2016-03-29 17:55:38 UTC
In bash I add `export HISTCONTROL=ignoreboth` in .bashrc. Fish by default doesn't add command that start from space.
Comment 74 Christian Persch 2016-05-08 09:38:16 UTC
Comment on attachment 295652 [details] [review]
emulation: Add sequences and signals for desktop notification

+       void (*notification_received)(VteTerminal* terminal, const gchar *summary, const gchar *body);

"Nice" ABI break between upstream and fedora's vte by adding this right in the middle instead of at the end before the padding... :-P
Comment 75 Debarshi Ray 2016-05-09 15:19:14 UTC
(In reply to Christian Persch from comment #74)
> Comment on attachment 295652 [details] [review] [review]
> emulation: Add sequences and signals for desktop notification
> 
> +       void (*notification_received)(VteTerminal* terminal, const gchar
> *summary, const gchar *body);
> 
> "Nice" ABI break between upstream and fedora's vte by adding this right in
> the middle instead of at the end before the padding... :-P

Oh, damn! I went overboard while sprinkling code needed to add the notification-received signal - blindly adding everything near the window-title-changed stuff.

I went through all the packages that link against vte in Fedora, and only gnome-terminal uses the function pointers in VteTerminalClass. I will undo the ABI break in the next round of updates.

Thanks for the review.
Comment 76 Bastien Nocera 2016-05-09 15:34:05 UTC
(In reply to Debarshi Ray from comment #75)
> (In reply to Christian Persch from comment #74)
> > Comment on attachment 295652 [details] [review] [review] [review]
> > emulation: Add sequences and signals for desktop notification
> > 
> > +       void (*notification_received)(VteTerminal* terminal, const gchar
> > *summary, const gchar *body);
> > 
> > "Nice" ABI break between upstream and fedora's vte by adding this right in
> > the middle instead of at the end before the padding... :-P
> 
> Oh, damn! I went overboard while sprinkling code needed to add the
> notification-received signal - blindly adding everything near the
> window-title-changed stuff.
> 
> I went through all the packages that link against vte in Fedora, and only
> gnome-terminal uses the function pointers in VteTerminalClass. I will undo
> the ABI break in the next round of updates.

You'll need to check whether there are users of the function pointers at the end of the struct, that means users of window_title_changed, icon_title_changed, etc.
Comment 77 Debarshi Ray 2016-05-09 17:17:20 UTC
(In reply to Bastien Nocera from comment #76)
> (In reply to Debarshi Ray from comment #75)
> > I went through all the packages that link against vte in Fedora, and only
> > gnome-terminal uses the function pointers in VteTerminalClass. I will undo
> > the ABI break in the next round of updates.
> 
> You'll need to check whether there are users of the function pointers at the
> end of the struct, that means users of window_title_changed,
> icon_title_changed, etc.

Right.

Actually, Fedora's gnome-terminal is only affected because it uses the notification_received vfunc, which will now get moved around. Its use of child_exited is not a problem.
Comment 78 Bastien Nocera 2016-05-10 10:34:39 UTC
(In reply to Debarshi Ray from comment #75)
> and only
> gnome-terminal uses the function pointers in VteTerminalClass.

I missed that bit.
Comment 79 Egmont Koblinger 2016-09-16 22:43:49 UTC
FYI: bash-4.4 was just released.

Although its maintainer refused to implement what I asked for for a proper notification support, he has added this:

jj. New prompt string: PS0.  Expanded and displayed by interactive shells after
    reading a complete command but before executing it.

Just like PS1 and friends, PS0 can contain command substitution with $() or ``. So as far as I understand, this is equivalent to the missing preexec feature (which could only be implemented by misusing the debug trap).

This might make us one tiny (alas really tiny) step closer to properly implement notifications. Unfortunately though, we still cannot reliably call "history 1" from this (it gets fooled e.g. by commands starting by spaces).

If PS0 and PS1 both emitted a certain (different) new escape sequence, the terminal emulator could probably quite reliably figure out the command being executed by examining the on-screen contents between the two points where these escape sequences were printed. (It'd still be fooled by job control I'm afraid.)
Comment 80 jc 2017-01-21 19:59:45 UTC
The feature is nice but it seems to be working only with bash.
I have failed to make it work for zsh on Fedora 25.
I read all comments here but it is not clear to me if zsh is supported or not.
Am I missing something?
Comment 81 Kevin Cox 2017-01-21 20:39:57 UTC
Works fine with zsh for me. Note that zsh won't escape this sequence itself but you can work around this by doing something like the following.

precmd() {
	local command="$(history -1 | sed 's/^ *[0-9]\+ *//')"
	printf "\033]777;notify;Command completed;%s\007" "$command"
}
Comment 82 jc 2017-01-22 08:56:02 UTC
Thank you for the tip! With this, it works perfectly :-)
Comment 83 Kevin Cox 2017-02-15 03:01:03 UTC
Debarshi Ray: Do you have any more progress on this? Is there anything holding this back?
Comment 84 Christian Persch 2017-02-15 18:36:11 UTC
(In reply to Kevin Cox from comment #83)
> Do you have any more progress on this? Is there anything
> holding this back?

If you'd read all of the bug, you'd have seen that this is stalled because the vte/gnome-terminal developers do not think that this patch is taking the right approach.

If you want a starting point of what needs to be done: at a minimum, bash needs to gain hooks for pre- and post- command execution, and for job control (fg, bg, %n), so that vte.sh can reliably determine just what the current 'task' actually is.
Comment 85 Kevin Cox 2017-02-16 02:14:05 UTC
IIUC it seems like these arguments are all about the method of teaching bash how to emit this. Would it be possible to ship the handling in VTE and gnome-terminal and pull the discussion on how to teach the shell about it into a different issue? Basically what you suggested in #69 but it had an ABI break which looks easy to fix. I was wondering what was preventing that part to be committed.
Comment 86 Debarshi Ray 2018-04-19 15:41:31 UTC
(In reply to Egmont Koblinger from comment #79)
> FYI: bash-4.4 was just released.
> 
> Although its maintainer refused to implement what I asked for for a proper
> notification support, he has added this:
> 
> jj. New prompt string: PS0.  Expanded and displayed by interactive shells
> after
>     reading a complete command but before executing it.

This seems relevant for automatically updating the title (bug 711060) to represent the current foreground command. I added a comment to that bug outlining a vague plan. What do you think?

> Just like PS1 and friends, PS0 can contain command substitution with $() or
> ``. So as far as I understand, this is equivalent to the missing preexec
> feature (which could only be implemented by misusing the debug trap).
> 
> This might make us one tiny (alas really tiny) step closer to properly
> implement notifications. Unfortunately though, we still cannot reliably call
> "history 1" from this (it gets fooled e.g. by commands starting by spaces).

I am not sure that PS0 (or anything that's similar to Z shell's preexec) is obviously related to notifying about long-running commands. Or am I missing something?

> If PS0 and PS1 both emitted a certain (different) new escape sequence, the
> terminal emulator could probably quite reliably figure out the command being
> executed by examining the on-screen contents between the two points where
> these escape sequences were printed. (It'd still be fooled by job control
> I'm afraid.)

(In reply to Christian Persch from comment #84)
> If you want a starting point of what needs to be done: at a minimum, bash
> needs to gain hooks for pre- and post- command execution, and for job
> control (fg, bg, %n), so that vte.sh can reliably determine just what the
> current 'task' actually is.

I don't think that simple pre- and post- command execution hooks would let us implement notifications that work with job control. We always had PROMPT_COMMAND as a post-execution hook, and now PS0 is a pre-execution hook. What we want is a way to find out the actual foreground command that was interactively typed in by the user from a post-execution hook, and not just fg or bg.

Having said that, in practice, does job control really matter? This feature is meant for notifying about a long-running wget, make, git clone, and such. Those things usually have some progress indicator, which makes it unlikely that the user would put them in the background. My usual workflow is to type something in emacs, then hit ctrl+z, start a build, possibly switch windows to look at something else, and wait for the notification. When the notification comes in, I switch back to the window, hit fg, and continue typing in emacs.

We could add a setting to turn this off in case somebody is put off by the notifications not working with job control.
Comment 87 Debarshi Ray 2018-04-19 16:08:44 UTC
(In reply to Debarshi Ray from comment #86)
> Having said that, in practice, does job control really matter? This feature
> is meant for notifying about a long-running wget, make, git clone, and such.
> Those things usually have some progress indicator, which makes it unlikely
> that the user would put them in the background. My usual workflow is to type
> something in emacs, then hit ctrl+z, start a build, possibly switch windows
> to look at something else, and wait for the notification. When the
> notification comes in, I switch back to the window, hit fg, and continue
> typing in emacs.
> 
> We could add a setting to turn this off in case somebody is put off by the
> notifications not working with job control.

To back this up with real numbers, I went through the Fedora and RHEL bug trackers to see what people have been complaining about. So, far I have only seen two bugs related to this:

(a) https://bugzilla.redhat.com/show_bug.cgi?id=1321345: The notification is wrong if the history was disabled, and it doesn't work with the fish shell.

(b) https://bugzilla.redhat.com/show_bug.cgi?id=1353216: This is basically a badly configured system where the PROMPT_COMMAND is emitting the escape sequence but vte is unable to handle it. I think some of those users were using the pre-2.91 VTE ABI, which doesn't know about the escape sequence.

Given that we have been shipping the notification patches since Fedora 22 and RHEL 7.4, and that vte and gnome-terminal are pretty popular, that's a really small number of bug reports and/or people. (a) could be addressed by a setting, and (b) should be solved by people not using the old ABI.
Comment 88 Debarshi Ray 2018-04-19 16:12:49 UTC
(In reply to Egmont Koblinger from comment #47)
> E.g. if I configure ssh to forward VTE_VERSION, the remote host will emit
> these notifications similarly to the local one (hey, _if_ I end up liking
> such notifications then I'd like them to work over ssh too). Over a slow or
> stalled network connection the response can very easily get delayed until
> the window or tab is no longer active, and then you'd get a faulty
> notification. We could work around this (along with the initial invalid
> notification) by looking at the history number if bash didn't have the
> ignoredups feature, but it does and it causes problems as you've pointed out
> earlier.

Well, we don't forward VTE_VERSION today and various things already don't work at the moment. eg., the terminal title doesn't reflect the username, hostname or the current working directory of the ssh session.
Comment 89 Debarshi Ray 2018-05-09 18:13:06 UTC
(In reply to Debarshi Ray from comment #86)
> I am not sure that PS0 (or anything that's similar to Z shell's preexec) is
> obviously related to notifying about long-running commands. Or am I missing
> something?
>
> [...]
>
> I don't think that simple pre- and post- command execution hooks would let
> us implement notifications that work with job control. We always had
> PROMPT_COMMAND as a post-execution hook, and now PS0 is a pre-execution
> hook. What we want is a way to find out the actual foreground command that
> was interactively typed in by the user from a post-execution hook, and not
> just fg or bg.

Ok, thinking about it some more, I see what you mean. We could call tcgetpgrp from the preexec hook to read the current foreground process, and then notify from the precmd hook.

The vte patches in bug 711060 add VteTerminal signals to match the shell hooks.
Comment 90 Debarshi Ray 2018-05-10 17:25:26 UTC
Created attachment 371905 [details] [review]
[gnome-terminal] Notify when a long-running foreground process terminates
Comment 91 Debarshi Ray 2018-05-16 14:12:16 UTC
The patches are also in the following branches:
vte.git:wip/rishi/precmd-preexec
gnome-terminal.git:wip/rishi/auto-title-notify
Comment 92 Debarshi Ray 2018-07-17 08:05:38 UTC
Ping. Any comments about the new approach that uses Bash's PS0 as a pre-exec hook?
Comment 93 Egmont Koblinger 2018-07-17 09:50:45 UTC
Sorry for the long silence.

My favorite two (non-exclusive) approaches so far:

1. Implicit (activity-based) coloring of the title (last section of comment 20).

2. Generic escape sequence for arbitrary (not just "command finished") notifications (comment 54). Gluing with shells would be left to distributions, i.e. it would be totally up to you whether you use PS0 or the debug trap or a patched bash or whatever in downstream RH. (As long as bash doesn't properly support a hook for command completion, I wouldn't want to include any gluing attempt with its inevitable bugs in VTE, let alone then deal with the bugreports).

I might work on the latter one (or both; but for reasons I don't understand (comment 20, the section with [+] and [-] bullet points) the 2nd one seems to be more frequently requested) in the 0-55 series (I almost certainly won't have time in the current 0-53 cycle).
Comment 94 Debarshi Ray 2018-07-17 14:37:40 UTC
(In reply to Egmont Koblinger from comment #93)
> 1. Implicit (activity-based) coloring of the title (last section of comment
> 20).

Isn't that different from getting notifications on command completion?

> 2. Generic escape sequence for arbitrary (not just "command finished")
> notifications (comment 54). Gluing with shells would be left to
> distributions, i.e. it would be totally up to you whether you use PS0 or the
> debug trap or a patched bash or whatever in downstream RH.

The second iteration of the patches don't use a "notification" or "command finished" escape sequence. They use a pair of "pre-exec" and "post-exec" escape sequences that are triggered by Bash's PS0 and PROMPT_COMMAND. They are also used to implement automatic title updates (bug 711060).

Do you have any thoughts on how you'd like to implement automatic title updates?

> (As long as bash
> doesn't properly support a hook for command completion, I wouldn't want to
> include any gluing attempt with its inevitable bugs in VTE, let alone then
> deal with the bugreports).

As I mentioned in comment 87, I can't find any bugs worth mentioning in all the time that Fedora and RHEL have been shipping these patches. The fact that we no longer rely on Bash's history being enabled further reduces the chances of breakage.
Comment 95 Egmont Koblinger 2018-07-17 14:57:42 UTC
> Isn't that different from getting notifications on command completion?

Yup, it's somewhat related, but doesn't give desktop notifications.

> They use a pair of "pre-exec" and "post-exec" [...]

Looks like I really need to catch up with the work you've been doing. It's not necessarily in my preferred direction (could even be better, who knows). Alas I hardly have time nowadays.
Comment 96 GNOME Infrastructure Team 2021-06-10 20:36:23 UTC
-- 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/gnome-terminal/-/issues/7378.