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 557593 - gnome-terminal should set the urgent hint upon receiving a terminal bell
gnome-terminal should set the urgent hint upon receiving a terminal bell
Status: RESOLVED OBSOLETE
Product: gnome-terminal
Classification: Core
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: GNOME Terminal Maintainers
GNOME Terminal Maintainers
: 588283 641982 694807 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-10-23 11:43 UTC by Bojan Matic
Modified: 2021-06-10 19:47 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
naive implementation of vte beep/urgency hint support (1.48 KB, patch)
2008-10-27 09:31 UTC, Mikel Ward
none Details | Review
patch against 2.22 that sets the urgency hint only if the whole window doesn't have focus (1.64 KB, patch)
2008-10-27 11:14 UTC, Mikel Ward
needs-work Details | Review
patch against 2.22 that sets the urgency hint for an unfocussed window and bolds the tab title for an unfocussed tab (8.26 KB, patch)
2008-10-29 06:47 UTC, Mikel Ward
none Details | Review
patch against 2.22 that sets the urgency hint for an unfocussed window and bolds the tab title for an unfocussed tab (respin) (8.18 KB, patch)
2008-10-29 11:55 UTC, Mikel Ward
needs-work Details | Review
same as before, this time against 2.24 (5.17 KB, patch)
2008-11-05 11:06 UTC, Mikel Ward
none Details | Review
patch against trunk that highlights the tab/window on bell (with pref and docs) (11.75 KB, patch)
2008-11-10 10:59 UTC, Mikel Ward
needs-work Details | Review
work-in-progress using Christian's design, still needs prefs, docs (12.82 KB, patch)
2009-05-02 01:41 UTC, Mikel Ward
none Details | Review
work-in-progress using Christian's design, with some prefs and docs (16.00 KB, patch)
2009-05-03 11:52 UTC, Mikel Ward
none Details | Review
ready-for-review patch based on Christian's design (18.91 KB, patch)
2009-05-03 14:05 UTC, Mikel Ward
needs-work Details | Review
added code to display a DBUS desktop notification popup on bell (apply on top of previous patch) (11.92 KB, patch)
2009-05-15 05:53 UTC, Mikel Ward
needs-work Details | Review
patch to implement the bell raised property (4.29 KB, patch)
2009-10-10 00:23 UTC, Mikel Ward
none Details | Review
patch to implement two preferences, required by other patches in this series (9.05 KB, patch)
2009-10-10 00:26 UTC, Mikel Ward
none Details | Review
patch to make the window set the urgency hint when a bell is emitted (2.82 KB, patch)
2009-10-10 00:27 UTC, Mikel Ward
none Details | Review
patch to make the tab's title bold if a bell is emitted (1.48 KB, patch)
2009-10-10 00:32 UTC, Mikel Ward
none Details | Review
draft combined patch (21.05 KB, patch)
2010-04-17 02:41 UTC, Mikel Ward
none Details | Review
ditto but with fixed "even if the window" label (23.45 KB, patch)
2010-04-17 03:02 UTC, Mikel Ward
none Details | Review
patch against git HEAD to add a property called "bell-raised" (5.91 KB, patch)
2010-05-18 03:28 UTC, Mikel Ward
needs-work Details | Review
patch against git HEAD to make the tab title bold, includes prefs and docs (9.46 KB, patch)
2010-05-18 03:30 UTC, Mikel Ward
none Details | Review
patch against git HEAD to set the window urgency hint, includes prefs and docs (10.40 KB, patch)
2010-05-18 03:32 UTC, Mikel Ward
none Details | Review

Description Bojan Matic 2008-10-23 11:43:23 UTC
The usability improvement is that the user is notified about a terminal needing his attention if he has the PC speaker disabled and is on another workspace or has gnome-terminal minimized. It would also make using CLI IRC clients a nicer experience.

Other information:
Comment 1 Christian Persch 2008-10-23 12:42:06 UTC
Not sure. Blinking windows in the window list are annoying...
Comment 2 Bojan Matic 2008-10-24 13:57:41 UTC
What's wrong with providing it as an option ? Those who don't want it, need not enable it.
urxvt does this, and I'm sure there are other terminal emulators supporting this feature.
Comment 3 Mikel Ward 2008-10-27 09:29:54 UTC
Now that bug 329108 in Vte has been fixed, GNOME Terminal can listen for the "beep" event.

I'll attach a simple patch against GNOME Terminal here.
Comment 4 Mikel Ward 2008-10-27 09:31:38 UTC
Created attachment 121413 [details] [review]
naive implementation of vte beep/urgency hint support

Patch against GNOME Terminal 2.22.

It does the minimal stuff required to work to my satisfaction: it doesn't yet include a preference to turn it off.

I'll re-spin against trunk when I upgrade my laptop to GNOME 2.24.
Comment 5 Mikel Ward 2008-10-27 11:14:24 UTC
Created attachment 121423 [details] [review]
patch against 2.22 that sets the urgency hint only if the whole window doesn't have focus

In the related Vte bug (bug 329108), Mariano and Behad suggested that GNOME Terminal should only set the urgency hint if the entire window was not visible.

