From: Dmitry Gutov <dmitry@gutov.dev>
To: Spencer Baugh <sbaugh@janestreet.com>
Cc: mail@daniel-mendler.de, Juri Linkov <juri@linkov.net>,
48356@debbugs.gnu.org, monnier@iro.umontreal.ca,
jdtsmith@gmail.com, Visuwesh <visuweshm@gmail.com>,
Eli Zaretskii <eliz@gnu.org>
Subject: bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary
Date: Sat, 20 Apr 2024 03:12:59 +0300 [thread overview]
Message-ID: <616b1604-f21e-482b-9991-68559bcc0545@gutov.dev> (raw)
In-Reply-To: <ierplumbveq.fsf@janestreet.com>
[-- Attachment #1: Type: text/plain, Size: 2704 bytes --]
On 18/04/2024 17:25, Spencer Baugh wrote:
>>> But then the user could change the mind
>>> and still select a candidate. This would interfere with the
>>> contents of the minibuffer.
>>
>> Suppose they do. But the list of completions they're shown is for an
>> outdated input. Does it really make more sense to erase the current
>> input than to insert the completion where it was supposed to go?
>
> Yeah, this patch changes the behavior in the case of an outdated
> *Completions* buffer, and the new behavior is buggy, but I think the old
> behavior was also buggy and the new behavior is better.
>
> Example: In emacs -Q, if I type
>
> C-x C-f ~/src/emacs/emacs-2 <TAB>
>
> I get completions "emacs-28" and "emacs-29"
>
> Suppose I then type /src (without hitting tab or updating the
> *Completions* buffer) so that the minibuffer contains:
>
> ~/src/emacs/emacs-2/src
>
> Then I click on one of the completions to choose it.
>
> Before this patch the minibuffer will contain:
>
> ~/src/emacs/emacs-28/
>
> and after this patch it will contain
>
> ~/src/emacs/emacs-28//src
>
> Both of these are wrong IMO, but IMO the second one is closer to
> correct, and it's more consistent with the normal case (when
> *Completions* is up to date) and with in-buffer completion behavior, so
> I think this patch is an improvement and could be installed.
>
> Still, maybe we can get it to the point where clicking on an outdated
> completion makes the minibuffer contain
>
> ~/src/emacs/emacs-28/src
>
> instead? Which I think is the correct behavior.
>
> But I don't think we need to do that before landing this patch.
That gave me an idea. With a bit more spaghetti, we can support both
this scenario and the others mentioned previously that didn't have field
boundaries in the "new" input:
Whenever POINT > END, we can re-query the completion boundaries again
inside completion-list-insert-choice-function and adjust END. In theory,
I guess we'd ideally also recompute the completion entity first (to pass
the correct prefix and suffix), but the capf function is not in context
anymore. As long as the field boundaries simply looks for chars in
passed string, this should work fine.
The attached v4 adds a new step and also fixes an issue apparently
introduced in 2021 with 0ce2f591ff9 when making
minibuffer-completion-table and others buffer-local. The bindings for
CTABLE and other 2 vars currently always set them to nil because the
variables are looked up right after the current buffer is changed
(with-current-buffer standard-output ...).
This can affect the completion--done call at the end, reverting it to
the previous behavior. But nobody noticed the change, so...
[-- Attachment #2: base-suffix-v4.diff --]
[-- Type: text/x-patch, Size: 12043 bytes --]
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index ad6a0928cda..61395577035 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -112,20 +112,6 @@ completion-boundaries
(cons (or (cadr boundaries) 0)
(or (cddr boundaries) (length suffix)))))
-(defun completion-base-suffix (start end collection predicate)
- "Return suffix of completion of buffer text between START and END.
-COLLECTION and PREDICATE are, respectively, the completion's
-completion table and predicate, as in `completion-boundaries' (which see).
-Value is a substring of buffer text between point and END. It is
-the completion suffix that follows the completion boundary."
- (let ((suffix (buffer-substring (point) end)))
- (substring
- suffix
- (cdr (completion-boundaries (buffer-substring start (point))
- collection
- predicate
- suffix)))))
-
(defun completion-metadata (string table pred)
"Return the metadata of elements to complete at the end of STRING.
This metadata is an alist. Currently understood keys are:
@@ -1377,7 +1363,7 @@ completion--replace
(setq newtext (substring newtext 0 (- suffix-len))))
(goto-char beg)
(let ((length (- end beg))) ;Read `end' before we insert the text.
- (insert-and-inherit newtext)
+ (insert-before-markers-and-inherit newtext)
(delete-region (point) (+ (point) length)))
(forward-char suffix-len)))
@@ -2598,17 +2584,23 @@ minibuffer-completion-help
(base-size (or (cdr last) 0))
(prefix (unless (zerop base-size) (substring string 0 base-size)))
(minibuffer-completion-base (substring string 0 base-size))
- (base-prefix (buffer-substring (minibuffer--completion-prompt-end)
- (+ start base-size)))
- (base-suffix (concat (completion-base-suffix start end
- minibuffer-completion-table
- minibuffer-completion-predicate)
- (buffer-substring end (point-max))))
+ (ctable minibuffer-completion-table)
+ (cpred minibuffer-completion-predicate)
+ (cprops completion-extra-properties)
+ (field-end
+ (save-excursion
+ (forward-char
+ (cdr (completion-boundaries (buffer-substring start (point))
+ ctable
+ cpred
+ (buffer-substring (point) end))))
+ (point-marker)))
+ (field-char (and (< field-end end) (char-after field-end)))
(all-md (completion--metadata (buffer-substring-no-properties
start (point))
base-size md
- minibuffer-completion-table
- minibuffer-completion-predicate))
+ ctable
+ cpred))
(ann-fun (completion-metadata-get all-md 'annotation-function))
(aff-fun (completion-metadata-get all-md 'affixation-function))
(sort-fun (completion-metadata-get all-md 'display-sort-function))
@@ -2687,38 +2679,31 @@ minibuffer-completion-help
(with-current-buffer standard-output
(setq-local completion-base-position
- (list (+ start base-size)
- ;; FIXME: We should pay attention to completion
- ;; boundaries here, but currently
- ;; completion-all-completions does not give us the
- ;; necessary information.
- end))
- (setq-local completion-base-affixes
- (list base-prefix base-suffix))
+ (list (+ start base-size) field-end))
(setq-local completion-list-insert-choice-function
- (let ((ctable minibuffer-completion-table)
- (cpred minibuffer-completion-predicate)
- (cprops completion-extra-properties))
(lambda (start end choice)
- (if (and (stringp start) (stringp end))
- (progn
- (delete-minibuffer-contents)
- (insert start choice)
- ;; Keep point after completion before suffix
- (save-excursion (insert
- (completion--merge-suffix
- choice
- (1- (length choice))
- end))))
- (unless (or (zerop (length prefix))
- (equal prefix
- (buffer-substring-no-properties
- (max (point-min)
- (- start (length prefix)))
- start)))
- (message "*Completions* out of date"))
- ;; FIXME: Use `md' to do quoting&terminator here.
- (completion--replace start end choice))
+ (unless (or (zerop (length prefix))
+ (equal prefix
+ (buffer-substring-no-properties
+ (max (point-min)
+ (- start (length prefix)))
+ start)))
+ (message "*Completions* out of date"))
+ (when (> (point) end)
+ ;; Completion suffix has changed, have to adapt.
+ (setq end (+ end
+ (cdr (completion-boundaries
+ (concat prefix choice) ctable cpred
+ (buffer-substring end (point))))))
+ ;; Stopped before some field boundary.
+ (when (> (point) end)
+ (setq field-char (char-after end))))
+ (when (and field-char
+ (= (aref choice (1- (length choice)))
+ field-char))
+ (setq end (1+ end)))
+ ;; FIXME: Use `md' to do quoting&terminator here.
+ (completion--replace start end choice)
(let* ((minibuffer-completion-table ctable)
(minibuffer-completion-predicate cpred)
(completion-extra-properties cprops)
@@ -2729,7 +2714,7 @@ minibuffer-completion-help
;; completion is not finished.
(completion--done result
(if (eq (car bounds) (length result))
- 'exact 'finished)))))))
+ 'exact 'finished))))))
(display-completion-list completions nil group-fun)))))
nil)))
@@ -4877,8 +4862,7 @@ minibuffer-next-completion
(next-line-completion (or n 1))
(next-completion (or n 1)))
(when auto-choose
- (let ((completion-use-base-affixes t)
- (completion-auto-deselect nil))
+ (let ((completion-auto-deselect nil))
(choose-completion nil t t))))))
(defun minibuffer-previous-completion (&optional n)
@@ -4916,8 +4900,7 @@ minibuffer-choose-completion
minibuffer, but don't quit the completions window."
(interactive "P")
(with-minibuffer-completions-window
- (let ((completion-use-base-affixes t))
- (choose-completion nil no-exit no-quit))))
+ (choose-completion nil no-exit no-quit)))
(defun minibuffer-choose-completion-or-exit (&optional no-exit no-quit)
"Choose the completion from the minibuffer or exit the minibuffer.
diff --git a/lisp/simple.el b/lisp/simple.el
index e4629ce3db7..741cf59f344 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9856,16 +9856,6 @@ completion-base-position
where the completion should be inserted and END (if non-nil) is the end
of the text to replace. If END is nil, point is used instead.")
-(defvar completion-base-affixes nil
- "Base context of the text corresponding to the shown completions.
-This variable is used in the *Completions* buffer.
-Its value is a list of the form (PREFIX SUFFIX) where PREFIX is the text
-before the place where completion should be inserted, and SUFFIX is the text
-after the completion.")
-
-(defvar completion-use-base-affixes nil
- "Non-nil means to restore original prefix and suffix in the minibuffer.")
-
(defvar completion-list-insert-choice-function #'completion--replace
"Function to use to insert the text chosen in *Completions*.
Called with three arguments (BEG END TEXT), it should replace the text
@@ -10126,7 +10116,6 @@ choose-completion
(with-current-buffer (window-buffer (posn-window (event-start event)))
(let ((buffer completion-reference-buffer)
(base-position completion-base-position)
- (base-affixes completion-base-affixes)
(insert-function completion-list-insert-choice-function)
(completion-no-auto-exit (if no-exit t completion-no-auto-exit))
(choice
@@ -10159,13 +10148,7 @@ choose-completion
(with-current-buffer buffer
(choose-completion-string
choice buffer
- ;; Don't allow affixes to replace the whole buffer when not
- ;; in the minibuffer. Thus check for `completion-in-region-mode'
- ;; to ignore non-nil value of `completion-use-base-affixes' set by
- ;; `minibuffer-choose-completion'.
- (or (and (not completion-in-region-mode)
- completion-use-base-affixes base-affixes)
- base-position
+ (or base-position
;; If all else fails, just guess.
(list (choose-completion-guess-base-position choice)))
insert-function)))))
@@ -10321,11 +10304,9 @@ completion-setup-function
(buffer-substring (minibuffer-prompt-end) (point)))))))
(with-current-buffer standard-output
(let ((base-position completion-base-position)
- (base-affixes completion-base-affixes)
(insert-fun completion-list-insert-choice-function))
(completion-list-mode)
(setq-local completion-base-position base-position)
- (setq-local completion-base-affixes base-affixes)
(setq-local completion-list-insert-choice-function insert-fun))
(setq-local completion-reference-buffer mainbuf)
(if base-dir (setq default-directory base-dir))
next prev parent reply other threads:[~2024-04-20 0:12 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-11 17:23 bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary Daniel Mendler
2022-03-13 17:56 ` Juri Linkov
2022-03-13 20:35 ` bug#48356: [External] : " Drew Adams
2022-03-14 3:10 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-03-14 18:53 ` Juri Linkov
2022-03-14 20:55 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-03-15 2:14 ` Daniel Mendler
2022-03-15 7:53 ` Juri Linkov
2022-03-20 20:34 ` Juri Linkov
2024-04-08 21:59 ` Dmitry Gutov
2024-04-08 22:27 ` Dmitry Gutov
2024-04-08 23:50 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-10 1:33 ` Dmitry Gutov
2024-04-10 2:38 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-11 1:00 ` Dmitry Gutov
2024-04-11 6:55 ` Eli Zaretskii
2024-04-11 10:36 ` Dmitry Gutov
2024-04-11 21:59 ` Dmitry Gutov
2024-04-14 16:44 ` Juri Linkov
2024-04-14 23:55 ` Dmitry Gutov
2024-04-18 14:25 ` Spencer Baugh
2024-04-20 0:12 ` Dmitry Gutov [this message]
2024-05-04 2:23 ` Dmitry Gutov
2024-05-09 2:33 ` Dmitry Gutov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=616b1604-f21e-482b-9991-68559bcc0545@gutov.dev \
--to=dmitry@gutov.dev \
--cc=48356@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=jdtsmith@gmail.com \
--cc=juri@linkov.net \
--cc=mail@daniel-mendler.de \
--cc=monnier@iro.umontreal.ca \
--cc=sbaugh@janestreet.com \
--cc=visuweshm@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).