bug#33399: 25.2; xclip fails to set X selection when using tramp

bug#33399: 25.2; xclip fails to set X selection when using tramp

[Ian Mackinnon]

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

When visiting a file with tramp and killing text with xclip-mode
enabled, the text gets sent to the kill-ring but the X clipboard does
not get set. Additionally, the xclip shell process does not close.

Steps to reproduce:

-   Ensure xclip is installed (In the shell: `which xclip`)
-   Install xclip Emacs package
-   Set variables `x-select-enable-clipboard` and/or
`x-select-enable-primary` to `t`.
-   Enable xclip (`M-x xclip-mode RET`)
-   Visit a file using tramp (eg. `/sudo:user@localhost:/tmp/tramp-file`)
-   Type some text
-   Select a region and kill it

Expected behaviour

-   Selected text gets sent to the kill-ring and X clipboard or primary
    buffer (this can be verified in the shell with `xclip -o -selection
primary` and/or
    `xclip -o -selection clipboard`.
-   xclip processes exit cleanly

Observed behaviour

-   Selected text gets sent to the kill-ring, but not the X clipboard or
    primary buffers.
-   xclip processes do not terminate (user is warned that they are still
    running when exiting Emacs).
-   Further, yanking text at this point causes the contents of the X
    clipboard or primary buffer to be inserted at the tip of the
    kill-ring so that gets yanked instead of the killed text.

Workarounds

As suggested in Emacs StackExchange by user Stefan
(https://emacs.stackexchange.com/q/45991/7189) replacing
`start-file-process` with `start-process` in a copy of the xclip 1.4
source fixes this particular issue though whether this causes any
unwanted side effects has not been extensively tested.

--

In GNU Emacs 25.2.2 (x86_64-pc-linux-gnu, GTK+ Version 3.23.2)
 of 2018-08-27, modified by Debian built on lgw01-amd64-037
System Description: Ubuntu 18.10

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/25.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --build x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/25.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-x=yes --with-x-toolkit=gtk3
 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
 -fdebug-prefix-map=/build/emacs-nDyT48/emacs-25.2+1=.
-fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro''

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

Important settings:
  value of $LC_COLLATE: C
  value of $LC_MONETARY: en_GB.UTF-8
  value of $LC_NUMERIC: en_GB.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  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
  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.
Quit
C-x C-g is undefined

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache 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 term/xterm xterm
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 89350 6746)
 (symbols 48 19877 0)
 (miscs 40 40 121)
 (strings 32 14378 4594)
 (string-bytes 1 411363)
 (vectors 16 9777)
 (vector-slots 8 385958 16769)
 (floats 8 166 359)
 (intervals 56 247 0)
 (buffers 976 18))

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

bug#33399: 25.2; xclip fails to set X selection when using tramp

[Stefan Monnier]

Hi Leo,

