GNOME Bugzilla – Bug 681329
Support for xterm's 1006 mouse extension
Last modified: 2013-09-14 12:43:24 UTC
Created attachment 220486 [details] [review] support xterm's 1006 mouse mode This is a followup of bug 662423. Could you please apply the attached patch? The extension in the referred bug (invented by urxvt's author) allows the terminal to report coordinates beyond 223. However, for multiple reasons, xterm's author decided to come up with another extension, addressing the same issue and more. His reasons are, quoting from xterm's changelog: o the responses will not be confused with line-deletion and scrolling controls. [This matters when using a control sequence that reports back control sequences, apparently only supported by xterm - this comment was added by me] o the button encoding is a little simpler, since it does not add an unnecessary 32 because the integer parameter does not have to be represented as a printable character. o the control responses for pressing and releasing a mouse button differ, allowing an application to tell which button was released. My reasons in favor of the new extension are: o It is much easier to patch in support for this new extension into already existing applications. The urxvt extension, due to the lack of a unique prefix, usually requires heavy modifications in application sources which look up escapes in a table or search tree. The new extension has a prefix that you can recognize and jump into the code that's responsible solely for parsing the remaining arguments. o Emacs-25 will support this new extension only, and not the other one, see http://bzr.savannah.gnu.org/lh/emacs/trunk/revision/109093 . The reason is what I mentioned in the previous point. Implementing the urxvt extension would have required ugly hacks extending the interface between the C and the Lisp code (as there's a moment where neither the C engine responsible for generic stuff, nor the Lisp extension responsible for parsing mouse events knows to whom the currently seen partial escape will belong to), while the new extension could easily be implemented solely in Lisp. (And, of course, I have verified that emacs trunk on vte with my patch indeed works.) o Similarly, I created a while ago the patch for midnight commander using the old extension, and I know that most of the bloatware I needed to add could be removed with the new one. I also created patch for both extensions for the joe text editor, the one for the new extension is way simpler and more straightforward, more likely to be accepted by mainstream. I think I have a good reason to assume that for dozens of other applications out there the situation would be similar. A note about the patch: So far the way of encoding the mouse info was split into two methods: vte_terminal_get_mouse_tracking_info constructed amongst others the number representing the button and stuff, and vte_terminal_feed_mouse_event created the escapes. Since with the new mode the way of encoding the button does depend on the chosen output format, I no longer found it fortunate to have it split into two (and check which extension is being used in both methods), I found it simpler to do everything in one method.
Looks ok. Can you attach the patch as a git format-patch patch series against vte-0-34 branch containing this patch on top of a *revert* commit of the patch that added the urxvt mode support (I don't think we need to support that one going forward) ?
I don't think dropping the urxvt support is a good idea. Right now Midnight Commander uses that extension out of the box, and Vim uses that in case it's configured to do so. Dropping support would break these two applications in vte. Generally there are multiple ways to do something in a terminal, e.g. there are multiple ways to switch to a color, multiple ways to move the cursor somewhere, etc. Terminal emulators support as much as they can, and applications choose one to do. This is okay this way, and this should be the same with the mouse extensions. It's not reasonable for an application to enable both extensions and parse both possible incoming escape sequences; nor is it a wise idea to break the fix in some applications. If all the popular terminals that support the urxvt mode right now will add support for the new xterm extension as well, and then all the applications will migrate to using the newer one, then support for the urxvt mode can be dropped. I am happy to assist in this process, but knowing the speed of people acting on enhancement requests, knowing the release cycles of software and so on, realistically this is going to take about a year or two.
I think since the urxvt extension is still very new, and thus very few terminal programmes support it yet, it's exactly now that we *can* still remove it, as opposed to later. And given that the urxvt extension has the problems detailed in comment 0, I think it would be better if we removed it *now*.
Created attachment 221937 [details] [review] emulation: Support urxvt extended mouse tracking mode ... in addition to the new-and-improved xterm extended mouse tracking mode.
I would agree with you if there was some mandate from an upper level management, or if the authors of all terminals and all applications clearly agreed to go for the new xterm extension and drop the urxvt one for good. This is not the case. This will be an evolutionary process, no one tell if one or the other extension will die out or if the two will live simoultaneously, or if a third one will win. This evolution is mainly how open source software works. I cannot even guarantee that urxvt or konsole or whichever terminal will ever add support for the new xterm extension. Right now, if you install the latest stable from everything, then beyond-223 mouse clicks work out of the box in Midnight Commander, in many terminals including Xterm, Urxvt, Vte, Konsole etc. I would really hate to see it break in Vte, and to reach out to Midnight Commander's devs explaining them why it broke and why they should migrate to a new method which will be broken for a while in konsole and urxvt - if they ever implement the new feature at all, otherwise it'll be broken in those terminals forever. Put the user first, put them before the technical details and uglinesses of implementing two similar standards. Don't break a feature for them that works currently.
I realised that the patch in attachment 221937 [details] [review] wouldn't be enough to reliably add back urxvt mode support, since if you first enable xterm mode then urxvt mode the xterm mode will still take precedence. So a more extended patch would be needed that turns off the urxvt (xterm) mode when enabling the xterm (urxvt) mode.
It's not specified anywhere what should happen if multiple extensions are switched on at the same time. One possible behavior is to turn off the other one at that point. Another one is to create a stacking order (to hide the other one, but to revert to that one when the current one gets turned off). Yet another one is to have an independent boolean for each, and use the most advanced that's enabled. My proposed patch implements the latter. This is also the behavior currently in use by xterm, konsole etc. The reason why I chose this behavior is: This is the simplest to implement, and it really doesn't matter. First, it's reasonable to assume that each application only turns on one extension, and turns it off when exits. (On a non-clean exit, a manual terminal reset is often necessary anyways.) Second, even if an application turns on multiple, it's reasonable to assume that the application understands all of them and doesn't really care which one is being used. That's why I consistently chose, also in my proposed patches to other terminal emulators, that each extension has its own boolean independently from others, and at runtime the priority is 1006 > 1015 > 1005 (not impl. by vte) > 1000. The only thing that really does matter is that the emulator doesn't mix the two extensions, as it is emphasized by the comment in the patch. E.g. if the leading prefix was ^[[< (as per 1006) and the trailing letter was always 'M' (as per 1015) if an application turned on both, that would be really bad. Other than this trivial constraint, which extension to use if multiple ones are switched on really doesn't matter IMHO. As far as I remember, the de facto reference implementation Xterm also uses separate booleans, and a fixed preference order.
Having looked at xterm code now, I see that this new 1006 mode also does emits when the selection is extended, for example. The patch doesn't implement this at all. So is it really wise to only support half of 1006 mode ?
This sounds like something I overlooked. Hold on for a while... I'll be back in September after a vacation and will look into this.
Could you please clarify your last comment? I can't see any change in this regard, introduced by this extension. In xterm, mouse drag events are reported if the 1002 mode is enabled (as opposed to 1000), and so are in vte. If a modifier such as shift is pressed, which causes the terminal to highlight as if mouse wasn't enabled by the application, then this is not reported by the terminals, which is the right thing to do. In a side note, vte reports the release event if such a modifier is used and the mouse was not dragged - IMHO this is a bug that's unrelated to the current issue.
I looked at xterm's "highlight tracking" (1001) mode. I'm not sure this is what you meant. This mode doesn't seem to be supported by vte, and even though the source handles this escape, it does it wrong. The enum in src/vte-private.h contains MOUSE_TRACKING_HILITE_TRACKING in the list, but this mode is not just a certain level of "what to track", but a fundamentally different feature that doesn't fit in this list. I don't think anyone else than xterm implements this feature, and pretty much sounds like a broken one (an application not cooperating properly with the terminal can freeze the terminal - wtf?). I'm still eager to hear what you think I missed from my patch, I can't figure it out on my own, sorry :)
Yes, that was it. And after checking again, I'm not going to hold up this bug for implementing the selection mode. And you've concinved me we shouldn't drop urxvt support; so I'll re-commit the patch for 0.34.1.
Committed to 0-34 and next. (One comment on the xterm mode itself: /* Encode the modifiers. */ if (terminal->pvt->modifiers & GDK_SHIFT_MASK) { cb |= 4; } if (terminal->pvt->modifiers & VTE_META_MASK) { cb |= 8; } if (terminal->pvt->modifiers & GDK_CONTROL_MASK) { cb |= 16; } /* Encode a drag event. */ if (is_drag) { cb |= 32; } So if drag really is 32, then that means that mouse button reporting never can report the 4th modifier (real "meta", not the vte "meta" which really is "alt"), see bug 657749.)
This is already the case with the legacy 1002 mode, and the 1015 extension. The 1006 extension doesn't modify the meaning of the numbers (well, it does a bit around mouse release, but this is irrelevant here), it is basically about the way these numbers are encoded. The bit at the 32 position (which becomes 64 because the 1002 and 1015 modes offset the final number by 32) are already being used for signaling drag by many emulators, including vte itself. My patch just reorganizes the code, the old version went something like vte_terminal_maybe_send_mouse_drag(...) { ... vte_terminal_get_mouse_tracking_info (...) cb += 32; /* for movement */ <- I moved it from here to the code that you quoted vte_terminal_feed_mouse_event (...) } If new modifiers are needed, I guess values 64, 128... could be used (noting that 128 plus other modifiers, shifted by 32 in the legacy encoding scheme might already overflow), but I would probably ask xterm's author for guidance.
Right; that wasn't meant as criticism of the patch, but of the xterm extension itself :-)
I didn't take it as criticism, just clarified the situation :) Thanks very much for commiting the patch!