bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

`xterm-mouse-mode' sends \e[?1005h to enable UTF-8 mouse coordinates,
but it never checks that the terminal actually supports that.  Then
`xterm-mouse--read-utf8-char' reads an UTF-8 sequence without checking
that the terminal actually sent one.  On terminals that don't support
UTF-8 coordinates (such as Chromium's hterm) this leads to garbage if
any mouse coordinate is larger than 127.  For example, when clicking in
the right part of the terminal in hterm, the menu opens and the
following is shown in lossage:

   ESC [ M SPC \237 F ESC [ M # \237 F [tmm-menubar-mouse]

Instead, this should have been interpreted as two events with
single-byte mouse coordinates.

I'd suggest the following (and will happily provide patches if
accepted):

- Don't enable UTF-8 coordinates at all.  It is too hard to figure out
  whether they are enabled.  Rather, err on the safe side and only use
  single-byte coordinates.  (The superior SGR mode, which doesn't suffer
  from this problem, should remain enabled.)

- In `xterm-mouse--read-number-from-terminal', specify a very short
  timeout.  The terminal will always write the entire escape sequence as
  a unit, so waiting only increases the chance to accidentially read
  unrelated events.

- As an optimization, provide a `read-byte' function written in C that
  reads a single byte without taking the current terminal encoding into
  account.



In GNU Emacs 25.0.92.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2016-03-14 built on REDACTED
Repository revision: 80ec484ac83e6965a843dabf766ade057ee1bc6a
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04 LTS

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GCONF GSETTINGS NOTIFY ACL
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec epg epg-config gnus-util mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util help-fns help-mode easymenu cl-loaddefs pcase
cl-lib mail-prsvr mail-utils time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 88738 7446)
 (symbols 48 19932 0)
 (miscs 40 463 186)
 (strings 32 14918 4643)
 (string-bytes 1 445677)
 (vectors 16 12226)
 (vector-slots 8 442799 4664)
 (floats 8 164 18)
 (intervals 56 206 0)
 (buffers 976 12)
 (heap 1024 30327 1014))

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich.  Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen
Sie die E-Mail und alle Anhänge.  Vielen Dank.

This e-mail is confidential.  If you are not the right addressee please do not
forward it, please inform the sender, and please erase this e-mail including
any attachments.  Thanks.

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 14 Mar 2016 13:56:41 +0100
> 
> I'd suggest the following (and will happily provide patches if
> accepted):
> 
> - Don't enable UTF-8 coordinates at all.  It is too hard to figure out
>   whether they are enabled.  Rather, err on the safe side and only use
>   single-byte coordinates.  (The superior SGR mode, which doesn't suffer
>   from this problem, should remain enabled.)

How about providing a user option, off by default, to enable that?  A
user who knows that this works on her machine will then be able to use
the feature.

> - In `xterm-mouse--read-number-from-terminal', specify a very short
>   timeout.  The terminal will always write the entire escape sequence as
>   a unit, so waiting only increases the chance to accidentially read
>   unrelated events.

Won't this break when working on a remote machine via a slow link?

> - As an optimization, provide a `read-byte' function written in C that
>   reads a single byte without taking the current terminal encoding into
>   account.

You should be able to achieve the same effect by binding
keyboard-coding-system to no-conversion, no?

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Mo., 14. März 2016 um 17:43 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Mon, 14 Mar 2016 13:56:41 +0100
> >
> > I'd suggest the following (and will happily provide patches if
> > accepted):
> >
> > - Don't enable UTF-8 coordinates at all.  It is too hard to figure out
> >   whether they are enabled.  Rather, err on the safe side and only use
> >   single-byte coordinates.  (The superior SGR mode, which doesn't suffer
> >   from this problem, should remain enabled.)
>
> How about providing a user option, off by default, to enable that?  A
> user who knows that this works on her machine will then be able to use
> the feature.
>

I considered that, but then it would be impossible to have different
terminals with and without the option. Might still be a good idea, if it's
off by default.


>
> > - In `xterm-mouse--read-number-from-terminal', specify a very short
> >   timeout.  The terminal will always write the entire escape sequence as
> >   a unit, so waiting only increases the chance to accidentially read
> >   unrelated events.
>
> Won't this break when working on a remote machine via a slow link?
>

Maybe, haven't tried.


>
> > - As an optimization, provide a `read-byte' function written in C that
> >   reads a single byte without taking the current terminal encoding into
> >   account.
>
> You should be able to achieve the same effect by binding
> keyboard-coding-system to no-conversion, no?
>

Yes, but that feels kind of like an abstraction inversion. Still probably
good enough. As said, this is only an optimization.

[-- Attachment #2: Type: text/html, Size: 2429 bytes --]

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 1102 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am Mo., 14. März 2016 um
18:24 Uhr:

> Eli Zaretskii <eliz@gnu.org> schrieb am Mo., 14. März 2016 um 17:43 Uhr:
>
>> > From: Philipp Stephani <p.stephani2@gmail.com>
>> > Date: Mon, 14 Mar 2016 13:56:41 +0100
>> >
>> > I'd suggest the following (and will happily provide patches if
>> > accepted):
>> >
>> > - Don't enable UTF-8 coordinates at all.  It is too hard to figure out
>> >   whether they are enabled.  Rather, err on the safe side and only use
>> >   single-byte coordinates.  (The superior SGR mode, which doesn't suffer
>> >   from this problem, should remain enabled.)
>>
>> How about providing a user option, off by default, to enable that?  A
>> user who knows that this works on her machine will then be able to use
>> the feature.
>>
>
> I considered that, but then it would be impossible to have different
> terminals with and without the option. Might still be a good idea, if it's
> off by default.
>

Added a patch. I've had to use latin-1 instead of no-conversion to prevent
resetting the meta mode.

