#2131 Bindings with ctrl not working on qt, wxt at Ubuntu

[Martin Saturka]

Bindings with "ctrl" mostly do not work on qt, wxt terminals (at Linux, XFCE). It is just a few ctrl-something combinations that work for me. While the registartion works (and it is listed at show bind), it does not fire on the key presses.

Notice that:
it works for me when I use x11 terminal,
it is for the most of ctrl-key combinations, i.e. they are not eaten by desktop environment (even though it eats some ctrl-key cases).
* bindings with just alt seem to work (though not when set with both ctrl and alt).

It seems to be alike the #1670 bug ticket.
I compiled gnuplot 5.2.6 with
TERMLIBS="-lX11" ./configure --with-x
setting, and it seems to be the same beaviour as with distribution-based gnuplot.
I use Xubuntu 16.04 (Ubuntu Linux + XFCE), 64b.

[Martin Saturka]

I've debugged it a bit, and the reason why it does not work for me (and according to one previous bug report not only for me) is that the .text() method on QKeyEvent returns small numbers for chars. It is alike this:
https://bugs.launchpad.net/ubuntu/+source/qtubuntu/+bug/1664610

I have the case with "3" in .text() for ctrl+c, and alike for other cases.
Thus the character value that is sent via socket (from the qt part) to gnuplot core contains very small ascii values for the most of keys (unless the ascii value of the key is small enough).

The .key() method seems to be correct for me (at least at a few cases that I've checked), even though it does not distinguish lower/upper cases. I would say that still better than the current state.

May be a checking whether result from .text() is lesser than .key(), or just whthere they mismatch, and in such cases to use the .key() value. This would still lose the lower/upper case distinction in my case, though it should not destroy situations where .text() is correct.

Notice that without ctrl pressed, I get correct values in .text(); this is just ctrl-related stuff.

[Martin Saturka]

Added patch restores ctrl+az keys under qt for me, even though it treats ctrl-A as ctrl-a (i.e. it neglects case) where it would be broken otherwise.

[Martin Saturka]

in wxterminal/wxt_gui.cpp you have:

/* ctrl+q does not send 113 but 17 */
/* WARNING : may be the same for other combinations */

thus you know about the broken behavior that I described above, and you've just ignored it, ouch :-(

[Martin Saturka]

Here you have an analogical patch for wxt terminal that solves it for me alike that one above for qt.

[Ethan Merritt]

Seems to work, but could you help me understand why this does not convert <tab> to "i"and <cr> to "m" and so on? Which of the logical tests prevents that?</cr></tab>

[Martin Saturka]

While the qt-terminal patch is safe on that, the wx-terminal patch fails on the tab-vs-i etc. collisions, ouch. It is due to wx not providing a unified cross-platform way for dealing with it.

I've tried to deal with the wx-wise collisions via a hack at a new version of the wx-terminal patch, though it needs testings on other platforms. It is at the #773 patches ticket. Could we move there? The patches there should be nicer than those ones here.

A quick summary:

• Event at qt-terminal has correct key() method that just does not distinguish lower from upper cases. The text() method has correct cases for a-z/A-Z chars when it works, though it completely fails under some situations.

• Event at wx-terminal lacks equivalent to the key() method that would be correct (modulo char cases) always. Thus either losing tab-for-i or i-for-tab, etc. with ctrl pressed, or using platform dependent methods (and that need testings under all supported platforms).

[Martin Saturka]

To avoid a mutual diverging, I meant that the patches with (hoping that) self-explanatory comments are at:
https://sourceforge.net/p/gnuplot/patches/773/

[Martin Saturka]

BTW Do the ctrl-key bindings work on other interactive terminals, like the "win" one?
When we try to fight the ctrl stuff here, it would be probably reasonable to check that too.

[Ethan Merritt]

From the Qt documentation:

QString QKeyEvent::text() const
Returns the Unicode text that this key generated.
Return values when modifier keys such as Shift, Control, Alt, and Meta are pressed differ among platforms and could return an empty string.

I think this means the "ASCII keys" section of the original gnuplot qt terminal code is wrong to return event->text(). It should just return event->key() and let the event_keypress() code in mouse.c sort out whether it is upper or lower case. Unfortunately the code in mouse.c looks wrong also. Maybe I need more coffee, but isn't the test at mouse.c:1452 basically a no-op? (c & 0xff) will never be zero.

    if ((modifier_mask & Mod_Shift) && ((c & 0xff) == 0))
        c = toupper(c);

But any change there will affect all terminals, not just Qt. So I need to think some more.

[Martin Saturka]

Since it seems to be something more general than this issue, I've created a ticket for it:
https://sourceforge.net/p/gnuplot/feature-requests/487/

[Ethan Merritt]

Revised qt patches look OK to me.

I will re-work the wxt patch so that event.GetRawKeyCode() is only invoked inside
#ifdef wxHAS_RAW_KEY_CODES
as reccomended by the wxWidgets docs. Platforms without that will give the wrong result, but no worse than the current code.

I will only apply these to 5.3 for now so that there is time for people to test and report any issues on non-linux platforms.

Thanks for looking into this.

[Martin Saturka]

Thanks! I do not need it immediately, thus 5.3 if OK for me. And I understand that the changes may have some (for us) hidden effects, thus it is better to let them be tested for a longer time.

Regarding wxt, I would just suggest to let the blacklist-based way in where wxHAS_RAW_KEY_CODES is not defined. It should be safe; and the 17->q transition has to be kept there, as it is by now, but you know better than me :-)