> As suggested in Emacs StackExchange by user Stefan
> (https://emacs.stackexchange.com/q/45991/7189) replacing
> `start-file-process` with `start-process` in a copy of the xclip 1.4
> source fixes this particular issue though whether this causes any
> unwanted side effects has not been extensively tested.

xclip.el uses start-file-processes everywhere nowadays, largely because
of changes I introduced, but I only used it because the previous code
used it, not because I thought it was needed.

Tracking the origin of start-file-processes in xclip.el I see it was
introduced by the following change:

    commit f160382d9dd3acb36f562e82993e07765411c354
    Author: Leo Liu <sdl.web@gmail.com>
    Date:   Fri Sep 6 12:32:20 2013 +0800
    
        * xclip.el: Use pbcopy and pbpaste if available
        
        (xclip-use-pbcopy&paste): New variable.
        (xclip-set-selection, xclip-selection-value, xclip-mode): Use it.

[ I appended the corresponding diff after my sig for reference.  ]

Do you remember why you used start-file-processes there?


        Stefan


diff --git a/packages/xclip/xclip.el b/packages/xclip/xclip.el
index d08dcd255..bc9b64057 100644
--- a/packages/xclip/xclip.el
+++ b/packages/xclip/xclip.el
@@ -5,7 +5,7 @@
 ;; Author: Leo Liu <sdl.web@gmail.com>
 ;; Keywords: convenience, tools
 ;; Created: 2007-12-30
-;; Version: 1.1
+;; Version: 1.2
 
 ;; This program is free software; you can redistribute it and/or modify
 ;; it under the terms of the GNU General Public License as published by
@@ -41,6 +41,15 @@ This is in addition to, but in preference to, the primary selection."
   :type 'boolean
   :group 'killing)
 
+(defcustom xclip-use-pbcopy&paste (and xclip-select-enable-clipboard
+                                       (eq system-type 'darwin)
+                                       (executable-find "pbcopy")
+                                       t)
+  "Non-nil means using pbcopy and pbpaste instead of xclip.
+If non-nil `xclip-program' is ignored."
+  :type 'boolean
+  :group 'killing)
+
 (defvar xclip-last-selected-text-clipboard nil
   "The value of the CLIPBOARD X selection from xclip.")
 
@@ -51,12 +60,17 @@ This is in addition to, but in preference to, the primary selection."
   "TYPE is a symbol: primary, secondary and clipboard.
 
 See also `x-set-selection'."
-  (when (getenv "DISPLAY")
-    (let* ((process-connection-type nil)
-           (proc (start-process "xclip" nil xclip-program
-                                "-selection" (symbol-name type))))
+  (let* ((process-connection-type nil)
+         (proc (cond
+                (xclip-use-pbcopy&paste
+                 (start-file-process "pbcopy" nil "pbcopy"))
+                ((getenv "DISPLAY")
+                 (start-file-process "xclip" nil xclip-program
+                                     "-selection" (symbol-name type))))))
+    (when proc
       (process-send-string proc data)
-      (process-send-eof proc))))
+      (process-send-eof proc))
+    data))
 
 (defun xclip-select-text (text)
   "See `x-select-text'."
@@ -68,24 +82,28 @@ See also `x-set-selection'."
 
 (defun xclip-selection-value ()
   "See `x-selection-value'."
-  (when (getenv "DISPLAY")
-    (let ((clip-text (when xclip-select-enable-clipboard
-                       (with-output-to-string
+  (let ((clip-text (when xclip-select-enable-clipboard
+                     (with-output-to-string
+                       (cond
+                        (xclip-use-pbcopy&paste
+                         (process-file "pbpaste" nil standard-output nil))
+                        ((getenv "DISPLAY")
                          (process-file xclip-program nil standard-output nil
-                                       "-o" "-selection" "clipboard")))))
-      (setq clip-text
-            (cond                       ; Check clipboard selection.
-             ((or (not clip-text) (string= clip-text ""))
-              (setq xclip-last-selected-text-clipboard nil))
-             ((eq clip-text xclip-last-selected-text-clipboard)
-              nil)
-             ((string= clip-text xclip-last-selected-text-clipboard)
-              ;; Record the newer string so subsequent calls can use
-              ;; the `eq' test.
-              (setq xclip-last-selected-text-clipboard clip-text)
-              nil)
-             (t (setq xclip-last-selected-text-clipboard clip-text))))
-      (or clip-text
+                                       "-o" "-selection" "clipboard")))))))
+    (setq clip-text
+          (cond                         ; Check clipboard selection.
+           ((or (not clip-text) (string= clip-text ""))
+            (setq xclip-last-selected-text-clipboard nil))
+           ((eq clip-text xclip-last-selected-text-clipboard)
+            nil)
+           ((string= clip-text xclip-last-selected-text-clipboard)
+            ;; Record the newer string so subsequent calls can use the
+            ;; `eq' test.
+            (setq xclip-last-selected-text-clipboard clip-text)
+            nil)
+           (t (setq xclip-last-selected-text-clipboard clip-text))))
+    (or clip-text
+        (when (getenv "DISPLAY")
           (let ((primary-text (with-output-to-string
                                 (process-file xclip-program nil
                                               standard-output nil "-o"))))
@@ -113,9 +131,11 @@ See also `x-set-selection'."
   :global t
   (if xclip-mode
       (progn
-        (or (executable-find xclip-program)
+        (or xclip-use-pbcopy&paste
+            (executable-find xclip-program)
             (signal 'file-error (list "Searching for program"
                                       xclip-program "no such file")))
+        ;; NOTE: See `tty-run-terminal-initialization' and term/README
         (add-hook 'terminal-init-xterm-hook 'turn-on-xclip))
     (remove-hook 'terminal-init-xterm-hook 'turn-on-xclip)))
 

bug#33399: 25.2; xclip fails to set X selection when using tramp

[Leo Liu]

Hi all,

Sorry for the delay.

On 2018-11-15 13:37 -0500, Stefan Monnier wrote:
> Do you remember why you used start-file-processes there?
>
>
>         Stefan

start-file-process looks like an odd choice and I can't remember why it
was chosen. Feel free to change it.

Leo

bug#33399: 25.2; xclip fails to set X selection when using tramp

[Stefan Monnier]

> start-file-process looks like an odd choice and I can't remember why it
> was chosen. Feel free to change it.

OK, thanks.  I just pushed a corresponding change.
Should appear in v1.5 soon.


        Stefan