[-- Attachment #1.2: Type: text/html, Size: 1810 bytes --]

[-- Attachment #2: 0001-Add-customization-option-for-UTF-8-coordinates.patch --]
[-- Type: application/octet-stream, Size: 14461 bytes --]

From 159e4ea4f457fab41028b6f3515a2d7856c3e434 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Mon, 14 Mar 2016 23:20:21 +0100
Subject: [PATCH] Add customization option for UTF-8 coordinates

* international/mule.el (keyboard-coding-system): Define as generalized
variable place.
* lisp/xt-mouse.el (xterm-mouse-utf-8): New customization option.
(xterm-mouse--read-coordinate): New function to replace
`xterm-mouse--read-utf8-char'; uses UTF-8 only if enabled.
(xterm-mouse--read-number-from-terminal): Adapt to new name.
(xterm-mouse-tracking-enable-sequence)
(xterm-mouse-tracking-disable-sequence): Replace constants with
functions, mark constants as obsolete.
(xterm-mouse--tracking-sequence): New helper function.
(turn-on-xterm-mouse-tracking-on-terminal): Use new functions;
enable UTF-8 only if customization option says so; store UTF-8
flag in terminal parameter.
* test/automated/xt-mouse-tests.el: Add tests for xt-mouse.el.
---
 lisp/international/mule.el       |   3 +
 lisp/xt-mouse.el                 | 124 +++++++++++++++++++++++++++++----------
 test/automated/xt-mouse-tests.el | 110 ++++++++++++++++++++++++++++++++++
 3 files changed, 207 insertions(+), 30 deletions(-)
 create mode 100644 test/automated/xt-mouse-tests.el

diff --git a/lisp/international/mule.el b/lisp/international/mule.el
index 60a90ae..971a49e 100644
--- a/lisp/international/mule.el
+++ b/lisp/international/mule.el
@@ -1484,6 +1484,9 @@ set-keyboard-coding-system
   (set-keyboard-coding-system-internal coding-system terminal)
   (setq keyboard-coding-system coding-system))
 
+(gv-define-setter keyboard-coding-system (coding-system &optional terminal)
+  `(set-keyboard-coding-system ,coding-system ,terminal))
+
 (defcustom keyboard-coding-system nil
   "Specify coding system for keyboard input.
 If you set this on a terminal which can't distinguish Meta keys from
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 5975e60..aa2d793 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -134,23 +134,30 @@ xterm-mouse-truncate-wrap
             (fdiff (- f (* 1.0 maxwrap dbig))))
        (+ (truncate fdiff) (* maxwrap dbig))))))
 
-(defun xterm-mouse--read-utf8-char (&optional prompt seconds)
-  "Read an utf-8 encoded character from the current terminal.
-This function reads and returns an utf-8 encoded character of
-command input. If the user generates an event which is not a
-character (i.e., a mouse click or function key event), read-char
-signals an error.
-
-The returned event may come directly from the user, or from a
-keyboard macro. It is not decoded by the keyboard's input coding
-system and always treated with an utf-8 input encoding.
-
-The optional arguments PROMPT and SECONDS work like in
-`read-event'."
-  (let ((tmp (keyboard-coding-system)))
-    (set-keyboard-coding-system 'utf-8)
-    (prog1 (read-event prompt t seconds)
-      (set-keyboard-coding-system tmp))))
+(defcustom xterm-mouse-utf-8 nil
+  "Non-nil if UTF-8 coordinates should be used to read mouse coordinates.
+Set this to non-nil if you are sure that your terminal
+understands UTF-8 coordinates, but not SGR coordinates."
+  :type 'boolean
+  :risky t
+  :group 'xterm)
+
+(defun xterm-mouse--read-coordinate ()
+  "Read a mouse coordinate from the current terminal.
+If `xterm-mouse-utf-8' was non-nil when
+`turn-on-xterm-mouse-tracking-on-terminal' was called, reads the
+coordinate as an UTF-8 code unit sequence; otherwise, reads a
+single byte."
+  (cl-letf (((keyboard-coding-system)
+             (if (terminal-parameter nil 'xterm-mouse-utf-8)
+                 'utf-8-unix
+               ;; Use Latin-1 instead of no-conversion to avoid
+               ;; flicker due to `set-keyboard-coding-system' changing
+               ;; the meta mode.
+               'iso-latin-1-unix)))
+    ;; Wait only a little; we assume that the entire escape sequence
+    ;; has already been sent when this function is called.
+    (read-char nil nil 0.1)))
 
 ;; In default mode, each numeric parameter of XTerm's mouse report is
 ;; a single char, possibly encoded as utf-8.  The actual numeric
@@ -170,7 +177,7 @@ xterm-mouse--read-number-from-terminal
                    (<= ?0 c ?9))
             (setq n (+ (* 10 n) c (- ?0))))
           (cons n c))
-      (cons (- (setq c (xterm-mouse--read-utf8-char)) 32) c))))
+      (cons (- (setq c (xterm-mouse--read-coordinate)) 32) c))))
 
 ;; XTerm reports mouse events as
 ;; <EVENT-CODE> <X> <Y> in default mode, and
@@ -314,6 +321,38 @@ xterm-mouse-mode
     (mapc #'turn-off-xterm-mouse-tracking-on-terminal (terminal-list))
     (setq mouse-position-function nil)))
 