The next logical step is to mark tabs with unacknowledged events in some special way, for instance bolding the title.
Comment 6 Christian Persch 2008-10-27 11:26:04 UTC
Patch doesn't apply to svn trunk; setting patch status.
Comment 7 Mikel Ward 2008-10-29 06:47:36 UTC
Created attachment 121561 [details] [review]
patch against 2.22 that sets the urgency hint for an unfocussed window and bolds the tab title for an unfocussed tab

This patch does everything I want it to:
1) If a terminal receives a beep and its window isn't focussed, the window's urgency hint is set
2) If a terminal receives a beep and it itself isn't focussed, its tab's title is bolded

Both can apply simultaneously, i.e. if an inactive tab in an unfocussed window beeps, the tab title is bolded and the window is made urgent.
Comment 8 Mikel Ward 2008-10-29 11:55:05 UTC
Created attachment 121576 [details] [review]
 patch against 2.22 that sets the urgency hint for an unfocussed window and bolds the tab title for an unfocussed tab (respin)

Minor changes:
- name unused GtkDirection variable consistently (calling it "foo" doesn't look good)
- don't call gtk_label_set_use_markup, since that shouldn't be necessary when only using attributes

Some other things to think about:
- whether logically the window or the screen should handle the events
- whether the events should be highlight-tab or if instead the beep event should be caught and thrown all the way up to the screen or window, whichever is handling it (this would require a method to ask the screen whether its associated terminal widget had the input focus)
- whether we should offer a similar feature for other events, such as when a tab's title changes
Comment 9 Mikel Ward 2008-11-05 11:06:01 UTC
Created attachment 122001 [details] [review]
same as before, this time against 2.24

Updated version for 2.24.

I haven't tried against trunk as trunk seems to be broken at the moment.
Comment 10 Mikel Ward 2008-11-05 11:51:47 UTC
Comment on attachment 122001 [details] [review]
same as before, this time against 2.24

There are some minor differences between the patches for 2.22 and 2.24
1) I don't explicitly emit a focus event from the screen and instead catch both focus and focus-in-event events - I'm not sure if this is better (another option is to handle the clear urgency hint on focus-in-event and handle the clear tab highlight on focus event)
2) Fixed the type of weight in unhighlight_screen_cb
Comment 11 Christian Persch 2008-11-05 11:59:17 UTC
(In reply to comment #9)
> I haven't tried against trunk as trunk seems to be broken at the moment.
Broken how exactly?

 

Comment 12 Mikel Ward 2008-11-05 22:32:24 UTC
When I tried to build an unmodified version of trunk a few days ago I got some compile errors.

I'll try again later today and let you know specifics if there are any problems.
Comment 13 Mikel Ward 2008-11-10 10:59:05 UTC
Created attachment 122318 [details] [review]
patch against trunk that highlights the tab/window on bell (with pref and docs)

The previous patch actually applied against trunk and worked fine.

I used that as the basis for this patch.

This version also adds a "highlight on bell" preference and a "C" locale description to the documentation.

I don't really know what I'm doing with the GNOME prefs.  It looks like it works fine, but can somebody please check the finer points such as default/mandatory preferences?

Also: I noticed the existing documentation says that the Terminal bell option should be enabled to disable the bell, which is the opposite of its current purpose (at least as far as the UI is concerned).  I'd be happy to make a patch if you like.
Comment 14 Mikel Ward 2008-11-10 11:12:16 UTC
The wording for the preferences UI might not be ideal.

Some other possibilities:
1. _Highlight the tab/window if a terminal if it emits a bell and the tab/window is unfocused
2. _Highlight the tab/window if a terminal if it emits a bell it doesn't have the input focus
3. _Highlight a background terminal's tab/window if emits a bell
4. _Highlight unfocused terminal on bell

Right now I prefer #3.
Comment 15 Christian Persch 2008-11-10 13:04:35 UTC
(In reply to comment #13)
> Also: I noticed the existing documentation says that the Terminal bell option
> should be enabled to disable the bell, which is the opposite of its current
> purpose (at least as far as the UI is concerned).  I'd be happy to make a patch
> if you like.

Yes, please file a separate bug for that to correct the documentation.
Comment 16 Mariano Suárez-Alvarez 2008-11-11 03:39:04 UTC
Having the word "focus" on anything but window manager-related specs is quite suboptimal ;-)

Shouldn't this be done UI-wise somewhere in the System -> Preferences -> Look and Feel -> Window capplet, with a "Visually mark windows and tabs where activity has occurred" labeled checkbox, and have the terminal simply set the urgency hint upon activity unconditionally, letting the wm be the one who actually checks the preference and decides whether to do something to alert the user? (Maybe  with a non-UI gconf key so that `expert' users can make g-t disregard the system-side setting, if really, really needed... In extremis, this could be a profile setting) There are other apps where action happens, and putting UI in all of them is, hmm, redundant...

Comment 17 Mikel Ward 2008-11-11 07:37:20 UTC
Pidgin and urxvt already do this explicitly on their own.

If you handle it at the Metacity level, then you also have to co-ordinate this with XFCE, KWin, etc, which will take time.

