bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

["Mattias Engdegård" <mattiase@acm.org>]

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

tags 36879 patch
quit

Daniel Eklöf skrev:

>Have I configured something wrong? Or is this a bug?

I don't think you did anything wrong; I can reproduce the bug (in
XTerm; I don't have your fancy emulator).

The question is rather, how did this code ever work in the first
place? As you observed, when XTerm sends the reply, it uses BEL as
terminator. Emacs uses BEL (C-g) as INTR char, which means that not
only is special effort required to avoid having it quit the current
elisp code -- this could have been done using inhibit-quit -- but when
the pty receives the BEL from XTerm, it immediately discards unread
characters and raises SIGINT.

Thus, it's very much a race: the only way it could ever work would be
if Emacs has been able to read the entire reply except the BEL, and be
sitting inside (read-char) when the BEL reaches the pty. Needless to
say, this is rather unlikely.

We could tell the tty not to discard the queue upon INTR by setting the
NOFLSH flag, but (1) I don't know how portable that is, (2) it's not
what we normally want when C-g is used interactively, and (3) it would
still process the BEL out-of-order with respect to earlier chars.

Attached is a rather heavy-handed patch which temporarily changes the
quit-char to something unlikely while sending the OSC 52 request and
reading the reply. It also allows the reply to be terminated by ESC \
(ST) as well.

Since XTerm parrots our choice of string terminator (BEL or ST), this
suggests a simpler solution: just use ST, and the trouble with BEL is
no more. Unfortunately the code has provisions for screen/tmux, where
the entire request is wrapped in a DCS request:

 ESC P ... ESC \

which means that we cannot use ST as terminator in that case. However,
I haven't been able to make this facility work with tmux at all, and
with screen only by reverting 4183482f4d (bug#20356) AND explicitly
setting TERM=screen (the default is screen.xterm-256color).

In addition, changing quit-char can be visually annoying; it causes
reinitialisation of the entire tty, something you don't want every time
you press C-y. Perhaps it's fine to drop screen support from this
particular function? I attached another, alternative patch that does
that instead.



[-- Attachment #2: heavy.patch --]
[-- Type: text/x-patch, Size: 3218 bytes --]

From df344bde2bbe459eb5aea668388d2606a38fe12c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sat, 3 Aug 2019 12:08:27 +0200
Subject: [PATCH] Fix XTerm OSC 52 selection retrieval (bug#36879)

When asking XTerm for the selection via OSC 52, set the quit char to
something other than BEL since that char is used as string terminator
in the reply.  Otherwise, the pty will raise SIGINT as soon as it sees
the BEL, discarding any unread chars.

Also allow the reply to be terminated by ST as well as BEL.

* lisp/term/xterm.el (gui-backend-get-selection):
Change quit char temporarily.  Detect ST as string terminator in reply.
Use time-out when reading reply.
---
 lisp/term/xterm.el | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
index c4b0a8fb6e..29460ea55b 100644
--- a/lisp/term/xterm.el
+++ b/lisp/term/xterm.el
@@ -954,14 +954,35 @@ gui-backend-get-selection
          (query (concat "\e]52;" (xterm--selection-char type) ";")))
     (with-temp-buffer
       (set-buffer-multibyte nil)
-      (xterm--query
-       (concat (when screen "\eP") query "?\a" (when screen "\e\\"))
-       (list (cons query (lambda ()
-                           (while (let ((char (read-char)))
-                                    (unless (eq char ?\a)
-                                      (insert char)
-                                      t))))))
-       'no-async)
+      (let ((old-quit-char (nth 3 (current-input-mode))))
+        ;; Temporarily set the quit char to something else, because
+        ;; C-g may be sent from the terminal as a string terminator,
+        ;; and the resulting SIGINT (which occurs when the interrupt
+        ;; char is received by the tty, not when we read it) would
+        ;; flush unread chars from the tty input queue.
+        (unwind-protect
+            (progn
+              (set-quit-char ?\377)
+              (xterm--query
+               (concat (when screen "\eP") query "?\a" (when screen "\e\\"))
+               (list (cons query
+                           (lambda ()
+                             ;; Read data up to the string terminator,
+                             ;; either ST (ESC \) or BEL.
+                             (let (char last)
+                               (while (and (setq char (read-char
+                                                       nil nil
+                                                       xterm-query-timeout))
+                                           (not (or (eq char ?\a)
+                                                    (and (eq char ?\\)
+                                                         (eq last ?\e)))))
+                                 (when last
+                                   (insert last))
+                                 (setq last char))
+                               (when (eq char ?\a)
+                                 (insert last))))))
+               'no-async))
+        (set-quit-char old-quit-char)))
       (base64-decode-region (point-min) (point-max))
       (decode-coding-region (point-min) (point-max) 'utf-8-unix t))))
 
-- 
2.21.0


[-- Attachment #3: light.patch --]
[-- Type: text/x-patch, Size: 2917 bytes --]

From da122da5273a57b25edfe3e8885ea2250b88bf5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sat, 3 Aug 2019 12:08:27 +0200
Subject: [PATCH] Fix XTerm OSC 52 selection retrieval (bug#36879)

When asking XTerm for the selection via OSC 52, use ST as string
terminator in the request to get ST as terminator in the reply,
because BEL is messy to receive in many ways.

* lisp/term/xterm.el (gui-backend-get-selection):
Use ST as string terminator in request and reply.
Use a time-out when reading the reply.
---
 lisp/term/xterm.el | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
index c4b0a8fb6e..4b56b2ce4a 100644
--- a/lisp/term/xterm.el
+++ b/lisp/term/xterm.el
@@ -946,21 +946,31 @@ gui-backend-get-selection
     (type data-type
      &context (window-system nil)
               ;; Only applies to terminals which have it enabled.
-              ((terminal-parameter nil 'xterm--get-selection) (eql t)))
+              ((terminal-parameter nil 'xterm--get-selection) (eql t))
+              ;; Doesn't work in screen; see bug#36879.
+              ((eq (terminal-parameter nil 'terminal-initted)
+                   'terminal-init-screen)
+               (eql nil)))
   (unless (eq data-type 'STRING)
     (error "Unsupported data type %S" data-type))
-  (let* ((screen (eq (terminal-parameter nil 'terminal-initted)
-                     'terminal-init-screen))
-         (query (concat "\e]52;" (xterm--selection-char type) ";")))
+  (let ((query (concat "\e]52;" (xterm--selection-char type) ";")))
     (with-temp-buffer
       (set-buffer-multibyte nil)
       (xterm--query
-       (concat (when screen "\eP") query "?\a" (when screen "\e\\"))
-       (list (cons query (lambda ()
-                           (while (let ((char (read-char)))
-                                    (unless (eq char ?\a)
-                                      (insert char)
-                                      t))))))
+       ;; Use ST as query terminator to get ST as reply terminator (bug#36879).
+       (concat query "?\e\\")
+       (list (cons query
+                   (lambda ()
+                     ;; Read data up to the string terminator, ST.
+                     (let (char last)
+                       (while (and (setq char (read-char
+                                               nil nil
+                                               xterm-query-timeout))
+                                   (not (and (eq char ?\\)
+                                             (eq last ?\e))))
+                         (when last
+                           (insert last))
+                         (setq last char))))))
        'no-async)
       (base64-decode-region (point-min) (point-max))
       (decode-coding-region (point-min) (point-max) 'utf-8-unix t))))
-- 
2.21.0