+(defun xterm-mouse-tracking-enable-sequence ()
+  "Return a control sequence to enable XTerm mouse tracking.
+The returned control sequence enables basic mouse tracking, mouse
+motion events and finally extended tracking on terminals that
+support it.  The following escape sequences are understood by
+modern xterms:
+
+\"\\e[?1000h\" \"Basic mouse mode\": Enables reports for mouse
+            clicks.  There is a limit to the maximum row/column
+            position (<= 223), which can be reported in this
+            basic mode.
+
+\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events during dragging operations.
+
+\"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an
+            extension to the basic mouse mode, which uses UTF-8
+            characters to overcome the 223 row/column limit.
+            This extension may conflict with non UTF-8
+            applications or non UTF-8 locales.  It is only
+            enabled when the option `xterm-mouse-utf-8' is
+            non-nil.
+
+\"\\e[?1006h\" \"SGR coordinate extension\": Enables a newer
+            alternative extension to the basic mouse mode, which
+            overcomes the 223 row/column limit without the
+            drawbacks of the UTF-8 coordinate extension.
+
+The two extension modes are mutually exclusive, where the last
+given escape sequence takes precedence over the former."
+  (apply #'concat (xterm-mouse--tracking-sequence ?h)))
+
 (defconst xterm-mouse-tracking-enable-sequence
   "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"
   "Control sequence to enable xterm mouse tracking.
@@ -343,10 +382,34 @@ xterm-mouse-tracking-enable-sequence
 The two extension modes are mutually exclusive, where the last
 given escape sequence takes precedence over the former.")
 
+(make-obsolete-variable
+ 'xterm-mouse-tracking-enable-sequence
+ "use the function `xterm-mouse-tracking-enable-sequence' instead."
+ "25.1")
+
+(defun xterm-mouse-tracking-disable-sequence ()
+  "Return a control sequence to disable XTerm mouse tracking.
+The control sequence resets the modes set by
+`xterm-mouse-tracking-enable-sequence'."
+  (apply #'concat (nreverse (xterm-mouse--tracking-sequence ?l))))
+
 (defconst xterm-mouse-tracking-disable-sequence
   "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"
   "Reset the modes set by `xterm-mouse-tracking-enable-sequence'.")
 
+(make-obsolete-variable
+ 'xterm-mouse-tracking-disable-sequence
+ "use the function `xterm-mouse-tracking-disable-sequence' instead."
+ "25.1")
+
+(defun xterm-mouse--tracking-sequence (suffix)
+  "Return a control sequence to enable or disable mouse tracking.
+SUFFIX is the last character of each escape sequence (?h to
+enable, ?l to disable)."
+  (mapcar
+   (lambda (code) (format "\e[?%d%c" code suffix))
+   `(1000 1002 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
+
 (defun turn-on-xterm-mouse-tracking-on-terminal (&optional terminal)
   "Enable xterm mouse tracking on TERMINAL."
   (when (and xterm-mouse-mode (eq t (terminal-live-p terminal))
@@ -360,18 +423,19 @@ turn-on-xterm-mouse-tracking-on-terminal
       (with-selected-frame (car (frames-on-display-list terminal))
         (define-key input-decode-map "\e[M" 'xterm-mouse-translate)
         (define-key input-decode-map "\e[<" 'xterm-mouse-translate-extended))
-      (condition-case err
-          (send-string-to-terminal xterm-mouse-tracking-enable-sequence
-                                   terminal)
-        ;; FIXME: This should use a dedicated error signal.
-        (error (if (equal (cadr err) "Terminal is currently suspended")
-                   nil                  ;The sequence will be sent upon resume.
-                 (signal (car err) (cdr err)))))
-      (push xterm-mouse-tracking-enable-sequence
-            (terminal-parameter nil 'tty-mode-set-strings))
-      (push xterm-mouse-tracking-disable-sequence
-            (terminal-parameter nil 'tty-mode-reset-strings))
-      (set-terminal-parameter terminal 'xterm-mouse-mode t))))
+      (let ((enable (xterm-mouse-tracking-enable-sequence))
+            (disable (xterm-mouse-tracking-disable-sequence)))
+        (condition-case err
+            (send-string-to-terminal enable terminal)
+          ;; FIXME: This should use a dedicated error signal.
+          (error (if (equal (cadr err) "Terminal is currently suspended")
+                     nil ; The sequence will be sent upon resume.
+                   (signal (car err) (cdr err)))))
+        (push enable (terminal-parameter nil 'tty-mode-set-strings))
+        (push disable (terminal-parameter nil 'tty-mode-reset-strings))
+        (set-terminal-parameter terminal 'xterm-mouse-mode t)
+        (set-terminal-parameter terminal 'xterm-mouse-utf-8
+                                xterm-mouse-utf-8)))))
 
 (defun turn-off-xterm-mouse-tracking-on-terminal (terminal)
   "Disable xterm mouse tracking on TERMINAL."
diff --git a/test/automated/xt-mouse-tests.el b/test/automated/xt-mouse-tests.el
new file mode 100644
index 0000000..c7e835c
--- /dev/null
+++ b/test/automated/xt-mouse-tests.el
@@ -0,0 +1,110 @@
+;;; xt-mouse-tests.el --- Test suite for xt-mouse.  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; Author: Philipp Stephani <phst@google.com>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'xt-mouse)
+
+(defmacro with-xterm-mouse-mode (&rest body)
+  "Run BODY with `xterm-mouse-mode' temporarily enabled."
+  (declare (indent 0))
+  ;; Make the frame huge so that the test input events below don't hit
+  ;; the menu bar.
+  `(cl-letf (((frame-width nil) 2000)
+             ((frame-height nil) 2000)
+             ;; Reset XTerm parameters so that the tests don't get
+             ;; confused.
+             ((terminal-parameter nil 'xterm-mouse-x) nil)
+             ((terminal-parameter nil 'xterm-mouse-y) nil)
+             ((terminal-parameter nil 'xterm-mouse-last-down) nil)
+             ((terminal-parameter nil 'xterm-mouse-last-click) nil))
+     (if xterm-mouse-mode
+         (progn ,@body)
+       (unwind-protect
+           (progn
+             ;; `xterm-mouse-mode' doesn't work in the initial
+             ;; terminal.  Since we can't create a second terminal in
+             ;; batch mode, fake it temporarily.
+             (cl-letf (((symbol-function 'terminal-name)
+                        (lambda (&optional _terminal) "fake-terminal")))
+               (xterm-mouse-mode))
+             ,@body)
+         (xterm-mouse-mode 0)))))
+
+(ert-deftest xt-mouse-tracking-basic ()
+  (should (equal (xterm-mouse-tracking-enable-sequence)
+                 "\e[?1000h\e[?1002h\e[?1006h"))
+  (should (equal (xterm-mouse-tracking-disable-sequence)
+                 "\e[?1006l\e[?1002l\e[?1000l"))
+  (with-xterm-mouse-mode
+    (should xterm-mouse-mode)
+    (should (terminal-parameter nil 'xterm-mouse-mode))
+    (should-not (terminal-parameter nil 'xterm-mouse-utf-8))
+    (let* ((unread-command-events (append "\e[M%\xD9\x81"
+                                          "\e[M'\xD9\x81" nil))
+           (key (read-key)))
+      (should (consp key))
+      (cl-destructuring-bind (event-type position . rest) key
+        (should (equal event-type 'S-mouse-2))
+        (should (consp position))
+        (cl-destructuring-bind (_ _ xy . rest) position
+          (should (equal xy '(184 . 95))))))))
+
+(ert-deftest xt-mouse-tracking-utf-8 ()
+  (let ((xterm-mouse-utf-8 t))
+    (should (equal (xterm-mouse-tracking-enable-sequence)
+                   "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"))
+    (should (equal (xterm-mouse-tracking-disable-sequence)
+                   "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"))
+    (with-xterm-mouse-mode
+      (should xterm-mouse-mode)
+      (should (terminal-parameter nil 'xterm-mouse-mode))
+      (should (terminal-parameter nil 'xterm-mouse-utf-8))
+      ;; The keyboard driver doesn't decode bytes in
+      ;; `unread-command-events'.
+      (let* ((unread-command-events (append "\e[M%\u0640\u0131"
+                                            "\e[M'\u0640\u0131" nil))
+             (key (read-key)))
+        (should (consp key))
+        (cl-destructuring-bind (event-type position . rest) key
+          (should (equal event-type 'S-mouse-2))
+          (should (consp position))
+          (cl-destructuring-bind (_ _ xy . rest) position
+            (should (equal xy '(1567 . 271)))))))))
+
+(ert-deftest xt-mouse-tracking-sgr ()
+  (with-xterm-mouse-mode
+    (should xterm-mouse-mode)
+    (should (terminal-parameter nil 'xterm-mouse-mode))
+    (should-not (terminal-parameter nil 'xterm-mouse-utf-8))
+    (let* ((unread-command-events (append "\e[<5;1569;273;M"
+                                          "\e[<5;1569;273;m" nil))
+           (key (read-key)))
+      (should (consp key))
+      (cl-destructuring-bind (event-type position . rest) key
+        (should (equal event-type 'S-mouse-2))
+        (should (consp position))
+        (cl-destructuring-bind (_ _ xy . rest) position
+          (should (equal xy '(1568 . 271))))))))
+
+;;; xt-mouse-tests.el ends here
-- 
2.7.0

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 14 Mar 2016 23:03:21 +0000
> Cc: 23009@debbugs.gnu.org
> 
> Added a patch. I've had to use latin-1 instead of no-conversion to prevent resetting the meta mode. 

Not sure I understand the problem you had with no-conversion.  Can you
elaborate?

> --- a/lisp/international/mule.el
> +++ b/lisp/international/mule.el
> @@ -1484,6 +1484,9 @@ set-keyboard-coding-system
>    (set-keyboard-coding-system-internal coding-system terminal)
>    (setq keyboard-coding-system coding-system))
>  
> +(gv-define-setter keyboard-coding-system (coding-system &optional terminal)
> +  `(set-keyboard-coding-system ,coding-system ,terminal))

I don't think you can do that: mule.el is preloaded, while gv.el
isn't.

It isn't a catastrophe to temporarily switch keyboard encoding "the
dull way".

> +               ;; Use Latin-1 instead of no-conversion to avoid
> +               ;; flicker due to `set-keyboard-coding-system' changing
> +               ;; the meta mode.

Ah, so that's the problem...  Did you try raw-text instead?

And anyway, doesn't latin-1 give you trouble for bytes in the 128..159
range?

Other than that, I have no comments.  Let's wait for a few days to
give others time to chime in, if they want to.

Thanks.

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Di., 15. März 2016 um 18:57 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Mon, 14 Mar 2016 23:03:21 +0000
> > Cc: 23009@debbugs.gnu.org
> >
> > Added a patch. I've had to use latin-1 instead of no-conversion to
> prevent resetting the meta mode.
>
> Not sure I understand the problem you had with no-conversion.  Can you
> elaborate?
>
> > --- a/lisp/international/mule.el
> > +++ b/lisp/international/mule.el
> > @@ -1484,6 +1484,9 @@ set-keyboard-coding-system
> >    (set-keyboard-coding-system-internal coding-system terminal)
> >    (setq keyboard-coding-system coding-system))
> >
> > +(gv-define-setter keyboard-coding-system (coding-system &optional
> terminal)
> > +  `(set-keyboard-coding-system ,coding-system ,terminal))
>
> I don't think you can do that: mule.el is preloaded, while gv.el
> isn't.
>
> It isn't a catastrophe to temporarily switch keyboard encoding "the
> dull way".
>

OK, done.


>
> > +               ;; Use Latin-1 instead of no-conversion to avoid
> > +               ;; flicker due to `set-keyboard-coding-system' changing
> > +               ;; the meta mode.
>
> Ah, so that's the problem...  Did you try raw-text instead?
>
>
Same issue, with both raw-text and no-conversion the
set-keyboard-coding-system function switches the meta mode between t and
nil, leading to flicker.
I guess if the meta mode was nil initially, it would flicker with latin-1.
I'm quite puzzled about this behavior and the meta mode in general now. It
seems that for this purpose (reading a single byte from the terminal) both
nil (ignore top bit) and t (treat top bit as meta) seem wrong, but both
lead to the correct result (i.e. the byte is returned as-is, without
interpretation of the top bit).
Ideally there were a function to read a single byte, ignoring the meta mode
and all conversions.


> And anyway, doesn't latin-1 give you trouble for bytes in the 128..159
> range?
>
>
It works at least in HTerm without flicker or other issues.

[-- Attachment #2: Type: text/html, Size: 2959 bytes --]

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 1152 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am Sa., 19. März 2016 um
18:15 Uhr:

> Eli Zaretskii <eliz@gnu.org> schrieb am Di., 15. März 2016 um 18:57 Uhr:
>
>> > From: Philipp Stephani <p.stephani2@gmail.com>
>> > Date: Mon, 14 Mar 2016 23:03:21 +0000
>> > Cc: 23009@debbugs.gnu.org
>> >
>> > Added a patch. I've had to use latin-1 instead of no-conversion to
>> prevent resetting the meta mode.
>>
>> Not sure I understand the problem you had with no-conversion.  Can you
>> elaborate?
>>
>> > --- a/lisp/international/mule.el
>> > +++ b/lisp/international/mule.el
>> > @@ -1484,6 +1484,9 @@ set-keyboard-coding-system
>> >    (set-keyboard-coding-system-internal coding-system terminal)
>> >    (setq keyboard-coding-system coding-system))
>> >
>> > +(gv-define-setter keyboard-coding-system (coding-system &optional
>> terminal)
>> > +  `(set-keyboard-coding-system ,coding-system ,terminal))
>>
>> I don't think you can do that: mule.el is preloaded, while gv.el
>> isn't.
>>
>> It isn't a catastrophe to temporarily switch keyboard encoding "the
>> dull way".
>>
>
> OK, done.
>
>

Here's a new patch.

[-- Attachment #1.2: Type: text/html, Size: 2011 bytes --]

[-- Attachment #2: 0001-Add-customization-option-for-UTF-8-coordinates.patch --]
[-- Type: application/octet-stream, Size: 13969 bytes --]

From 2c8e431fe96d8755220eeff647aa5485fe238416 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Mon, 14 Mar 2016 23:20:21 +0100
Subject: [PATCH] Add customization option for UTF-8 coordinates

* international/mule.el (keyboard-coding-system): Define as generalized
variable place.
* lisp/xt-mouse.el (xterm-mouse-utf-8): New customization option.
(xterm-mouse--read-coordinate): New function to replace
`xterm-mouse--read-utf8-char'; uses UTF-8 only if enabled.
(xterm-mouse--read-number-from-terminal): Adapt to new name.
(xterm-mouse-tracking-enable-sequence)
(xterm-mouse-tracking-disable-sequence): Replace constants with
functions, mark constants as obsolete.
(xterm-mouse--tracking-sequence): New helper function.
(turn-on-xterm-mouse-tracking-on-terminal): Use new functions;
enable UTF-8 only if customization option says so; store UTF-8
flag in terminal parameter.
* test/automated/xt-mouse-tests.el: Add tests for xt-mouse.el.
---
 lisp/xt-mouse.el                 | 128 ++++++++++++++++++++++++++++++---------
 test/automated/xt-mouse-tests.el | 110 +++++++++++++++++++++++++++++++++
 2 files changed, 208 insertions(+), 30 deletions(-)
 create mode 100644 test/automated/xt-mouse-tests.el

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 5975e60..b6738b2 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -134,23 +134,34 @@ xterm-mouse-truncate-wrap
             (fdiff (- f (* 1.0 maxwrap dbig))))
        (+ (truncate fdiff) (* maxwrap dbig))))))
 
-(defun xterm-mouse--read-utf8-char (&optional prompt seconds)
-  "Read an utf-8 encoded character from the current terminal.
-This function reads and returns an utf-8 encoded character of
-command input. If the user generates an event which is not a
-character (i.e., a mouse click or function key event), read-char
-signals an error.
-
-The returned event may come directly from the user, or from a
-keyboard macro. It is not decoded by the keyboard's input coding
-system and always treated with an utf-8 input encoding.
-
-The optional arguments PROMPT and SECONDS work like in
-`read-event'."
-  (let ((tmp (keyboard-coding-system)))
-    (set-keyboard-coding-system 'utf-8)
-    (prog1 (read-event prompt t seconds)
-      (set-keyboard-coding-system tmp))))
+(defcustom xterm-mouse-utf-8 nil
+  "Non-nil if UTF-8 coordinates should be used to read mouse coordinates.
+Set this to non-nil if you are sure that your terminal
+understands UTF-8 coordinates, but not SGR coordinates."
+  :type 'boolean
+  :risky t
+  :group 'xterm)
+
+(defun xterm-mouse--read-coordinate ()
+  "Read a mouse coordinate from the current terminal.
+If `xterm-mouse-utf-8' was non-nil when
+`turn-on-xterm-mouse-tracking-on-terminal' was called, reads the
+coordinate as an UTF-8 code unit sequence; otherwise, reads a
+single byte."
+  (let ((previous-keyboard-coding-system (keyboard-coding-system)))
+    (unwind-protect
+        (progn
+          (set-keyboard-coding-system
+           (if (terminal-parameter nil 'xterm-mouse-utf-8)
+               'utf-8-unix
+             ;; Use Latin-1 instead of no-conversion to avoid flicker
+             ;; due to `set-keyboard-coding-system' changing the meta
+             ;; mode.
+             'latin-1))
+          ;; Wait only a little; we assume that the entire escape sequence
+          ;; has already been sent when this function is called.
+          (read-char nil nil 0.1))
+      (set-keyboard-coding-system previous-keyboard-coding-system))))
 
 ;; In default mode, each numeric parameter of XTerm's mouse report is
 ;; a single char, possibly encoded as utf-8.  The actual numeric
@@ -170,7 +181,7 @@ xterm-mouse--read-number-from-terminal
                    (<= ?0 c ?9))
             (setq n (+ (* 10 n) c (- ?0))))
           (cons n c))
-      (cons (- (setq c (xterm-mouse--read-utf8-char)) 32) c))))
+      (cons (- (setq c (xterm-mouse--read-coordinate)) 32) c))))
 
 ;; XTerm reports mouse events as
 ;; <EVENT-CODE> <X> <Y> in default mode, and
@@ -314,6 +325,38 @@ xterm-mouse-mode
     (mapc #'turn-off-xterm-mouse-tracking-on-terminal (terminal-list))
     (setq mouse-position-function nil)))
 
+(defun xterm-mouse-tracking-enable-sequence ()
+  "Return a control sequence to enable XTerm mouse tracking.
+The returned control sequence enables basic mouse tracking, mouse
+motion events and finally extended tracking on terminals that
+support it.  The following escape sequences are understood by
+modern xterms:
+
+\"\\e[?1000h\" \"Basic mouse mode\": Enables reports for mouse
+            clicks.  There is a limit to the maximum row/column
+            position (<= 223), which can be reported in this
+            basic mode.
+
+\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events during dragging operations.
+
+\"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an
+            extension to the basic mouse mode, which uses UTF-8
+            characters to overcome the 223 row/column limit.
+            This extension may conflict with non UTF-8
+            applications or non UTF-8 locales.  It is only
+            enabled when the option `xterm-mouse-utf-8' is
+            non-nil.
+
+\"\\e[?1006h\" \"SGR coordinate extension\": Enables a newer
+            alternative extension to the basic mouse mode, which
+            overcomes the 223 row/column limit without the
+            drawbacks of the UTF-8 coordinate extension.
+
+The two extension modes are mutually exclusive, where the last
+given escape sequence takes precedence over the former."
+  (apply #'concat (xterm-mouse--tracking-sequence ?h)))
+
 (defconst xterm-mouse-tracking-enable-sequence
   "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"
   "Control sequence to enable xterm mouse tracking.
@@ -343,10 +386,34 @@ xterm-mouse-tracking-enable-sequence
 The two extension modes are mutually exclusive, where the last
 given escape sequence takes precedence over the former.")
 
+(make-obsolete-variable
+ 'xterm-mouse-tracking-enable-sequence
+ "use the function `xterm-mouse-tracking-enable-sequence' instead."
+ "25.1")
+
+(defun xterm-mouse-tracking-disable-sequence ()
+  "Return a control sequence to disable XTerm mouse tracking.
+The control sequence resets the modes set by
+`xterm-mouse-tracking-enable-sequence'."
+  (apply #'concat (nreverse (xterm-mouse--tracking-sequence ?l))))
+
 (defconst xterm-mouse-tracking-disable-sequence
   "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"
   "Reset the modes set by `xterm-mouse-tracking-enable-sequence'.")
 
+(make-obsolete-variable
+ 'xterm-mouse-tracking-disable-sequence
+ "use the function `xterm-mouse-tracking-disable-sequence' instead."
+ "25.1")
+
+(defun xterm-mouse--tracking-sequence (suffix)
+  "Return a control sequence to enable or disable mouse tracking.
+SUFFIX is the last character of each escape sequence (?h to
+enable, ?l to disable)."
+  (mapcar
+   (lambda (code) (format "\e[?%d%c" code suffix))
+   `(1000 1002 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
+
 (defun turn-on-xterm-mouse-tracking-on-terminal (&optional terminal)
   "Enable xterm mouse tracking on TERMINAL."
   (when (and xterm-mouse-mode (eq t (terminal-live-p terminal))
@@ -360,18 +427,19 @@ turn-on-xterm-mouse-tracking-on-terminal
       (with-selected-frame (car (frames-on-display-list terminal))
         (define-key input-decode-map "\e[M" 'xterm-mouse-translate)
         (define-key input-decode-map "\e[<" 'xterm-mouse-translate-extended))
-      (condition-case err
-          (send-string-to-terminal xterm-mouse-tracking-enable-sequence
-                                   terminal)
-        ;; FIXME: This should use a dedicated error signal.
-        (error (if (equal (cadr err) "Terminal is currently suspended")
-                   nil                  ;The sequence will be sent upon resume.
-                 (signal (car err) (cdr err)))))
-      (push xterm-mouse-tracking-enable-sequence
-            (terminal-parameter nil 'tty-mode-set-strings))
-      (push xterm-mouse-tracking-disable-sequence
-            (terminal-parameter nil 'tty-mode-reset-strings))
-      (set-terminal-parameter terminal 'xterm-mouse-mode t))))
+      (let ((enable (xterm-mouse-tracking-enable-sequence))
+            (disable (xterm-mouse-tracking-disable-sequence)))
+        (condition-case err
+            (send-string-to-terminal enable terminal)
+          ;; FIXME: This should use a dedicated error signal.
+          (error (if (equal (cadr err) "Terminal is currently suspended")
+                     nil ; The sequence will be sent upon resume.
+                   (signal (car err) (cdr err)))))
+        (push enable (terminal-parameter nil 'tty-mode-set-strings))
+        (push disable (terminal-parameter nil 'tty-mode-reset-strings))
+        (set-terminal-parameter terminal 'xterm-mouse-mode t)
+        (set-terminal-parameter terminal 'xterm-mouse-utf-8
+                                xterm-mouse-utf-8)))))
 
 (defun turn-off-xterm-mouse-tracking-on-terminal (terminal)
   "Disable xterm mouse tracking on TERMINAL."
diff --git a/test/automated/xt-mouse-tests.el b/test/automated/xt-mouse-tests.el
new file mode 100644
index 0000000..c7e835c
--- /dev/null
+++ b/test/automated/xt-mouse-tests.el
@@ -0,0 +1,110 @@
+;;; xt-mouse-tests.el --- Test suite for xt-mouse.  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; Author: Philipp Stephani <phst@google.com>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'xt-mouse)
+
+(defmacro with-xterm-mouse-mode (&rest body)
+  "Run BODY with `xterm-mouse-mode' temporarily enabled."
+  (declare (indent 0))
+  ;; Make the frame huge so that the test input events below don't hit
+  ;; the menu bar.
+  `(cl-letf (((frame-width nil) 2000)
+             ((frame-height nil) 2000)
+             ;; Reset XTerm parameters so that the tests don't get
+             ;; confused.
+             ((terminal-parameter nil 'xterm-mouse-x) nil)
+             ((terminal-parameter nil 'xterm-mouse-y) nil)
+             ((terminal-parameter nil 'xterm-mouse-last-down) nil)
+             ((terminal-parameter nil 'xterm-mouse-last-click) nil))
+     (if xterm-mouse-mode
+         (progn ,@body)
+       (unwind-protect
+           (progn
+             ;; `xterm-mouse-mode' doesn't work in the initial
+             ;; terminal.  Since we can't create a second terminal in
+             ;; batch mode, fake it temporarily.
+             (cl-letf (((symbol-function 'terminal-name)
+                        (lambda (&optional _terminal) "fake-terminal")))
+               (xterm-mouse-mode))
+             ,@body)
+         (xterm-mouse-mode 0)))))
+
+(ert-deftest xt-mouse-tracking-basic ()
+  (should (equal (xterm-mouse-tracking-enable-sequence)
+                 "\e[?1000h\e[?1002h\e[?1006h"))
+  (should (equal (xterm-mouse-tracking-disable-sequence)
+                 "\e[?1006l\e[?1002l\e[?1000l"))
+  (with-xterm-mouse-mode
+    (should xterm-mouse-mode)
+    (should (terminal-parameter nil 'xterm-mouse-mode))
+    (should-not (terminal-parameter nil 'xterm-mouse-utf-8))
+    (let* ((unread-command-events (append "\e[M%\xD9\x81"
+                                          "\e[M'\xD9\x81" nil))
+           (key (read-key)))
+      (should (consp key))
+      (cl-destructuring-bind (event-type position . rest) key
+        (should (equal event-type 'S-mouse-2))
+        (should (consp position))
+        (cl-destructuring-bind (_ _ xy . rest) position
+          (should (equal xy '(184 . 95))))))))
+
+(ert-deftest xt-mouse-tracking-utf-8 ()
+  (let ((xterm-mouse-utf-8 t))
+    (should (equal (xterm-mouse-tracking-enable-sequence)
+                   "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"))
+    (should (equal (xterm-mouse-tracking-disable-sequence)
+                   "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"))
+    (with-xterm-mouse-mode
+      (should xterm-mouse-mode)
+      (should (terminal-parameter nil 'xterm-mouse-mode))
+      (should (terminal-parameter nil 'xterm-mouse-utf-8))
+      ;; The keyboard driver doesn't decode bytes in
+      ;; `unread-command-events'.
+      (let* ((unread-command-events (append "\e[M%\u0640\u0131"
+                                            "\e[M'\u0640\u0131" nil))
+             (key (read-key)))
+        (should (consp key))
+        (cl-destructuring-bind (event-type position . rest) key
+          (should (equal event-type 'S-mouse-2))
+          (should (consp position))
+          (cl-destructuring-bind (_ _ xy . rest) position
+            (should (equal xy '(1567 . 271)))))))))
+
+(ert-deftest xt-mouse-tracking-sgr ()
+  (with-xterm-mouse-mode
+    (should xterm-mouse-mode)
+    (should (terminal-parameter nil 'xterm-mouse-mode))
+    (should-not (terminal-parameter nil 'xterm-mouse-utf-8))
+    (let* ((unread-command-events (append "\e[<5;1569;273;M"
+                                          "\e[<5;1569;273;m" nil))
+           (key (read-key)))
+      (should (consp key))
+      (cl-destructuring-bind (event-type position . rest) key
+        (should (equal event-type 'S-mouse-2))
+        (should (consp position))
+        (cl-destructuring-bind (_ _ xy . rest) position
+          (should (equal xy '(1568 . 271))))))))
+
+;;; xt-mouse-tests.el ends here
-- 
2.7.0

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 19 Mar 2016 17:16:13 +0000
> Cc: 23009@debbugs.gnu.org
> 
> Here's a new patch. 

Thanks, pushed.

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Fr., 25. März 2016 um 11:19 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 19 Mar 2016 17:16:13 +0000
> > Cc: 23009@debbugs.gnu.org
> >
> > Here's a new patch.
>
> Thanks, pushed.
>

Thanks, confirmed that it's now working in HTerm.

We still might consider solving the flicker problem for "no-conversion".
Honestly I don't understand the behavior of the meta mode at all: it seems
that for the purpose of read-char the meta-mode is completely ignored? If
so, would it make sense to use set-keyboard-coding-system-internal, which
doesn't appear to set the meta-mode?

[-- Attachment #2: Type: text/html, Size: 1105 bytes --]

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 26 Mar 2016 17:31:46 +0000
> Cc: 23009-done@debbugs.gnu.org
> 
> We still might consider solving the flicker problem for "no-conversion". Honestly I don't understand the
> behavior of the meta mode at all: it seems that for the purpose of read-char the meta-mode is completely
> ignored? If so, would it make sense to use set-keyboard-coding-system-internal, which doesn't appear to set
> the meta-mode?

Hmm... what does current-input-mode return on HTerm in "emacs -Q"?  I
expect to see a non-nil, non-t value in the 3rd element of its return
value.  If that's so, then testing that value for identity with the
one we want to pass to set-input-meta-mode, and avoiding the latter
call if the mode is already what we want, might avoid the flickering.

Can you see if this is doable?

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 26. März 2016 um 19:08 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 26 Mar 2016 17:31:46 +0000
> > Cc: 23009-done@debbugs.gnu.org
> >
> > We still might consider solving the flicker problem for "no-conversion".
> Honestly I don't understand the
> > behavior of the meta mode at all: it seems that for the purpose of
> read-char the meta-mode is completely
> > ignored? If so, would it make sense to use
> set-keyboard-coding-system-internal, which doesn't appear to set
> > the meta-mode?
>
> Hmm... what does current-input-mode return on HTerm in "emacs -Q"?


(t nil 0 7)


>   I
> expect to see a non-nil, non-t value in the 3rd element of its return
> value.  If that's so, then testing that value for identity with the
> one we want to pass to set-input-meta-mode, and avoiding the latter
> call if the mode is already what we want, might avoid the flickering.
>
>
Yes, that's already the case (set-input-meta-mode doesn't reinitialize the
terminal if the meta mode doesn't change), and it's why I use latin-1
instead of no-conversion. With latin-1, a single mouse click results in 8
invocations of (set-input-meta-mode 8) (two mouse events with two
coordinates each, and a set and reset per coordinate). With no-conversion,
the same click results in four sequences of (set-input-meta-mode t)
(set-input-meta-mode
8), which causes the flicker.

[-- Attachment #2: Type: text/html, Size: 2314 bytes --]

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 26 Mar 2016 22:26:17 +0000
> Cc: 23009@debbugs.gnu.org
> 
>  Hmm... what does current-input-mode return on HTerm in "emacs -Q"?
> 
> (t nil 0 7)
> 
>  I
>  expect to see a non-nil, non-t value in the 3rd element of its return
>  value. If that's so, then testing that value for identity with the
>  one we want to pass to set-input-meta-mode, and avoiding the latter
>  call if the mode is already what we want, might avoid the flickering.
> 
> Yes, that's already the case (set-input-meta-mode doesn't reinitialize the terminal if the meta mode doesn't
> change), and it's why I use latin-1 instead of no-conversion. With latin-1, a single mouse click results in 8
> invocations of (set-input-meta-mode 8) (two mouse events with two coordinates each, and a set and reset
> per coordinate). With no-conversion, the same click results in four sequences of (set-input-meta-mode t)
> (set-input-meta-mode 8), which causes the flicker.

Then I guess I'm confused about the reason for (set-input-meta-mode t)
in the case of no-conversion -- don't we want Emacs to pass the 8th
bit through without interpreting it?  Perhaps Handa-san could comment
on this, as I'm otherwise inclined to add raw-text to the list of the
coding-system types that want a non-nil, non-t value to be passed to
set-input-meta-mode.

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> Date: Sun, 27 Mar 2016 18:21:10 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 23009@debbugs.gnu.org
> 
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 26 Mar 2016 22:26:17 +0000
> > Cc: 23009@debbugs.gnu.org
> > 
> >  Hmm... what does current-input-mode return on HTerm in "emacs -Q"?
> > 
> > (t nil 0 7)
> > 
> >  I
> >  expect to see a non-nil, non-t value in the 3rd element of its return
> >  value. If that's so, then testing that value for identity with the
> >  one we want to pass to set-input-meta-mode, and avoiding the latter
> >  call if the mode is already what we want, might avoid the flickering.
> > 
> > Yes, that's already the case (set-input-meta-mode doesn't reinitialize the terminal if the meta mode doesn't
> > change), and it's why I use latin-1 instead of no-conversion. With latin-1, a single mouse click results in 8
> > invocations of (set-input-meta-mode 8) (two mouse events with two coordinates each, and a set and reset
> > per coordinate). With no-conversion, the same click results in four sequences of (set-input-meta-mode t)
> > (set-input-meta-mode 8), which causes the flicker.
> 
> Then I guess I'm confused about the reason for (set-input-meta-mode t)
> in the case of no-conversion -- don't we want Emacs to pass the 8th
> bit through without interpreting it?  Perhaps Handa-san could comment
> on this, as I'm otherwise inclined to add raw-text to the list of the
> coding-system types that want a non-nil, non-t value to be passed to
> set-input-meta-mode.

Ping!

Philipp, can you try adding raw-text to that list, and see if the
result works correctly?

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 1823 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 2. Apr. 2016 um 11:43 Uhr:

> > Date: Sun, 27 Mar 2016 18:21:10 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: 23009@debbugs.gnu.org
> >
> > > From: Philipp Stephani <p.stephani2@gmail.com>
> > > Date: Sat, 26 Mar 2016 22:26:17 +0000
> > > Cc: 23009@debbugs.gnu.org
> > >
> > >  Hmm... what does current-input-mode return on HTerm in "emacs -Q"?
> > >
> > > (t nil 0 7)
> > >
> > >  I
> > >  expect to see a non-nil, non-t value in the 3rd element of its return
> > >  value. If that's so, then testing that value for identity with the
> > >  one we want to pass to set-input-meta-mode, and avoiding the latter
> > >  call if the mode is already what we want, might avoid the flickering.
> > >
> > > Yes, that's already the case (set-input-meta-mode doesn't reinitialize
> the terminal if the meta mode doesn't
> > > change), and it's why I use latin-1 instead of no-conversion. With
> latin-1, a single mouse click results in 8
> > > invocations of (set-input-meta-mode 8) (two mouse events with two
> coordinates each, and a set and reset
> > > per coordinate). With no-conversion, the same click results in four
> sequences of (set-input-meta-mode t)
> > > (set-input-meta-mode 8), which causes the flicker.
> >
> > Then I guess I'm confused about the reason for (set-input-meta-mode t)
> > in the case of no-conversion -- don't we want Emacs to pass the 8th
> > bit through without interpreting it?  Perhaps Handa-san could comment
> > on this, as I'm otherwise inclined to add raw-text to the list of the
> > coding-system types that want a non-nil, non-t value to be passed to
> > set-input-meta-mode.
>
> Ping!
>
> Philipp, can you try adding raw-text to that list, and see if the
> result works correctly?
>

Yes, it works (unsurprisingly), I've attached a patch.

[-- Attachment #1.2: Type: text/html, Size: 2624 bytes --]

[-- Attachment #2: 0002-Simplify-8-bit-character-handling-for-raw-text.patch --]
[-- Type: application/octet-stream, Size: 4799 bytes --]

From 3ded3ff4d38e66bd69d219c0a00edad02feedac6 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sat, 2 Apr 2016 21:07:00 +0200
Subject: [PATCH 2/2] Simplify 8-bit character handling for 'raw-text.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

'raw-text shouldn’t incur any conversion, so it should pass through the
high bit without interpreting it.

* lisp/xt-mouse.el (xterm-mouse--read-coordinate): Use 'no-conversion
instead of 'latin-1.
* lisp/international/mule.el (set-keyboard-coding-system): Treat
‘raw-text’ as another coding type that requires 8-bit characters.
---
 lisp/international/mule.el | 65 +++++++++++++++++++++-------------------------
 lisp/xt-mouse.el           |  5 +---
 2 files changed, 30 insertions(+), 40 deletions(-)

diff --git a/lisp/international/mule.el b/lisp/international/mule.el
index 60a90ae..21ab7e1 100644
--- a/lisp/international/mule.el
+++ b/lisp/international/mule.el
@@ -1445,42 +1445,35 @@ set-keyboard-coding-system
   (let ((coding-type (coding-system-type coding-system))
 	(saved-meta-mode
 	 (terminal-parameter terminal 'keyboard-coding-saved-meta-mode)))
-    (if (not (eq coding-type 'raw-text))
-	(let (accept-8-bit)
-	  (if (not (or (coding-system-get coding-system :suitable-for-keyboard)
-		       (coding-system-get coding-system :ascii-compatible-p)))
-	      (error "Unsuitable coding system for keyboard: %s" coding-system))
-	  (cond ((memq coding-type '(charset utf-8 shift-jis big5 ccl))
-		 (setq accept-8-bit t))
-		((eq coding-type 'iso-2022)
-		 (let ((flags (coding-system-get coding-system :flags)))
-		   (or (memq '7-bit flags)
-		       (setq accept-8-bit t))))
-		(t
-		 (error "Unsupported coding system for keyboard: %s"
-			coding-system)))
-	  (if accept-8-bit
-	      (progn
-		(or saved-meta-mode
-		    (set-terminal-parameter terminal
-					    'keyboard-coding-saved-meta-mode
-					    (cons (nth 2 (current-input-mode))
-						  nil)))
-		(set-input-meta-mode 8 terminal))
-	    (when saved-meta-mode
-	      (set-input-meta-mode (car saved-meta-mode) terminal)
-	      (set-terminal-parameter terminal
-				      'keyboard-coding-saved-meta-mode
-				      nil)))
-	  ;; Avoid end-of-line conversion.
-	  (setq coding-system
-		(coding-system-change-eol-conversion coding-system 'unix)))
-
-      (when saved-meta-mode
-	(set-input-meta-mode (car saved-meta-mode) terminal)
-	(set-terminal-parameter terminal
-				'keyboard-coding-saved-meta-mode
-				nil))))
+    (let (accept-8-bit)
+      (if (not (or (coding-system-get coding-system :suitable-for-keyboard)
+                   (coding-system-get coding-system :ascii-compatible-p)))
+          (error "Unsuitable coding system for keyboard: %s" coding-system))
+      (cond ((memq coding-type '(raw-text charset utf-8 shift-jis big5 ccl))
+             (setq accept-8-bit t))
+            ((eq coding-type 'iso-2022)
+             (let ((flags (coding-system-get coding-system :flags)))
+               (or (memq '7-bit flags)
+                   (setq accept-8-bit t))))
+            (t
+             (error "Unsupported coding system for keyboard: %s"
+                    coding-system)))
+      (if accept-8-bit
+          (progn
+            (or saved-meta-mode
+                (set-terminal-parameter terminal
+                                        'keyboard-coding-saved-meta-mode
+                                        (cons (nth 2 (current-input-mode))
+                                              nil)))
+            (set-input-meta-mode 8 terminal))
+        (when saved-meta-mode
+          (set-input-meta-mode (car saved-meta-mode) terminal)
+          (set-terminal-parameter terminal
+                                  'keyboard-coding-saved-meta-mode
+                                  nil)))
+      ;; Avoid end-of-line conversion.
+      (setq coding-system
+            (coding-system-change-eol-conversion coding-system 'unix))))
   (set-keyboard-coding-system-internal coding-system terminal)
   (setq keyboard-coding-system coding-system))
 
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index e520957..a2b6401 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -155,10 +155,7 @@ xterm-mouse--read-coordinate
           (set-keyboard-coding-system
            (if (terminal-parameter nil 'xterm-mouse-utf-8)
                'utf-8-unix
-             ;; Use Latin-1 instead of no-conversion to avoid flicker
-             ;; due to `set-keyboard-coding-system' changing the meta
-             ;; mode.
-             'latin-1))
+             'no-conversion))
           ;; Wait only a little; we assume that the entire escape sequence
           ;; has already been sent when this function is called.
           (read-char nil nil 0.1))
-- 
2.7.4

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 02 Apr 2016 19:10:45 +0000
> Cc: 23009@debbugs.gnu.org
> 
>  Ping!
> 
>  Philipp, can you try adding raw-text to that list, and see if the
>  result works correctly?
> 
> Yes, it works (unsurprisingly), I've attached a patch. 

Thanks.  Let's wait for a few more days, to let Handa-san an
opportunity to comment on that.

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> Date: Sat, 02 Apr 2016 22:53:14 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 23009@debbugs.gnu.org
> 
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 02 Apr 2016 19:10:45 +0000
> > Cc: 23009@debbugs.gnu.org
> > 
> >  Ping!
> > 
> >  Philipp, can you try adding raw-text to that list, and see if the
> >  result works correctly?
> > 
> > Yes, it works (unsurprisingly), I've attached a patch. 
> 
> Thanks.  Let's wait for a few more days, to let Handa-san an
> opportunity to comment on that.

Forgot to CC him.  Any comments?

bug#23009: 25.0.92; xterm-mouse-mode should not assume UTF-8 coordinates

[Eli Zaretskii]

> Date: Sat, 02 Apr 2016 22:53:14 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 23009@debbugs.gnu.org
> 
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 02 Apr 2016 19:10:45 +0000
> > Cc: 23009@debbugs.gnu.org
> > 
> >  Ping!
> > 
> >  Philipp, can you try adding raw-text to that list, and see if the
> >  result works correctly?
> > 
> > Yes, it works (unsurprisingly), I've attached a patch. 
> 
> Thanks.  Let's wait for a few more days, to let Handa-san an
> opportunity to comment on that.

No further comments, so I pushed this to the emacs-25 branch.

Thanks.