Changed by egmont (@egmontkob) on Nov 3, 2011 at 17:37 UTC (comment 1)

• 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.

Changed by andrew_b (@aborodin) on Nov 4, 2011 at 6:18 UTC (comment 2)

• Component changed from mc-core to mc-tty

This ticket looks like duplicate of #2399.

Changed by egmont (@egmontkob) on Nov 4, 2011 at 8:42 UTC (comment 3)

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!)

Changed by andrew_b (@aborodin) on Nov 4, 2011 at 10:37 UTC (comment 4)

• Blocking set to #2399

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

Changed by egmont (@egmontkob) on Nov 6, 2011 at 18:21 UTC (comment 5)

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.

Changed by egmont (@egmontkob) on Nov 6, 2011 at 22:02 UTC (comment 6)

• Summary changed from Support extended mouse clicks beyond 231 to Support extended mouse clicks beyond 223

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

Changed by angel_il (@ilia-maslakov) on Nov 7, 2011 at 6:20 UTC (comment 7)

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;

Changed by angel_il (@ilia-maslakov) on Nov 7, 2011 at 6:44 UTC (comment 8)

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);

Changed by angel_il (@ilia-maslakov) on Nov 7, 2011 at 6:52 UTC (comment 9)

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

Changed by egmont (@egmontkob) on Nov 7, 2011 at 8:17 UTC (comment 10)

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.

Changed by egmont (@egmontkob) on Nov 7, 2011 at 8:26 UTC

• Attachment xterm-276-urxvt-mouse-mode.patch (raw) added

Changed by egmont (@egmontkob) on Nov 7, 2011 at 11:48 UTC

• Attachment mc-4.8.0-extended-mouse-coordinates.patch (raw) added

Changed by egmont (@egmontkob) on Nov 7, 2011 at 11:48 UTC (comment 11)

Patch updated to address the warning of comment 7.

Changed by egmont (@egmontkob) on Nov 7, 2011 at 12:00 UTC (comment 12)

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

Changed by egmont (@egmontkob) on Nov 7, 2011 at 18:38 UTC

• Attachment putty-0.61-extended-mouse.patch (raw) added

Changed by egmont (@egmontkob) on Nov 8, 2011 at 10:30 UTC (comment 13)

(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.)

Changed by egmont (@egmontkob) on Nov 8, 2011 at 23:10 UTC (comment 14)

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

Changed by andrew_b (@aborodin) on Nov 9, 2011 at 11:23 UTC (comment 15)

• Votes set to andrew_b
• Milestone changed from Future Releases to 4.8.1
• Status changed from new to accepted
• Owner set to andrew_b
• Branch state changed from no branch to on review

Thanks. Applied with trivial changes.

Branch: 2662_extended_mouse_clicks (parent: master).
[ab9a66406a684d89e26bd14ce4cf90fa28e11c3b]

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

Changed by egmont (@egmontkob) on Nov 9, 2011 at 11:30 UTC (comment 16)

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.

Changed by angel_il (@ilia-maslakov) on Nov 25, 2011 at 9:19 UTC (comment 17)

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?

Changed by slavazanko (@slavaz) on Nov 25, 2011 at 13:12 UTC (comment 18)

• Votes changed from andrew_b to andrew_b slavazanko
• Branch state changed from on review to approved

Changed by egmont (@egmontkob) on Nov 25, 2011 at 13:45 UTC (comment 19)

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

Changed by andrew_b (@aborodin) on Nov 29, 2011 at 5:50 UTC (comment 20)

• Votes changed from andrew_b slavazanko to committed-master
• Keywords set to stable-candidate
• Blocking #2399 deleted
• Branch state changed from approved to merged

Merged to master.
[026b07a]

Changed by andrew_b (@aborodin) on Dec 8, 2011 at 7:40 UTC (comment 21)

• Status changed from accepted to testing
• Resolution set to fixed

Changed by egmont (@egmontkob) on Dec 8, 2011 at 8:36 UTC (comment 22)

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.)

Changed by egmont (@egmontkob) on Dec 8, 2011 at 8:42 UTC (comment 23)

• Resolution fixed deleted
• Status changed from testing to reopened

Changed by egmont (@egmontkob) on Dec 8, 2011 at 9:00 UTC

• Attachment mc-4.8.0-extended-mouse-broke-ctrl-space.patch (raw) added

fix for the ctrl+space bug

Changed by egmont (@egmontkob) on Dec 8, 2011 at 9:01 UTC (comment 24)

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.

Changed by slavazanko (@slavaz) on Dec 8, 2011 at 10:06 UTC (comment 25)

• Owner changed from andrew_b to slavazanko
• Status changed from reopened to accepted

Changed by slavazanko (@slavaz) on Dec 8, 2011 at 11:35 UTC (comment 26)

• Branch state changed from merged to on review

Created branch 2662_fix_space_calculation [2662_fix_space_calculation], parent branch: master.

Review, please.

Changed by slavazanko (@slavaz) on Dec 8, 2011 at 12:15 UTC (comment 27)

• Votes changed from committed-master to slavazanko

Changed by andrew_b (@aborodin) on Dec 9, 2011 at 10:06 UTC (comment 28)

• Branch state changed from on review to approved
• Votes changed from slavazanko to slavazanko andrew_b

Changed by slavazanko (@slavaz) on Dec 9, 2011 at 11:11 UTC (comment 29)

• Resolution set to fixed
• Votes changed from slavazanko andrew_b to commited-master
• Keywords stable-candidate deleted
• Status changed from accepted to testing
• Branch state changed from approved to merged

Merged to master:

git log --pretty=oneline f507987..b491287

Changed by slavazanko (@slavaz) on Dec 9, 2011 at 11:11 UTC (comment 30)

• Status changed from testing to closed

Changed by egmont (@egmontkob) on Jan 27, 2013 at 12:57 UTC (comment 31)

(Just FYI: Continuing in ticket #2956)