If you want to work towards your larger proposal, that's great, but is there some reason we can't implement my patch?
Comment 18 Mikel Ward 2008-11-18 11:35:07 UTC
Is there anything else I need to do to get this accepted?
Comment 19 Christian Persch 2008-12-16 13:41:38 UTC
I don't think this patch is taking the right approach, wich the highlight and unhighlight signals and how they're handled.

Here's how I think this should work:
- add a "bell-raised" gobject property on TerminalScreen. The handler of the "beep" signal would set it to TRUE, and focus-in on the screen (ie when this tab is switched to) would set to FALSE again; probably typing would also clear it (so it works if it's the focused tab while beep is emitted)
- the TerminalTabLabel would bold itself according to this property
- the TerminalWindow would listen to notify::bell-raised on all its screens, and set the urgency hint iff the pref is enabled and the window does not have focus
- focus-in on the window would clear the urgency hint but leave the tab label bolding intact, except on the focused tab

A patch along these lines would be acceptable to me.
Comment 20 Mikel Ward 2008-12-17 12:26:23 UTC
I'm not sure I like the idea of it bolding the current tab.

I have my shell echoing a bell in the prompt, and making the current tab bold all the time will probably be distracting.  (It would make me want to type something to clear the bold label, just to make things "clean", even if I had nothing to consciously do in that tab right now.)
Comment 21 Raphael Hertzog 2009-04-23 15:50:41 UTC
It seems to me that the current patch is better than nothing. It would be nice to have this feature in the not-too-distant future. It's important for many users to be able to set the urgent hint on their terminal by the way of the bell.

For example, if you use irssi and want to be notified that there's something new in the window. Or if you start some long-running process and you want to be informed when it's finished.
Comment 22 Christian Persch 2009-04-28 13:01:07 UTC
You're wrong. What we have here is an incorrect patch, without anyone committed to fixing its problems. Just committing it as-is is out of the question.

If you want this feature, how about providing an updated patch?
Comment 23 Mikel Ward 2009-04-28 14:12:01 UTC
Christian, I've actually just started getting my dev environment set up again for GNOME 2.26 and will try to make the time to write another patch using your design, but it will take a while.
Comment 24 Mikel Ward 2009-05-02 01:41:59 UTC
Created attachment 133774 [details] [review]
work-in-progress using Christian's design, still needs prefs, docs

Here's an initial implementation based on Christian's suggestion.

Looking at the code, it's obvious that Christian has laid a lot of ground work (e.g. terminal_tab_label_set_bold).  Thanks very much for that!

We still need to decide what to do if the current tab is focused.

My requirements:
- current tab is not bolded if a bell is emitted
  * based on the idea that putting a bell (ASCII 007) in your prompt
    so you start a long-running command, go away and do something else,
    (i.e. switch to another application window)
    and GNOME Terminal will bring attention to itself when the command
    has finished
  * best way I could find was to put a test in the tab title bell raised
    callback that tests if the screen that raised the event has focus

Christian's requirements:
- current tab is bolded, pressing a key clears the bell
  * presumably this is useful as a kind of visual bell,
    e.g. if syslog prints a message to console
  * added a callback for key-press-event
    does this make things slower (it would be a very frequent event!)?
  * can be enabled by changing FALSE to TRUE in terminal-tab-label.c line 107
    and re-compiling (if Christian is happy with this approach, I will make it
    a pref)

What does everyone think?
Comment 25 Mikel Ward 2009-05-03 11:52:11 UTC
Created attachment 133859 [details] [review]
work-in-progress using Christian's design, with some prefs and docs

Here's a minor update with all the debugging stuff stripped out and the old pref and docs put back in.

I'd be happy to get some suggestions on how to do the prefs to keep everyone happy.

Here's two ideas as a starting point:

Proposal 1
[ ] Bold the tab title and flash then window icon when the bell rings
  [ ] Even if the tab has focus

Proposal 2
When the bell rings
  [ ] Make a sound/flash the window
  [ ] Bold the tab title and flash the window icon
     [ ] Bold the tab title even if the tab has focus

Proposal 1 seems easier.

Proposal 2 aims to integrate with the existing "Terminal bell" preference.  The first checkbox would replace it.

The "Even if the tab has focus" text would need a note stating that the window's urgency hint will never be set if the window has the focus.

The "Make a sound/flash the window" text and setting depend on the value set under System->Preferences->Sound->Sounds->Visual Alert.  In my Ubuntu 9.04 setup, it doesn't even seem to make a sound, so perhaps the text should change to "Flash the window or screen".

I would concede changing the text from "Bold the tab title even if the tab has focus" to "Only bold the tab title if the tab isn't focused" and inverting the logic if you prefer.
Comment 26 Mikel Ward 2009-05-03 14:05:13 UTC
Created attachment 133862 [details] [review]
ready-for-review patch based on Christian's design

Please take a look at this one.  It should meet both our requirements.

Minor points:
- The *HIGHLIGHT_FOCUSED_ON_BELL names are getting a bit long, but I think it's right
- The wording I came up with for the prefs was:
  [ ] Flash window _icon and bold tab title on bell
  [ ] ...even if I am working in that tab and window

  The obvious inverse form is:
  [ ] Flash window _icon and bold tab title on bell
  [ ] ...only if I am not working in that tab (or window)

  Either seems fine to me.
- Focused windows are now also made urgent if that pref is enabled
  This seems logically consistent and seems to work OK
- Maybe it's worth also clearing the bell if the user clicks in the window
  and the window is already focused (in that case, we don't get the focus event)

Looking forward to your feedback.
Comment 27 Mikel Ward 2009-05-15 05:53:12 UTC
Created attachment 134686 [details] [review]
added code to display a DBUS desktop notification popup on bell (apply on top of previous patch)

This code makes GNOME Terminal show a popup desktop notification bubble when the terminal emits a bell.  The notification will be: title="Terminal", body=<raw window title>

Adds a dependency on libnotify (http://www.galago-project.org/news/index.php).

Tested on Ubuntu 8.10 with vte 0.20.1.

Patch is against GNOME Terminal r3419.
Comment 28 Mikel Ward 2009-05-31 08:14:33 UTC
I have been using the latest patch for two weeks on Ubuntu 9.04 with vte 0.20.1 and haven't seen any problems.

I'm hoping you will have a look at this one soon.
Comment 29 Christian Persch 2009-06-01 19:07:11 UTC
Reviewing attachment 133862 [details] [review]:

Please split this into several parts: a) implementing the bell-raised prop on TerminalScreen, b) reacting to the prop in the tab label, c) doing the window flashing in terminal-window. 

