bug#31209: 27.0.50; current-kill calls interprogram-paste-function more than once

bug#31209: 27.0.50; current-kill calls interprogram-paste-function more than once

[Basil L. Contovounesios]

Under lexical-binding, I expect the following:

(let* ((i 0)
       (kill-ring ())
       (kill-ring-yank-pointer ())
       (save-interprogram-paste-before-kill t)
       (interprogram-paste-function
        (lambda ()
          (setq i (1+ i))
          (list "a" "b" "c"))))
  (current-kill 0)
  (message "\
`interprogram-paste-function' called %d time(s)
`kill-ring' = %s"
           i kill-ring))

to evaluate to:

‘interprogram-paste-function’ called 1 time(s)
‘kill-ring’ = (a b c)

but the actual result is:

‘interprogram-paste-function’ called 4 time(s)
‘kill-ring’ = (a a b c b a b c c a b c)

This behaviour is due to current-kill passing each string returned by
interprogram-paste-function to kill-new, which in turn calls
interprogram-paste-function when save-interprogram-paste-before-kill is
non-nil, and was introduced along with
save-interprogram-paste-before-kill [1: 4ed8c7aadb].

[1: 4ed8c7aadb]: 2009-08-26 20:55:39 +0000
  (save-interprogram-paste-before-kill): New user option.
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4ed8c7aadb59c8ae75641710e3d9ff5bddbbd952

Patch addressing this to follow.

Thanks,

-- 
Basil

In GNU Emacs 27.0.50 (build 6, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2018-04-18 built on thunk
Repository revision: 5dff4905d73d0d42447ff4b114d1af726a689c6a
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Debian GNU/Linux buster/sid

bug#31209: 27.0.50; current-kill calls interprogram-paste-function more than once

[Basil L. Contovounesios]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Do-not-call-interprogram-paste-function-repeatedly.patch --]
[-- Type: text/x-diff, Size: 1260 bytes --]

From a82c662f645b82f6e9425160b586e8c607dd9a3e Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Thu, 12 Apr 2018 17:46:37 +0100
Subject: [PATCH 1/2] Do not call interprogram-paste-function repeatedly

* lisp/simple.el (current-kill): Disable interprogram-paste-function
so that kill-new doesn't call it repeatedly when
save-interprogram-paste-before-kill is enabled. (bug#31209)
---
 lisp/simple.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index b51be3a8f8..15c4a92804 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -4463,7 +4463,10 @@ current-kill
 	  ;; Disable the interprogram cut function when we add the new
 	  ;; text to the kill ring, so Emacs doesn't try to own the
 	  ;; selection, with identical text.
-	  (let ((interprogram-cut-function nil))
+          ;; Also disable the interprogram paste function, so that
+          ;; `kill-new' doesn't call it repeatedly.
+          (let ((interprogram-cut-function nil)
+                (interprogram-paste-function nil))
 	    (if (listp interprogram-paste)
                 ;; Use `reverse' to avoid modifying external data.
                 (mapc #'kill-new (reverse interprogram-paste))
-- 
2.17.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Improve-kill-related-documentation-bug-31209.patch --]
[-- Type: text/x-diff, Size: 5620 bytes --]

From 15d1a619039254df5ba1a43aa97e011bdb40175c Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Wed, 18 Apr 2018 18:35:38 +0100
Subject: [PATCH 2/2] Improve kill-related documentation (bug#31209)

* doc/lispref/text.texi (Low-Level Kill Ring): Fix typo under
current-kill.  Mention interprogram-paste-function under kill-new
and kill-append.
* lisp/simple.el (save-interprogram-paste-before-kill, kill-new)
(kill-append-merge-undo, kill-append): Touch-up docstrings.
---
 doc/lispref/text.texi | 14 +++++++++-----
 lisp/simple.el        | 31 +++++++++++++++++--------------
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 8cb6cf6242..da09b4ae1c 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -1143,7 +1143,7 @@ Low-Level Kill Ring
 @code{current-kill} calls the value of
 @code{interprogram-paste-function} (documented below) before
 consulting the kill ring.  If that value is a function and calling it
-returns a string or a list of several string, @code{current-kill}
+returns a string or a list of several strings, @code{current-kill}
 pushes the strings onto the kill ring and returns the first string.
 It also sets the yanking pointer to point to the kill-ring entry of
 the first string returned by @code{interprogram-paste-function},
@@ -1156,8 +1156,10 @@ Low-Level Kill Ring
 @defun kill-new string &optional replace
 This function pushes the text @var{string} onto the kill ring and
 makes the yanking pointer point to it.  It discards the oldest entry
-if appropriate.  It also invokes the value of
-@code{interprogram-cut-function} (see below).
+if appropriate.  It also invokes the values of
+@code{interprogram-paste-function} (subject to
+the user option @code{save-interprogram-paste-before-kill})
+and @code{interprogram-cut-function} (see below).
 
 If @var{replace} is non-@code{nil}, then @code{kill-new} replaces the
 first element of the kill ring with @var{string}, rather than pushing
@@ -1169,8 +1171,10 @@ Low-Level Kill Ring
 kill ring and makes the yanking pointer point to the combined entry.
 Normally @var{string} goes at the end of the entry, but if
 @var{before-p} is non-@code{nil}, it goes at the beginning.  This
-function also invokes the value of @code{interprogram-cut-function}
-(see below).
+function calls @code{kill-new} as a subroutine, thus causing the
+values of @code{interprogram-cut-function} and possibly
+@code{interprogram-paste-function} (see below) to be invoked by
+extension.
 @end defun
 
 @defvar interprogram-paste-function
diff --git a/lisp/simple.el b/lisp/simple.el
index 15c4a92804..8d04471a63 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -4341,12 +4341,11 @@ kill-ring-yank-pointer
   "The tail of the kill ring whose car is the last thing yanked.")
 
 (defcustom save-interprogram-paste-before-kill nil
-  "Save clipboard strings into kill ring before replacing them.
-When one selects something in another program to paste it into Emacs,
-but kills something in Emacs before actually pasting it,
-this selection is gone unless this variable is non-nil,
-in which case the other program's selection is saved in the `kill-ring'
-before the Emacs kill and one can still paste it using \\[yank] \\[yank-pop]."
+  "Save existing clipboard text into kill ring before replacing it.
+A non-nil value ensures that Emacs kill operations do not
+irrevocably overwrite existing clipboard text by saving it to the
+`kill-ring' prior to the kill.  Such text can subsequently be
+retrieved via \\[yank] \\[yank-pop]]."
   :type 'boolean
   :group 'killing
   :version "23.2")
@@ -4366,7 +4365,7 @@ kill-new
 the front of the kill ring, rather than being added to the list.
 
 When `save-interprogram-paste-before-kill' and `interprogram-paste-function'
-are non-nil, saves the interprogram paste string(s) into `kill-ring' before
+are non-nil, save the interprogram paste string(s) into `kill-ring' before
 STRING.
 
 When the yank handler has a non-nil PARAM element, the original STRING
@@ -4402,20 +4401,24 @@ kill-new
   (if interprogram-cut-function
       (funcall interprogram-cut-function string)))
 
-;; It has been argued that this should work similar to `self-insert-command'
-;; which merges insertions in undo-list in groups of 20 (hard-coded in cmds.c).
+;; It has been argued that this should work like `self-insert-command'
+;; which merges insertions in `buffer-undo-list' in groups of 20
+;; (hard-coded in `undo-auto-amalgamate').
 (defcustom kill-append-merge-undo nil
-  "Whether appending to kill ring also makes \\[undo] restore both pieces of text simultaneously."
+  "Amalgamate appending kills with the last kill for undo.
+When non-nil, appending or prepending text to the last kill makes
+\\[undo] restore both pieces of text simultaneously."
   :type 'boolean
   :group 'killing
   :version "25.1")
 
 (defun kill-append (string before-p)
   "Append STRING to the end of the latest kill in the kill ring.
-If BEFORE-P is non-nil, prepend STRING to the kill.
-Also removes the last undo boundary in the current buffer,
- depending on `kill-append-merge-undo'.
-If `interprogram-cut-function' is set, pass the resulting kill to it."
+If BEFORE-P is non-nil, prepend STRING to the kill instead.
+If `interprogram-cut-function' is non-nil, call it with the
+resulting kill.
+If `kill-append-merge-undo' is non-nil, remove the last undo
+boundary in the current buffer."
   (let* ((cur (car kill-ring)))
     (kill-new (if before-p (concat string cur) (concat cur string))
 	      (or (= (length cur) 0)
-- 
2.17.0


[-- Attachment #3: Type: text/plain, Size: 450 bytes --]


"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Patch addressing this to follow.

I attach two patches.

The first makes current-kill disable interprogram-paste-function in
addition to interprogram-cut-function before calling kill-new.

The second suggests potential clarifications in related documentation,
both in '(elisp) Low-Level Kill Ring' and simple.el docstrings.
Hopefully someone can further improve upon these.

Thanks,

-- 
Basil

bug#31209: 27.0.50; current-kill calls interprogram-paste-function more than once

[Basil L. Contovounesios]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> The second suggests potential clarifications in related documentation,
> both in '(elisp) Low-Level Kill Ring' and simple.el docstrings.
> Hopefully someone can further improve upon these.

I just noticed one more thing.  The docstring of
interprogram-paste-function ends with:

  Note that the function should return a string only if a program
  other than Emacs has provided a string for pasting; if Emacs
  provided the most recent string, the function should return nil.
  If it is difficult to tell whether Emacs or some other program
  provided the current string, it is probably good enough to return
  nil if the string is equal (according to `string=') to the last
                                            ^^^^^^^
  text Emacs provided.

Would equal-including-properties (as per kill-do-not-save-duplicates) be
more accurate here, or is string= fine?

-- 
Basil

bug#31209: 27.0.50; current-kill calls interprogram-paste-function more than once

[Noam Postavsky]

tags 31209 fixed
close 31209 27.1
quit

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> The first makes current-kill disable interprogram-paste-function in
> addition to interprogram-cut-function before calling kill-new.

Pushed to master [1: 532f5fb387].

> The second suggests potential clarifications in related documentation,
> both in '(elisp) Low-Level Kill Ring' and simple.el docstrings.
> Hopefully someone can further improve upon these.

Pushed to emacs-26 [2: 343d70b10e].

[1: 532f5fb387]: 2018-04-25 07:37:32 -0400
  Do not call interprogram-paste-function repeatedly
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=532f5fb3877ccd535a822f7c3c08d396621b4325

[2: 343d70b10e]: 2018-04-24 20:58:15 -0400
  Improve kill-related documentation (bug#31209)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=343d70b10edbd42ebe49ada3a8ef878b2ee044f0


> I just noticed one more thing.  The docstring of
> interprogram-paste-function ends with:
> 
>   Note that the function should return a string only if a program
>   other than Emacs has provided a string for pasting; if Emacs
>   provided the most recent string, the function should return nil.
>   If it is difficult to tell whether Emacs or some other program
>   provided the current string, it is probably good enough to return
>   nil if the string is equal (according to `string=') to the last
>                                             ^^^^^^^
>   text Emacs provided.
> 
> Would equal-including-properties (as per kill-do-not-save-duplicates) be
> more accurate here, or is string= fine?

I guess if we're talking about strings coming from other programs,
properties are unlikely to show up.