D8464: Add support for DECSCUSR.

[Martijn Hoogendoorn]

mhoogendoorn created this revision.
mhoogendoorn added a reviewer: Konsole.
mhoogendoorn added a project: Konsole.

REVISION SUMMARY
  This is an attempt to fix bug 347323 <https://bugs.kde.org/show_bug.cgi?id=347323>.
  BUG : 347323
  
  It adds support for the DECSCUSR escape code (`ESC [ <number> <space> q`),
  which allows changing of the cursor blinking and shape. This is already
  possible by sending an escape control sequence to change profile settings, but
  this has some downsides:
  
  - It changes the profile to an unnamed one, so "rightclick ; edit current profile" will not edit the profile you expect.
  - It changes the cursor for the whole view. For example, if we have tmux pass the sequence through to konsole, then changing the cursor in the left tmux pane will also change it in the right pane.
  
  The DECSCUSR control sequence is different in that it uses two characters to
  determine the type of command (the space, and q).
  
  A summary of the changes:
  =========================
  
  - Added a new construct called `TY_CSI_DBL` to switch on CSI control sequences with two command characters in `processToken`.
  - Added a character class `CSIDBLF` for characters that can come first in a 2 character command. Found these from a cursory glance through the list of Xterm Control Sequences <https://www.xfree86.org/4.8.0/ctlseqs.html>.
  - in `receiveChar` we check if we might be looking at a CSI command with 2 characters, if so we keep collecting characters.
  
    If we did find a 2 char CSI sequence we send it to `processToken()` with the argument. Note that this one-by-one passing of arguments would not work for all sequences, some get a rectangle as arguments (e.g. `CSI .. $ r`). It is placed before the `CSI m` check so that an incorrect sequence like `ESC [ 1 <space> m` is not interpreted as a `CSI m` sequence.
  - In `processToken()` we add a case for DECSCUSR and set blinking and shape accordingly.
  - added a call to `TerminalDisplay::update()` in `TerminalDisplay::setKeyboardCursorShape()`. This fixes an issue with the display of the cursor not updating (see below). Other `setXXX` functions do updates as well so this seemed a more appropriate place than `Vt102Emulation::processToken()`.
  
  Some problems/questions/remarks:
  ================================
  
  1. It will accept a code with a repeated first char, for example `ESC [ <space> <space> <space> 2 <space> <space> q` will be accepted as if it is just `ESC [ 2 <space> q`. Not sure how big of a problem that would be. Perhaps much stricter checks should be used in `receiveChar()`?
  2. Some defines add empty parentheses, like `epp( )`, so I copied that style. I don't think there is a difference, but I'm not 100% sure.
  3. Earlier in `processToken` multiline cases were indented at the level of the ':', so I did the same.
  4. In both `Vt102Emulation::processToken()` and `ViewManager::applyProfileToView` we have an if block to turn an int into an `Enum::CursorShape`, should this be made into a convenience method on the `Profile` class?
  5. Related, the whole retrieving of the profile defaults in `processToken()` feels a bit messy. We jump through hoops to get to the current profile, suddenly needing to know about `Session`, `SessionController` and `SessionManager`. Perhaps `TerminalDisplay` should have a method to reset to the default cursor?
  
  The cursor redraw problem:
  ==========================
  
  When we execute the following (starting with CursorShape=0, a block, and
  blinking cursor turned off):
  
    printf "hello"; sleep 1; printf "\e]50;CursorShape=1\x7"; sleep 2; printf "\e]50;CursorShape=0\x7\n"
  
  We expect to see "hello" followed by a block cursor, which after 1 second
  changes to an IBeam cursor. After another 2 seconds the cursor shape is reset
  and we are returned to the shell prompt.  This works, and uses the 'change
  profile' escape code. Trying it with the DECSCUSR code, we expect to see the
  same thing:
  
    printf "hello"; sleep 1; printf "\e[6 q"; sleep 2; printf "\e[2 q\n"
  
  However, without an `update()` call in
  `TerminalDisplay::setKeyboardCursorShape()` what we see is "hello" followed by
  a block cursor, which does not turn into an IBeam cursor after 1 second. If we
  make the first sleep short enough (on my pc 0.009s) it seems the cursor change
  gets done together with the printing of "hello" and we do see an IBeam cursor.
  
  I think the change profile sequence did not have that problem because some
  other profile setting getting applied triggered an update. Note that I first
  noticed this in vim. When entering insert mode with just 'i', the cursor does
  not move, and so the shape did not change until something else moved the
  cursor, or triggered a repaint.
  
  Initially I had an `updateCursor()`, and that works for the testcase above, but
  it fails in vim when `showmode` is set. As one of my use cases for this feature
  is vim, I've made it `update()`. On my pc I've not noticed a difference in
  regular use. Not sure how representative of a test it is but using `update()`:
  
    time for i in $(seq 1 10000); do printf '\e[6 q'; printf '\e[2 q'; done
  
  Takes ~0.135s (empty screen) to ~0.350s (screen filled with 'asdf') here
  (fullscreen on 1920x1080, 274x61 terminal size, truetype font, full hinting).

REPOSITORY
  R319 Konsole

REVISION DETAIL
  https://phabricator.kde.org/D8464

AFFECTED FILES
  src/TerminalDisplay.cpp
  src/Vt102Emulation.cpp

To: mhoogendoorn, #konsole
Cc: hindenburg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20171025/522b65d4/attachment.html>