+  g_signal_connect (screen, "focus",
+                    G_CALLBACK (terminal_screen_focus_cb),
+                    NULL);
+  g_signal_connect (screen, "focus-in-event",
+                    G_CALLBACK (terminal_screen_focus_in_event_cb),
+                    NULL);

+terminal_screen_focus_cb (TerminalScreen *screen)
+  g_object_set (screen, "bell-raised", FALSE, NULL);
+
+terminal_screen_focus_in_event_cb (TerminalScreen *screen)
+  g_object_set (screen, "bell-raised", FALSE, NULL);

I don't think focus should unset the bell-raised property. E.g. considering that this will also clear the indicator simply by switching to anther tab by way of passing over the one with the raised bell using switch left/right  keybinding. Maybe it should persist until the next keyboard input to the terminal? Or any better ideas?

+  g_object_set (screen, "bell-raised", FALSE, NULL);

In the implementation here, don't call g_object_set, but terminal_screen_set_bell_raised directly. And make terminal_screen_set_bell_raised a private func (static void).

+gboolean
+terminal_screen_beep_cb (TerminalScreen *screen)
[etc]

Make this static, and move it up before it's used so you don't have to add an unnecessary prototype.

+  g_signal_connect (screen, "beep",
+                    G_CALLBACK (terminal_screen_beep_cb),
+                    NULL);

+gboolean
+terminal_screen_beep_cb (TerminalScreen *screen)

Use the class vfunc slots here on VteTerminalClass instead of signal connections.

+gboolean
+terminal_screen_key_press_event_cb (TerminalScreen *screen)

Same.

Init priv->bell_raised to FALSE in terminal_screen_init.

+static void
+screen_bell_raised_tab_label_cb (TerminalScreen *screen,
+                                 GParamSpec *pspec,
+                                 TerminalTabLabel *tab_label)
+{
+  gboolean bell_raised;
+
+  g_object_get (screen, "bell-raised", &bell_raised, NULL);

Use terminal_screen_get_bell_raised here.

+  if (bell_raised)
+    {
+      gboolean screen_has_focus;
+      gboolean highlight_focused;
+
+      g_object_get (screen, "has-focus", &screen_has_focus, NULL);
+      highlight_focused = terminal_profile_get_property_boolean (
+          terminal_screen_get_profile (screen),
+          TERMINAL_PROFILE_HIGHLIGHT_FOCUSED_ON_BELL);
+
+      if (!screen_has_focus || highlight_focused)
+        terminal_tab_label_set_bold (tab_label, TRUE);
+    }
+  else
+    terminal_tab_label_set_bold (tab_label, FALSE);

I don't think we should distinguish between terminals with focus and those without, here.

+  g_signal_connect (priv->screen, "notify::bell-raised",
+                    G_CALLBACK (screen_bell_raised_tab_label_cb), tab_label);

You need to add a call to screen_bell_raised_tab_label_cb before this so the prop gets synced initially. And rename this in line with sync_tab_label above. And move it up before the connection to the close button, to right below the sync_tab_label handling.

+  void (* bell_raised_handler)(TerminalScreen *screen);

Remove this.

+void terminal_screen_set_bell_raised (TerminalScreen *screen,
+                                          gboolean raised);

And this.

+  PROP_HIGHLIGHT_ON_BELL,
+  PROP_HIGHLIGHT_FOCUSED_ON_BELL,
[etc]

I don't see the point of these 2 prefs. Nor can I figure out from their schema descriptions why they're two instead of just one. 

Plus, I don't think we should do the urgency hint thing at all (comment 1).
Comment 30 Christian Persch 2009-06-01 19:19:41 UTC
Reviewing attachment 134686 [details] [review]:

I don't really see the point of also notifying of this with a panel notification, but anyway here are my comments on the code:

+   libnotify >= $LIBNOTIFY_REQUIRED])

