Support extended mouse clicks beyond 223

[egmont]

The ancient way of reporting mouse coordinates only supports coordinates up to 231, so if your terminal is wider (or taller, but that's unlikely), you cannot use your mouse in the rightmost columns.

There are two (doh) new extensions to report mouse coordinates beyond 231. It would be nice to add support for them to MC.

I'm planning to come up with a patch as my time permits.

See technical details in the next comment...

[egmont]

• The old way of reporting mouse coordinates is the following:
  • Output DECSET 1000 to enable mouse
  • Expect escape sequences in the format \e[M<action+32><x+32><y+32> whereas <action+32>, <x+32> and <y+32> are single bytes. (Action is 0 for left click, 1 for middle click, 2 for right click, 3 for release, or something like this.)
  • Disadvantages of this format:
    • x and y can only go up to 231.
    • Coordinates above 95 are not ascii-compatible, so any character set converting layer (e.g. luit) messes them up.
    • The stream is not valid UTF-8, even if everything else is

• The first new extension, introduced by xterm-262, is the following:
  • Output DECSET 1000 to enable mouse, followed by DECSET 1005 to activate extended mode.
  • Expect escape sequences in the format \e[M<action+32><<x+32>><<y+32>> whereas <<x+32>> and <<y+32>> each can be up to two bytes long: coordinate+32 is encoded in UTF-8.
  • Disadvantates of this format:
    • There's still a limit of 2015 rows/columns (okay, it's not a real life problem).
    • Doesn't solve the luit issue.
    • It is "horribly broken" (quoting urxvt's changelog) in terms of compatibility with the previous standard. There is no way for an application to tell whether the underlying terminal supports this new mode (whether DECSET 1005 did actually change the behavior or not), but depending on this a completely different user action might generate the same input. Example:
      • If the terminal doesn't support this extension, then clicking at (162, 129) generates \e[M<32><194><161>
      • If the terminal supports this extension, then clicking at (129, 1) [bit of math: 129+32 = 161, U+0161 in UTF-8 is 194 161] generates \e[M<32><194><161><33>
      • so there's no way to tell whether the terminal ignored the 1005 escape sequence, the user clicked on (162, 129) and then typed an exclamation mark; or whether the terminal recognized the escape, and the user clicked on (129, 1).
    • Due to this horrible brokenness, there's no way to implement support it without explicitly asking the user (via a setting) if the terminal can speak this extension.

• The second new extension, introduced by rxvt-unicode-9.10, is the following:
  • Output DECSET 1000 to enable mouse, followed by DECSET 1015 to activate this extended mode.
  • Expect escape sequences in the format \e[{action+32};{x};{y}M where this time I used the braces to denote spelling out the numbers in decimal, rather than using raw bytes
  • The only thing I don't understand is why they kept the offset of 32 at action, but other than that, this format is totally okay, and solves all the weaknesses of the previous ones.

Currently, at least the following terminal emulators have support for these:

• xterm supports the xterm extension
• rxvt-unicode supports both extensions
• iterm2 supports both extensions

I'll file feature requests for the authors of other well-known terminals (including xterm) to implement at least the urxvt extension.

Given the reasons above how horribly broken the xterm extension is, my plan is to implement support for the urxvt extension only in MC. I'll come back as soon as I have a patch.

[andrew_b]

This ticket looks like duplicate of #2399.

[egmont]

Oh yes you're right, I didn't find that one.
I know it's impolite to mark the original one as the dupe of the new one, but given the amount of technical information in the tickets, could you please make an exception and mark that one as a duplicate of this? (Sorry, gms, and thanks for the original report!)

[andrew_b]

I've mark this ticket as blocking of #2399, and we'll close both tickets at the same time.

[egmont]

First patch attached, please take a look :)

Expected behavior:

• In urxvt and iterm2 (and maybe other terminals which support the urxvt extension): mouse works flawlessly even beyond column 232.
• In other terminals (including xterm which only supports the brain-damaged extension): no user-visible change, that is, mouse still only works up to column 231.

Right now I could only test it on iterm2, but urxvt really should behave the same.

[egmont]

So apparently 255-32 = 223, and not 231. Sorry it seems I can't do maths :D

[angel_il]

key.c:938:46: warning: declaration shadows a variable in the global scope
      [-Wshadow]
parse_extended_mouse_coordinates (const int *keys)
                                             ^
key.c:508:17: note: previous declaration is here
static key_def *keys = NULL;

[angel_il]

patch for xterm to allow generate mouse ESC-sequence beyond 223 col.

diff -r button.c button.utf-8-fix.c
--- a/button.c  Sat Aug 14 08:23:00 2010 +0200
+++ b/button.c  Thu Aug 26 16:16:48 2010 +0200
@@ -3994,1 +3994,27 @@
-#define MOUSE_LIMIT (255 - 32)
+#define MOUSE_LIMIT (2047 - 32)
+#define MOUSE_UTF_8_START (127 - 32)
+
+static unsigned
+EmitMousePosition(Char line[], unsigned count, int value)
+{
+    /* Add pointer position to key sequence
+     * 
+     * Encode large positions as two-byte UTF-8 
+     *
+     * NOTE: historically, it was possible to emit 256, which became
+     * zero by truncation to 8 bits. While this was arguably a bug,
+     * it's also somewhat useful as a past-end marker so we keep it.
+     */
+    if(value == MOUSE_LIMIT) {
+       line[count++] = CharOf(0);
+    }
+    else if(value < MOUSE_UTF_8_START) {
+       line[count++] = CharOf(' ' + value + 1);
+    }
+    else {
+       value += ' ' + 1;
+       line[count++] = CharOf(0xC0 + (value >> 6));
+       line[count++] = CharOf(0x80 + (value & 0x3F));
+    }
+    return count;
+}
@@ -4001,1 +4027,1 @@
-    Char line[6];
+    Char line[9]; /* \e [ > M Pb Pxh Pxl Pyh Pyl */
@@ -4021,2 +4047,0 @@
-    else if (row > MOUSE_LIMIT)
-       row = MOUSE_LIMIT;
@@ -4028,1 +4052,5 @@
-    else if (col > MOUSE_LIMIT)
+
+    /* Limit to representable mouse dimensions */
+    if (row > MOUSE_LIMIT)
+       row = MOUSE_LIMIT;
+    if (col > MOUSE_LIMIT)
@@ -4090,2 +4118,2 @@
-       line[count++] = CharOf(' ' + col + 1);
-       line[count++] = CharOf(' ' + row + 1);
+       count = EmitMousePosition(line, count, col);
+       count = EmitMousePosition(line, count, row);

[angel_il]

so... Konsole, gnome Termital do not support this feature...

[egmont]

The xterm patch you inlined made it into xterm-262, and only supports the broken extension (which is not compatible with my MC patch). I made a patch yesterday to add urxvt-extension to xterm and sent it to the author; for the record I also attach it here.

gnome-terminal patch is here: ​http://bugzilla.gnome.org/show_bug.cgi?id=662423

I've also sent a feature request (without patch) to PuTTY, and I'm planning to file a feature request for Konsole too.

[egmont]

Patch updated to address the warning of comment 7.

[egmont]

And just for reference, the Konsole ticket is here: ​http://bugs.kde.org/show_bug.cgi?id=285984

[egmont]

(A word about GNU Screen: It seems that screen doesn't forward any escape sequence it is not familiar with, including the request to switch to the urxvt 1015 mode. This means that MC with this urxvt patch should make no difference inside screen: mouse will still only work up to 223. This is good news for me because I was afraid that something might break inside screen, but luckily nothing breaks. I'll see if I can patch screen to properly forward the urxvt 1015 request, as well as the new style coordinates.)

[egmont]

Tmux ticket: ​https://sourceforge.net/tracker/?func=detail&aid=3435156&group_id=200378&atid=973265

[andrew_b]

Thanks. Applied with trivial changes.

Branch: 2662_extended_mouse_clicks (parent: master).
changeset:ab9a66406a684d89e26bd14ce4cf90fa28e11c3b

But I'd note that urxvt >= 9.10 is required to support mouse clicks beyond 223.

[egmont]

Thanks Andrew!

Yes currently urxvt >= 9.10 or iTerm2 is needed. Hopefully other terminals will follow, e.g. in the gnome-terminal ticket they already had a quite positive response.

I hope you don't mind if I keep this ticket as a starting point for keeping track of generic support of this feature in terminals and maybe other apps as well.

[angel_il]

egmont:
can you write automation tests for parse_extended_mouse_coordinates (void)
and i think need add param into parse_extended_mouse_coordinates. what you think about?

[egmont]

angel_li: I'll take a look, I guess it's not hard, I just have to find some spare time :)

[andrew_b]

Merged to master.
changeset:026b07a2ab507f1e7711eee87adb4cec35ef1c64

[egmont]

The change broke Ctrl+Space (directory size calculation), it causes mc to hang.

That key combo sends a 0 byte. parse_extended_mouse_coordinates() looks at the buffer of pending_keys to decide if it is a valid prefix of an extended mouse coordinate, but incorrectly assumes that pending_keys is a 0-terminated buffer. Hence when it includes a 0 as part of its actual content, it always reports that it is a valid prefix, and hence mc waits for the sequence to complete, but it will never complete.

The solution I guess is to use seq_append as the end of the buffer, instead of a 0 value.

I'll try to come up with a patch, but I'm quite busy nowadays. If it's blocking the release then feel free to revert and we'll fix it for the next release.

(I was about to refactor the code a little bit anyways, I promised to write unittests and figured out that for testing it would probably be a better design if the same parse_mouse_coordinates() method was responsible for both the old and the new style coordinates.)

[egmont]

fix for the ctrl+space bug

[egmont]

I added a patch that seems to work at a first glimpse. I'll keep on using this patch (although apparently I never use Ctrl+Space:)).

I'm still planning to do some refactoring later on to make it less hackish and easier to unittest.

[slavazanko]

Created branch 2662_fix_space_calculation, parent branch: master.

Review, please.

[slavazanko]

Merged to master:

git log --pretty=oneline f507987..b491287

[egmont]

(Just FYI: Continuing in ticket #2956)