bug#17899: 24.3.92.1; dabbrev-completion incorrectly searches all buffers

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#17899: 24.3.92.1; dabbrev-completion incorrectly searches all buffers

Stefan Guath
VERSION:
GNU Emacs 24.3.92.1 (x86_64-apple-darwin13.2.0, NS apple-appkit-1265.20)

TO REPRODUCE BUG:
* emacs -Q
* Create two buffers (C-x b) with the following contents:
foo.el: "idris0 idris1"
bar.el: "ideal0 ideal1"
* Go to end of foo.el, write "id" and execute dabbrev-completion (C-M-/)

EXPECTED RESULT
"id" should complete to "idris". The doc of dabbrev-completion is very clear on this:

"Like M-/ but finds all expansions in the current buffer
and presents suggestions for completion.

With a prefix argument ARG, it searches all buffers accepted by the
function pointed out by `dabbrev-friend-buffer-function' to find the
completions.

If the prefix argument is 16 (which comes from C-u C-u),
then it searches *all* buffers."

No prefix was given here, so it should just look in the current buffer.

ACTUAL RESULT
A new buffer opens with the text:

"In this buffer, type RET to select the completion near point.

Possible completions are:
ideal0                                 ideal1
idris0                                 idris1"

Apparently, the buffer bar.el was incorrectly taken into account. After killing buffer bar.el, dabbrev-completion completes as expected in foo.el.


Reply | Threaded
Open this post in threaded view
|

bug#17899: Source of bug?

Stefan Guath
`dabbrev-completion' dynamically binds `dabbrev-check-other-buffers' to the correct value, but later down the call chain (i.e. `dabbrev--find-all-expansions' -> `dabbrev--find-expansion' -> `dabbrev--make-friend-buffer-list'), `dabbrev--make-friend-buffer-list' instead uses the internal `dabbrev--check-other-buffers' which is not updated accordingly. Could this be the source of the error?

/Stefan Guath


Reply | Threaded
Open this post in threaded view
|

bug#17899: 24.3.92.1; dabbrev-completion incorrectly searches all buffers

Alan Third
In reply to this post by Stefan Guath
Stefan Guath <[hidden email]> writes:

> TO REPRODUCE BUG:
> * emacs -Q
> * Create two buffers (C-x b) with the following contents:
> foo.el: "idris0 idris1"
> bar.el: "ideal0 ideal1"
> * Go to end of foo.el, write "id" and execute dabbrev-completion (C-M-/)
>
> EXPECTED RESULT
> "id" should complete to "idris".

Hi, sorry it took so long for someone to look at this.

I believe the attached patch fixes it. The problem seems to have been
switching dabbrev.el over to using lexical scoping. It was previously
using let* to temporarily override a couple of global variables, which
works with dynamic scoping, but not lexical.