This will need to be an optional dependency, disabled by default.

+  PROP_NOTIFY_ON_BELL,
+  PROP_NOTIFY_FOCUSED_ON_BELL,

Again, I don't see the point of any prefs beyond a global "enable notifications for bell raised" pref (not per-profile), nor why these 2 instead of just one pref.

 gboolean
 terminal_screen_beep_cb (TerminalScreen *screen)
 {
+  gboolean should_notify;
+
   g_object_set (screen, "bell-raised", TRUE, NULL);
+
+  should_notify = terminal_profile_get_property_boolean (
+                      terminal_screen_get_profile (screen),
+                      TERMINAL_PROFILE_NOTIFY_ON_BELL);
[etc etc etc]

This code does not belong into TerminalScreen. It belongs either into TerminalWindow, or even TerminalApp.
Comment 31 Mikel Ward 2009-06-02 03:28:59 UTC
"I don't think we should do the urgency hint thing at all (comment 1)."

I understand you don't want it.

I do, and others do (https://bugs.launchpad.net/ubuntu/+source/gnome-terminal/+bug/272749), and the feature is available in Eterm, Konsole, urxvt, and xterm (and even PuTTY on Windows).  Clearly there is a demand for it.

I've put a video of my patch in action at http://mikelward.com/software/gnome/terminal-demo.ogv.  Please have a look.  Ignore the notifications for the time being and focus on the flashing window list icon.

I'd like to make sure you understand this use case before we discuss the implementation (which I'm open to changing).
Comment 32 Mikel Ward 2009-06-23 21:55:35 UTC
Have you had a chance to look at the video yet?
http://mikelward.com/software/gnome/terminal-demo.ogv
Comment 33 Alexander S 2009-07-10 22:08:41 UTC
*** Bug 588283 has been marked as a duplicate of this bug. ***
Comment 34 Mikel Ward 2009-08-20 01:18:42 UTC
Re comment 30:
"I don't really see the point of also notifying of this with a panel
notification, but anyway here are my comments on the code"

You're right.  I'm happy to drop that one and just focus on the urgency hint.

"I don't see the point of any prefs beyond a global "enable notifications
for bell raised" pref (not per-profile)"

So you mean the "Profile Preferences" dialog in fact edits profile preferences and global preferences, and changing a preference for one profile affects all profiles?  Or is there a separate UI for global prefs?

"nor why these 2 instead of just one pref"

There are two different use cases, as explained previously:
1) to replace a visual or audible bell (NOTIFY, NOTIFY_FOCUSED)
2) to notify the user when a command in a background window has completed (NOTIFY, !NOTIFY_FOCUSED)

Re comment 29:
"I don't think focus should unset the bell-raised property. E.g. considering
that this will also clear the indicator simply by switching to anther tab by
way of passing over the one with the raised bell using switch left/right 
keybinding. Maybe it should persist until the next keyboard input to the
terminal? Or any better ideas?"

I can't think of any better ideas at the moment.  I'd rather have it cleared when I do Ctrl+PgUp multiple times than still have it highlighted after focusing the tab.  This is how I've been using it for months, and I find it acceptable.

I guess I'll start by doing part a (implementing the bell-raised prop on
TerminalScreen), since that's probably the only bit we've got agreement thus far.
Comment 35 Mikel Ward 2009-10-10 00:23:45 UTC
Created attachment 145190 [details] [review]
patch to implement the bell raised property

First of four patches: this one implements the bell raised property on Terminal Screen.
Comment 36 Mikel Ward 2009-10-10 00:26:22 UTC
Created attachment 145191 [details] [review]
patch to implement two preferences, required by other patches in this series

Patch two of four: adds two preferences that the Window (patch three) and Tab Label (patch four) check to see whether to make the window urgent or embolden the tab title.

The exact form of these preferences is yet to be agreed on.
Comment 37 Mikel Ward 2009-10-10 00:27:18 UTC
Created attachment 145192 [details] [review]
patch to make the window set the urgency hint when a bell is emitted

Patch three of four: makes the window urgent if a bell is emitted according to preferences from patch two.
Comment 38 Mikel Ward 2009-10-10 00:32:31 UTC
Created attachment 145193 [details] [review]
patch to make the tab's title bold if a bell is emitted

Patch four of four: makes the tab's title bold when a bell is emitted according to preferences from patch two.
Comment 39 Mikel Ward 2009-10-10 00:42:32 UTC
I've split the patches as requested.

I've tried to implement the implementation changes.

The only thing I haven't done is move g_signal_connect ("beep") to a class vfunc signal handler.  At first glance I can't see how to do that: the beep signal is emitted by VteTerminalClass, but we don't own that, so I can't see how to add the signal handler there.

We obviously still need to have further discussions on the design however.

I'm guessing one way is to add more preferences so that the urgency hint isn't linked to the bold title, but I think we'd need four preferences to keep both of us happy, which seems a bit excessive to me.

