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 602726 - visual bell does not exist anymore
visual bell does not exist anymore
Status: RESOLVED OBSOLETE
Product: gnome-terminal
Classification: Core
Component: Profiles
git master
Other Linux
: Normal normal
: ---
Assigned To: GNOME Terminal Maintainers
GNOME Terminal Maintainers
: 586989 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-11-23 13:40 UTC by kerzenmacher
Modified: 2021-06-10 20:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
implement visual bell (5.03 KB, patch)
2014-06-10 10:19 UTC, Egmont Koblinger
committed Details | Review
Add three-state cmdline option to vte apps (4.59 KB, patch)
2014-08-25 21:21 UTC, Egmont Koblinger
committed Details | Review

Description kerzenmacher 2009-11-23 13:40:01 UTC
If I look at a file with 
less .bashrc
and go to the bottom of the file and keep hitting the space key then I get the system beep and not a visual bell. I could get the visual bell in previous versions of gnome-terminal by setting a variable in the profile. Now I can only either have a beep or not, but have not access to a visual bell.
If I put the following line into .inputrc
set bell-style visible 
then I get a visual bell on the command line, but not in the case described above, i.e. gnome-terminal does not pass on the bash command to other commands (here 'less' for example).
Comment 1 Christian Persch 2010-03-17 19:31:01 UTC
Works here, using g-t and vte master, metacity 2.24.
Comment 2 yellow 2010-09-10 20:54:13 UTC
I have same problem here.
GNOME Terminal 2.29.6
metacity 2.30.1
gnome 2.30.2
less 436
vim 7.2.330

~/.inputrc 
set bell-style visible
set prefer-visible-bell

~/.bashrc
set bell-style visible

~/.vimrc
set visualbell

$TERM is set to gnome-256color

visual bell does work in bash (ie, when i press down arrow it flash).
visual bell does not work in either less or vim.

Surprisingly visual bell does work in gvim.