From 40946b12d9583e87b89a3c942ed418990cc35355 Mon Sep 17 00:00:00 2001
From: Alan Third <[hidden email]>
Date: Thu, 5 Dec 2019 13:14:00 +0000
Subject: [PATCH] Fix dabbrev-completion (bug#17899)

* lisp/dabbrev.el (dabbrev--check-all-buffers): Add new variable.
(dabbrev-completion): Lexical scoping means we can't use let to
override global variables, so use setq.
(dabbrev--reset-global-variables): Reset new variable.
(dabbrev--make-friend-buffer-list): Use new variable.
* test/lisp/dabbrev-tests.el (dabbrev-completion-test):
(dabbrev-completion-test-with-argument): New tests.
---
 lisp/dabbrev.el            | 16 ++++++++++------
 test/lisp/dabbrev-tests.el | 30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/lisp/dabbrev.el b/lisp/dabbrev.el
index 23abe7ae16..0cb9b0b824 100644
--- a/lisp/dabbrev.el
+++ b/lisp/dabbrev.el
@@ -323,6 +323,9 @@ dabbrev--last-case-pattern
 ;; Same as dabbrev-check-other-buffers, but is set for every expand.
 (defvar dabbrev--check-other-buffers dabbrev-check-other-buffers)
 
+;; Same as dabbrev-check-all-buffers, but is set for every expand.
+(defvar dabbrev--check-all-buffers dabbrev-check-all-buffers)
+
 ;; The regexp for recognizing a character in an abbreviation.
 (defvar dabbrev--abbrev-char-regexp nil)
 
@@ -380,10 +383,7 @@ dabbrev-completion
 then it searches *all* buffers."
   (interactive "*P")
   (dabbrev--reset-global-variables)
-  (let* ((dabbrev-check-other-buffers (and arg t))
- (dabbrev-check-all-buffers
-  (and arg (= (prefix-numeric-value arg) 16)))
- (abbrev (dabbrev--abbrev-at-point))
+  (let* ((abbrev (dabbrev--abbrev-at-point))
          (beg (progn (search-backward abbrev) (point)))
          (end (progn (search-forward abbrev) (point)))
  (ignore-case-p (dabbrev--ignore-case-p abbrev))
@@ -420,6 +420,9 @@ dabbrev-completion
                            (t
                             (mapcar #'downcase completion-list)))))))
               (complete-with-action a list s p)))))
+    (setq dabbrev--check-other-buffers (and arg t))
+    (setq dabbrev--check-all-buffers
+          (and arg (= (prefix-numeric-value arg) 16)))
     (completion-in-region beg end table)))
 
 ;;;###autoload
@@ -623,7 +626,8 @@ dabbrev--reset-global-variables
  dabbrev--last-buffer-found nil
  dabbrev--abbrev-char-regexp (or dabbrev-abbrev-char-regexp
  "\\sw\\|\\s_")
- dabbrev--check-other-buffers dabbrev-check-other-buffers))
+ dabbrev--check-other-buffers dabbrev-check-other-buffers
+        dabbrev--check-all-buffers dabbrev-check-all-buffers))
 
 (defun dabbrev--select-buffers ()
   "Return a list of other buffers to search for a possible abbrev.
@@ -772,7 +776,7 @@ dabbrev--make-friend-buffer-list
       ;; If dabbrev-check-all-buffers, tack on all the other
       ;; buffers at the end of the list, except those which are
       ;; specifically to be ignored.
-      (if dabbrev-check-all-buffers
+      (if dabbrev--check-all-buffers
   (setq list
  (append list
  (dabbrev-filter-elements
diff --git a/test/lisp/dabbrev-tests.el b/test/lisp/dabbrev-tests.el
index a6ab2e7201..d26362db3d 100644
--- a/test/lisp/dabbrev-tests.el
+++ b/test/lisp/dabbrev-tests.el
@@ -40,3 +40,33 @@ dabbrev-expand-test
      ;; M-/ SPC M-/ M-/
      (execute-kbd-macro "\257 \257\257"))
    (should (string= (buffer-string) "ab  x\nab y\nab  y"))))
+
+(ert-deftest dabbrev-completion-test ()
+  "Test for bug#17899.
+dabbrev-completion should not look for expansions in other
+buffers unless a prefix argument is used."
+  (with-temp-buffer
+    (insert "axy")
+    (with-temp-buffer
+      (insert "abc\na")
+      (goto-char 6)
+      (save-window-excursion
+        (set-window-buffer nil (current-buffer))
+        ;; C-M-/
+        (execute-kbd-macro [201326639]))
+      (should (string= (buffer-string) "abc\nabc")))))
+
+(ert-deftest dabbrev-completion-test-with-argument ()
+  "Test for bug#17899.
+dabbrev-completion should not complete because it has found
+multiple expansions."
+  (with-temp-buffer
+    (insert "axy")
+    (with-temp-buffer
+      (insert "abc\na")
+      (goto-char 6)
+      (save-window-excursion
+        (set-window-buffer nil (current-buffer))
+        ;; C-u C-u C-M-/
+        (execute-kbd-macro [21 21 201326639]))
+      (should (string= (buffer-string) "abc\na")))))
--
2.21.0



--
Alan Third
Reply | Threaded
Open this post in threaded view
|

bug#17899: 24.3.92.1; dabbrev-completion incorrectly searches all buffers

Alan Third
On Thu, Dec 05, 2019 at 01:19:13PM +0000, Alan Third wrote:

> Stefan Guath <[hidden email]> writes:
>
> > TO REPRODUCE BUG:
> > * emacs -Q
> > * Create two buffers (C-x b) with the following contents:
> > foo.el: "idris0 idris1"
> > bar.el: "ideal0 ideal1"
> > * Go to end of foo.el, write "id" and execute dabbrev-completion (C-M-/)
> >
> > EXPECTED RESULT
> > "id" should complete to "idris".
>
> Hi, sorry it took so long for someone to look at this.
>
> I believe the attached patch fixes it. The problem seems to have been
> switching dabbrev.el over to using lexical scoping. It was previously
> using let* to temporarily override a couple of global variables, which
> works with dynamic scoping, but not lexical.

I’ve pushed to master and am therefore closing this bug report.
--
Alan Third