I don't see any evidence you've considered my actual use case, so please do have a look at the video I linked to (ignoring the desktop notification in the top right).  I would also like you to elaborate on the use case you had in mind.  Hopefully we can find a way forward.

Thanks
Comment 40 Mikel Ward 2009-10-10 00:55:44 UTC
Actually, four preferences would at least keep both of us happy.

[ ] Flash the window icon when the terminal rings the bell
    ( ) Unfocused windows  ( ) All windows

[ ] Bold the tab title when the terminal rings the bell
    ( ) Unfocused tabs     ( ) All tabs

I would use

[x] Flash
    (*) Unfocused
[x] Bold
    (*) Unfocused

I assume you are thinking of

[ ] Flash
[x] Bold
            (*) All

We would also have to make terminal_screen_focus_cb not clear bell-raised if the Bold/All preference is true.
Comment 41 Mikel Ward 2009-10-10 01:13:12 UTC
Out of interest, is there much overhead caused by a callback that fires on every key press?
Comment 42 Mariano Suárez-Alvarez 2009-10-12 03:19:37 UTC
This'll converge into an embedded python interpreter so the the user configure behaviour (<schema><key>/schemas/apps/gnome-terminal/global/python_string</key><type>string</type><short>Python script to configure terminal behaviour</short> &c)...

More seriously: I wonder if it would be too complicated to come up with python bindings for TerminalScreen, TerminalWindow and friends so that power users can just easily script new apps that do precisely what they want (a terminal.Window class parametrized over a class inheriting from terminal.Screen, and a terminal.Screen class with a on_activity() member function which gets called on activity, for subclasses to override, and so on, so that one can say

    import terminal
    import pynotify
    import smtplib
    from email.mime.text import MIMEText

    class MyTerminalScreen(terminal.Screen):
        # override the default, do-nothing on_activity member of terminal.Screen
        # and do something power-user-y
        def on_activity(self): 
            if not self.focused():
                self.label.set_markup("<b>%s</b>" % /self.title)
                self.containing_window.set_urgency_hint(True)
                n = pynotify.Notification('Danger, Will Robinson!', 'Something moved!', "dialog-warning")
                n.set_urgency(pynotify.URGENCY_NORMAL)
                n.set_timeout(pynotify.EXPIRES_NEVER)
                n.add_action("clicked","Button text", callback_function, None)
                n.show()
                msg = MIMEText('Activity has occurred!')
                msg['Subject'] = 'Attention!'
                msg['From'] = 'gnome-terminal@out.there.com'
                msg['To'] = 'power-user@out.there.com'
                s = smtplib.SMTP()
                s.sendmail(msg['From'], [msg['To']], msg.as_string())
                s.quit()

    class MyTerminalApp(terminal.App):
        screen_class = MyTerminalScreen
        def __init__(self):
            terminal.App.__init__(self)
            pynotify.init('GNOME Terminal')
        
    app = MyTerminalApp()
    app.run()


), instead of making one app do everything...
Comment 43 Mikel Ward 2010-03-09 05:45:12 UTC
> +  g_signal_connect (screen, "beep",
> +                    G_CALLBACK (terminal_screen_beep_cb),
> +                    NULL);
>
> +gboolean
> +terminal_screen_beep_cb (TerminalScreen *screen)
> 
> Use the class vfunc slots here on VteTerminalClass instead of signal
> connections.

Ah, you mean these in vte.c around line 11204 and 11226:

        widget_class->focus_in_event = vte_terminal_focus_in;

        klass->beep = NULL;

beep is fine, I can do:

  terminal_class->beep = terminal_screen_beep_cb;

in terminal_screen_class_init.

But I don't think it's right to change focus_in_event (or indeed enter_notify_event, which might be better), since vte has already assigned those to something.

Is there something I'm missing?