I google around and think that the control sequence for visual bell is not set correctly or something.
( in vim, t_vb=^[[?5h$<100/>^[[?5l  , while in gvim  t_vb=^[|f)

Is that the correct control sequence for visual bell in gnome-terminal ?

What should I do to make visual bell works ?
Comment 3 Christian Persch 2011-06-22 21:10:31 UTC
The 'visual bell' code in vte is broken. It paints to widget->window directly outside of a ::draw event, which won't work.
Comment 4 Christian Persch 2014-03-27 18:52:58 UTC
*** Bug 586989 has been marked as a duplicate of this bug. ***
Comment 5 Egmont Koblinger 2014-06-10 08:37:23 UTC
In current vte master, _vte_terminal_visible_beep() forces a repaint which ideally shouldn't have any user-visible effect.  It repairs display glitches, if any.

There's absolutely no sign of ever painting the canvas in reverse mode.
Comment 6 Egmont Koblinger 2014-06-10 10:19:52 UTC
Created attachment 278197 [details] [review]
implement visual bell

The details in patch are quite ugly, or at least I'm not proud of myself :/  I couldn't figure out a consistent naming of methods (to prefix with _ or _vte or _vte_terminal or not, bell vs. beep and so on).  Not sure if the stop and remove methods could/should be merged into one.

But after all it's working :)
Comment 7 Christian Persch 2014-08-16 18:07:22 UTC
Review of attachment 278197 [details] [review]:

::: src/vte.c
@@ +8758,2 @@
 	/* Reverse cell? */
+        if (cell->attr.reverse != !!terminal->pvt->visible_bell_tag) {

I'd introduce a variable 'gboolean reverse' in this function at the top, set it to pvt->visible_bell_tag != 0, and then us (cell->attr.reverse ^ reverse) here.
Comment 8 Egmont Koblinger 2014-08-16 18:59:27 UTC
What about a g-t config option?  (Esp. with UI freeze in 2 days...)
Comment 9 Christian Persch 2014-08-17 07:36:37 UTC
Ok to add one if you think it's necessary.
Comment 10 Egmont Koblinger 2014-08-25 20:50:38 UTC
Review of attachment 278197 [details] [review]:

::: src/vte.c
@@ +8758,2 @@
 	/* Reverse cell? */
+        if (cell->attr.reverse != !!terminal->pvt->visible_bell_tag) {

Done.
Comment 11 Egmont Koblinger 2014-08-25 20:50:40 UTC
Review of attachment 278197 [details] [review]:

::: src/vte.c
@@ +8758,2 @@
 	/* Reverse cell? */
+        if (cell->attr.reverse != !!terminal->pvt->visible_bell_tag) {

Done.
Comment 12 Egmont Koblinger 2014-08-25 20:58:41 UTC
vteapp defaults to visual bell (currently it's audible XOR visual bell), it's annoying, we could change that to have two separate options, or one with an argument (like --bell=none/audible/visual).

For g-t (3.16) I'm also thinking about a three state checkbox, just like "Cursor shape" which is right above this in the prefs dialog.  Even though the API allows to enable both audible and visual bell at the same time, I doubt anyone would look for that.
Comment 13 Egmont Koblinger 2014-08-25 21:21:50 UTC
Created attachment 284453 [details] [review]
Add three-state cmdline option to vte apps
Comment 14 Christian Persch 2014-08-26 08:57:44 UTC
Comment on attachment 284453 [details] [review]
Add three-state cmdline option to vte apps

>+	vte_terminal_set_audible_bell(terminal, bell_string != NULL && strcmp(bell_string, "audible") == 0);
>+	vte_terminal_set_visible_bell(terminal, bell_string != NULL && strcmp(bell_string, "visual") == 0);

x != NULL && g_str_equal(x, "y").

With that fixed, please commit. Thanks!
Comment 15 Egmont Koblinger 2014-08-26 09:12:45 UTC
(In reply to comment #14)

> x != NULL && g_str_equal(x, "y").

Looks like g_strcmp0() is even better, it does the NULL-check
Comment 16 Egmont Koblinger 2014-08-27 20:27:10 UTC
vte is now okay, reassigning to g-t.
Comment 17 Egmont Koblinger 2014-08-27 21:13:34 UTC
Hmmm, actually we might need some more vte work.

I didn't pay attention to the "set bell-style visible" inputrc setting and similar vim option.

bash's source code reveals that it emits the "vb" termcap capability which is "\E[?5h\E[?5l". This turns on reverse mode for the whole screen and then immediately turns off. Due to vte's buffering, it's extremely unlikely that the reverse mode is actually physically drawn.

vim's "t_vb=^[[?5h$<100/>^[[?5l" suggests that probably vim has its own syntax for specifying a delay (that's what I guess $<100/> means, I haven't verified). This should obviously lead to a visual bell.

Maybe vte could recognize if there's nothing between an "\E[?5h" and the "\E[?5l", and in that case guarantee that the visual bell is rang with its proper delay. Maybe this could be achieved by forcing an immediate screen repaint upon seeing \E[?5h but not on \E[?5l, then we don't need to look for two escape sequences immediately following each other, but the flicker might be too quick. I'm not sure what would be the right thing.

Or should we convert it to audible or no bell according to the user's prefs? But then we handle it equivalently to \a and we just might ask the users to configure audible bell in their apps which g-t will override to visual bell.

Arghhhhhhh...
Comment 18 Egmont Koblinger 2014-08-27 21:22:27 UTC
... or maybe it's still time to revert my patch and decide to drop visual bell support for good, before the new API becomes stable.  It's been broken for ages (maybe never worked?) and very few people complained.

I mean, apps could still do it with \e[?5h-delay-\e[?5l, why do we duplicate this feature?

ChPe?
Comment 19 Christian Persch 2014-08-28 19:09:42 UTC
I think visible bell is implemented wrong in vte. The flashing is annoying (and an a11y issue!), and from what I found in the VT520/VT525 manual (from vt100.net), all the visual bell did there was flash a bell symbol in the status line for 2 seconds (section 2.12.7.1 on page 2-43).

So I think vte should stop providing separate audible and visual bell settings. When it rings the bell, it should just emit a signal that g-t can catch and g-t will then either ring the bell, or show a symbol, or nothing, depending on user pref. (The signal _could_ still have a default handler that, when the user handler doesn't return true, runs and does the beep, for compat with simple vte embedders).
Comment 20 Egmont Koblinger 2014-08-28 19:21:42 UTC
(In reply to comment #19)
> I think visible bell is implemented wrong in vte. The flashing is annoying

it is indeed

> (and an a11y issue!), and from what I found in the VT520/VT525 manual (from
> vt100.net), all the visual bell did there was flash a bell symbol in the status
> line for 2 seconds (section 2.12.7.1 on page 2-43).

I was wondering that maybe it belongs to the window manager or something like that.

> So I think vte should stop providing separate audible and visual bell settings.

Sounds good to me.

> When it rings the bell, it should just emit a signal that g-t can catch and g-t
> will then either ring the bell, or show a symbol, or nothing, depending on user
> pref.

Or it could play some nice ringtone rather than an old-fashioned beep. (Actually I don't know what it's doing now: it never beeps for me and I have no clue why. This is good when using the terminal, but sucks if I'm about to work on this feature. :))

I like this idea of signal, seems the right thing to do.

I'm not sure if we can squeeze this to 0.36 (I probably won't have time to work on this), if you can do this that would be great.  Otherwise we'll do the painful deprecation of set_*_bell in 0.38.
Comment 21 Christian Persch 2014-08-28 19:33:54 UTC
IIRC you need to enable 'effect sounds' in gnome control centre for the bell to actually beep.

No need for deprecation if we can't get it done now, we can just bump API version again.
Comment 22 Egmont Koblinger 2014-08-28 19:38:46 UTC
(In reply to comment #21)
> IIRC you need to enable 'effect sounds' in gnome control centre for the bell to
> actually beep.

Yup I've been there, no change. Nevermind, I'll figure it out one day.

> No need for deprecation if we can't get it done now, we can just bump API
> version again.

I'd rather deprecate than bump just because of this. Of course if we'll have too many changes again then it's okay. We'll see.
Comment 23 Tony Houghton 2014-08-28 22:35:53 UTC
Doesn't vte already have a signal for this? But it's called "beep" instead of "bell". ROXTerm uses it to highlight the tab. Or is beep not quite the same thing as bell? I've got a dim memory of some other bug report involving that signal having the wrong name. Perhaps with the other API changes it might be an opportunity to fix it.

I agree that the other vte bell options should be stripped out and leave it to g-t etc on how/whether to alert the user.
Comment 24 Egmont Koblinger 2014-08-28 22:44:45 UTC
(In reply to comment #23)
> Doesn't vte already have a signal for this? But it's called "beep" instead of
> "bell".

Yup you're right :)

Seems all we need to do is rename this to "bell", make g-t act on this, and remove the rest from vte.
Comment 25 Egmont Koblinger 2014-08-28 22:48:58 UTC
(In reply to comment #21)
> IIRC you need to enable 'effect sounds' in gnome control centre for the bell to
> actually beep.

So, should it play some nice sound, or should it just beep the PC speaker?

None of the terminals beep for me, not even the Linux console.  I'm starting to suspect my laptop doesn't have an old-fashioned PC speaker.

This means that probably I wouldn't be able to test any patch that moves beeping to g-t.
Comment 26 Egmont Koblinger 2014-09-09 13:43:56 UTC
Any conclusion here?

Unless someone objects, in a couple of days I will:

- revert visual bell (it's annoying and a misinterpretation) and remove its API

- rename the signal from beep to bell (comment 23)

I wouldn't touch the audible API (other than renaming the signal) because:

- (comment 19) IMO having a default handler for the signal which is run only if the installed handler returns whatever value is not any simpler than the current API do enable/disable VTE itself beeping.

- I couldn't test such a change on my only computer, not sure why, maybe it doesn't even have a PC speaker
Comment 27 Tony Houghton 2014-09-09 15:26:42 UTC
Sounds good to me. Keeping the existing audible bell option in vte saves me having to implement it in ROXTerm ;-). And I think most terminals that want to beep would want to use some standard system beep so it makes sense to implement it in a library instead of every terminal app. Those that want to do their own fancy beep can do so because the vte one is optional.
Comment 28 Christian Persch 2014-09-09 20:16:34 UTC
Might be a bit late in the cycle to do API changes (the signal name), but otherwise sounds good.
Comment 29 Egmont Koblinger 2014-09-09 20:23:14 UTC
We're breaking the API anyways, and g-t doesn't use that signal.  Removing set_visible_bell() is also another breakage of the API.

Indeed the question is if we're allowed to do this at this stage of API freeze, if the said API hasn't seen a stable release yet.
Comment 30 Egmont Koblinger 2014-09-11 22:48:27 UTC
Done.

(Wiki says "APIs should be frozen", this doesn't sound like a 100% strict freeze.)
Comment 31 Christian Persch 2015-12-15 20:18:14 UTC
I think this is FIXED ?
Comment 32 GNOME Infrastructure Team 2021-06-10 20:03:00 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/6925.