And why do you prefer the signal slots?
Comment 44 Christian Persch 2010-03-09 22:14:08 UTC
If there's already a class handler for the signal, you can just *add* the code in there, no need for a 2nd handler.
Comment 45 Behdad Esfahbod 2010-03-10 05:32:15 UTC
(In reply to comment #44)
> If there's already a class handler for the signal, you can just *add* the code
> in there, no need for a 2nd handler.

I think what Mikel is saying is that *vte* already assigns a method to focus_in_event, so *g-t* cannot blindly override that, and I assume the code in question doesn't belong to vte?
Comment 46 Christian Persch 2010-03-10 11:28:32 UTC
In that case there's no problem either, since TerminalScreen inherits from VteTerminal, so you can just chain up (in fact, *have* to chain up) in the handler.
Comment 47 Behdad Esfahbod 2010-04-14 20:11:35 UTC
ChPe, feel like fixing and committing this?  Useful feature.
Comment 48 Mikel Ward 2010-04-15 12:01:56 UTC
Christian, it seems the parent's handler function has internal linkage, so it's not obvious how I can call that from TerminalScreen.

e.g. following the example of maman_bar_constructor at <http://library.gnome.org/devel/gobject/unstable/chapter-gobject.html>

static gboolean
vte_terminal_focus_in (GtkWidget *widget, GdkEventFocus *event)
{
  VteTerminalClass *terminal = VTE_TERMINAL_CLASS (widget);
  terminal->vte_terminal_focus_in (widget, event);

  /* my code here */
}

won't work:
terminal-screen.c:737: error: ‘VteTerminal’ has no member named ‘vte_terminal_focus_in’

What am I missing?
Comment 49 Mikel Ward 2010-04-15 12:05:44 UTC
Whoops, sorry, copy/paste error.  Function signature above should have been:

gboolean
terminal_screen_focus_in_event_cb (GtkWidget *widget, GdkEventFocus *event)

(possibly with a static modifier).
Comment 50 Behdad Esfahbod 2010-04-15 15:08:06 UTC
In your class_init function, save the widget_class->focus_in_event, and chain to it later on.  Something like that.
Comment 51 Mikel Ward 2010-04-15 21:13:47 UTC
Yep, thanks Behdad.  Wanted to make sure I wasn't missing something easier and cleaner.

Why do we go to all this trouble when we could do g_signal_connect instead?
Comment 52 Mikel Ward 2010-04-15 21:31:34 UTC
Anyway, happy to have another look on the weekend and do it that way.  Just interested to know the reason.
Comment 53 Behdad Esfahbod 2010-04-15 22:14:09 UTC
In general signals are to attach functions to individual object instances.  When doing that on a class level, the right way is to set the class method.  It's much more memory efficient and faster.
Comment 54 Behdad Esfahbod 2010-04-15 22:15:56 UTC
Actually it's much easier than what I wrote above.  To chain up, you can call:

  GTK_WIDGET_CLASS (terminal_screen_parent_class)->focus_in_event

Easier now?
Comment 55 Mikel Ward 2010-04-16 03:55:51 UTC
Awesome, thanks, that's exactly the kind of thing I was hoping I could do.

Will re-do patches this weekend.
Comment 56 Mikel Ward 2010-04-17 02:41:25 UTC
Created attachment 158933 [details] [review]
draft combined patch

First pass of patch to address problems identified by Christian.

Against revision 3436.

Will migrate from svn to git tomorrow and respin as three separate patches as requested.
Comment 57 Mikel Ward 2010-04-17 03:02:43 UTC
Created attachment 158934 [details] [review]
ditto but with fixed "even if the window" label

Fixed label for "even if the window" and added help for urgent window prefs.
Comment 58 Mikel Ward 2010-04-17 03:27:05 UTC
Use cases for setting the terminal window urgency hint:

1. Notify when command has completed in background window
   <http://mikelward.com/software/gnome/terminal-bell-demo2.ogv>

2. Better visual bell
   <http://mikelward.com/software/gnome/terminal-bell-demo4.ogv>

3. Why checking whether the window has focus is useful
   <http://mikelward.com/software/gnome/terminal-bell-demo3.ogv>

(if we removed the has-focus check, it would be equivalent to the "even if I'm working in that window" option being permanently enabled, and that would be annoying in use case 1)
Comment 59 Behdad Esfahbod 2010-04-20 19:00:03 UTC
We should notification bubbles while talking about this.  Someone should check what konsole does.
Comment 60 Mikel Ward 2010-04-20 21:23:50 UTC
Behdad, comment 27 (attachment 134686 [details] [review]) was my first attempt at notification bubbles.

It used libnotify and set the caption to the name of the terminal.  The intention was that in the ideal case, the user or application would change the title to something meaningful, e.g. "longcommand finished" -- doing so in zsh should be possible using precmd/preexec for example -- and it the degenerate case, it's still enough information to figure out which window to go to to see what happened.

Another possibility is by listening for an escape sequence like ESC ]0 or ESC k.

IIRC, Konsole does something similar to #1.

I would also point you to <http://mikelward.com/software/gnome/terminal-demo.ogv>, but my computer refuses to play .ogv videos at the moment, so I can't be sure it's the one I want to show you.

Once we figure out the approach, I should be able to work on it after doing the urgent and bell stuff.
Comment 61 Mikel Ward 2010-05-18 03:28:09 UTC
Created attachment 161305 [details] [review]
patch against git HEAD to add a property called "bell-raised"

Respun against git HEAD.  Also removed terminal_screen_set_bell_raised prototype.
Comment 62 Mikel Ward 2010-05-18 03:30:19 UTC
Created attachment 161306 [details] [review]
patch against git HEAD to make the tab title bold, includes prefs and docs

Respun against git HEAD.  This time this patch includes the prefs and docs rather than them being in a separate patch.
Comment 63 Mikel Ward 2010-05-18 03:32:46 UTC
Created attachment 161307 [details] [review]
patch against git HEAD to set the window urgency hint, includes prefs and docs

Respun against git HEAD.  Includes prefs and docs.
Comment 64 Mikel Ward 2010-05-18 03:44:38 UTC
Above patches should be applied using git apply.

It complains about trailing whitespace in help/C/gnome-terminal.xml.  I copied the existing style, which also has trailing whitespace.  Happy to change it.
Comment 65 Ben 2010-07-30 02:06:05 UTC
Hi All

Is there anything further that needs to be done with this before committing to trunk ? It seems to have stalled somewhat after Mikel submitted his patches.

Thanks;
Ben
Comment 66 Mikel Ward 2010-09-05 22:25:00 UTC
Christian, have you had time to take a look?

Thanks
Comment 67 Behdad Esfahbod 2010-09-21 00:24:44 UTC
ChPe, can you shed some light on what may be the next steps for this bug?
Comment 68 Christian Persch 2010-09-28 13:50:09 UTC
Comment on attachment 161305 [details] [review]
patch against git HEAD to add a property called "bell-raised"

Repeating what I've said in comment 29, I don't think focus-in should unset the property. Please split this out of this patch.

Also, the property needs to be reset when the child exists (terminal_screen_child_exited).

+     g_param_spec_boolean ("bell-raised", NULL, NULL,

Also split terminal_screen_key_press
() addition into a separate patch. *This* patch should only implement the property.
Use I_() for the prop name.
Comment 69 Mike 2011-02-06 07:41:33 UTC
I just discovered application monitoring in screen, but I was disappointed that it wouldn't let me do other things while waiting for activity/silence.

I'm impressed to see that this bug has been moving forward for many years, but it seems like a simple feature to add. Can we just get it pushed out? What pieces are missing to make this happen? 

Thanks for the hard work and dogged persistence on this feature!
Comment 70 Morten 2011-03-21 22:21:00 UTC
Applied a subset of the patch and tested. Works. Is this hanging on Mikel responding to ChPe? In the new Ubuntu Unity desktop the URGENT hint doesn't make that much of a deal of itself. Just a short wobble of the terminal icon for the first bell the window gets, and none after.

This is basically just a «Bump, I want this!» :)
Comment 71 Morten 2011-03-22 01:27:18 UTC
(In reply to comment #70)
> In the new Ubuntu Unity desktop the URGENT hint doesn't
> make that much of a deal of itself. Just a short wobble of the terminal icon
> for the first bell the window gets, and none after.

Oh, and the little arrow next to the window turns blue, and stays blue. Awesome =)
Comment 72 Yury V. Zaytsev 2011-05-18 12:24:12 UTC
Hi guys!

I'm all confused now. Which patches exactly do I have to apply to get the functionality? I can build a package for Ubuntu and put it into my PPA for everyone to use.

I work extensively on a cluster and I need to monitor the activity in my terminal windows, so I would highly appreciate if this would move forward.

Z.
Comment 73 Morten 2011-05-18 17:10:53 UTC
I made a PPA for this. It doesn't have the entire patch, the configuration options have been stripped. It just sets URGENT for all bells on non-active windows.

https://launchpad.net/~morethan/+archive/ppa
Comment 74 Yury V. Zaytsev 2011-09-23 11:57:57 UTC
FYI, I backported this patch to gnome-terminal shipping with Maverick and the package is available in my PPA:

https://launchpad.net/~zyv/+archive/ppa

Mikel, you're a genius!
Comment 75 Christian Persch 2013-03-10 17:21:16 UTC
*** Bug 641982 has been marked as a duplicate of this bug. ***
Comment 76 Christian Persch 2013-03-10 17:21:23 UTC
*** Bug 694807 has been marked as a duplicate of this bug. ***
Comment 77 Yury V. Zaytsev 2013-03-10 17:32:31 UTC
Hi Christian,

Any chance of getting this merged into gnome-terminal any time soon? I'm still hand-patching it and that's certainly less then an optimal solution. What needs to be done on the patch to let this happen?

Z.
Comment 78 Torkild Retvedt 2013-05-29 13:02:23 UTC
Hi,

Just wanted to add that I stumbled upon the patch Mikel wrote, while trying to figure out how to enable notifications in the terminal.

As Yury, I'm currently hand patching this, and just wanted to add that I think it would be of great value if this were to be merged upstream.

Am I correct to say that the reason this patch has not been merged is because there's a difference of opinion when it comes to the implementation design (as opposed to whether or not the feature is really needed)?

Are the comments from a few years back still valid?
Comment 79 Morten 2014-05-26 08:45:21 UTC
I am still applying this patch every time a new version of gnome-terminal hits Debian. Are there any unanswered questions here, any reason why this isn't being accepted?
Comment 80 Ben Gamari 2015-08-10 15:05:06 UTC
It would be great if we could merge this.
Comment 81 Michael Heyns 2015-12-10 04:46:06 UTC
This is a very useful feature to have. It has my vote.
Comment 82 nh2 2016-10-25 14:54:40 UTC
Can anybody indicate who is blocking this to land and for what reason?
Comment 83 Alex 2017-04-03 06:44:33 UTC
Are there any updates on this issue? It's been dragging along for some time now. A life sign from the maintainer(s) would be much appreciated.
Comment 84 Anis ELLEUCH 2017-12-06 13:24:23 UTC
I vote for this.

I am running irssi inside tmux inside an ssh login in a gnome-terminal :)

I am able to hear a beep when I receive a message, it would be great to enable urgent hint so I can see a notification when I don't use my headphones.
Comment 85 GNOME Infrastructure Team 2021-06-10 19:47:41 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/6698.