* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary @ 2021-05-11 17:23 Daniel Mendler 2022-03-13 17:56 ` Juri Linkov 2022-03-14 3:10 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 24+ messages in thread From: Daniel Mendler @ 2021-05-11 17:23 UTC (permalink / raw) To: 48356; +Cc: Stefan Monnier, JD Smith When selecting a candidate the suffix after the completion boundary is discarded by `choose-completion`/`choose-completion-string`. `choose-completion` is invoked when a candidate in the *Completions* buffer is selected with the mouse or RET. For example when completing a file path "~/emacs/master/li|/calc", where "|" is the cursor, and then the candidate "lisp" is selected in the *Completions* buffer, the result is "~/emacs/master/lisp/". The prefix "~/emacs/master/" is prepended to the selected candidate, but the suffix "/calc" is discarded. `choose-completion-string` contains logic which checks if the resulting string equals the car of the completion boundary. In that case the minibuffer is not exited. I propose the following change to the existing logic: When selecting a candidate with `choose-completion` and a suffix is present, the minibuffer should not be exited (completion continues) and the suffix is preserved. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 1 sibling, 1 reply; 24+ messages in thread From: Juri Linkov @ 2022-03-13 17:56 UTC (permalink / raw) To: Daniel Mendler; +Cc: 48356, Stefan Monnier, JD Smith [-- Attachment #1: Type: text/plain, Size: 1572 bytes --] forcemerge 48356 49931 thanks > When selecting a candidate the suffix after the completion boundary is > discarded by `choose-completion`/`choose-completion-string`. > `choose-completion` is invoked when a candidate in the *Completions* > buffer is selected with the mouse or RET. > > For example when completing a file path "~/emacs/master/li|/calc", where > "|" is the cursor, and then the candidate "lisp" is selected in the > *Completions* buffer, the result is "~/emacs/master/lisp/". The prefix > "~/emacs/master/" is prepended to the selected candidate, but the suffix > "/calc" is discarded. > > `choose-completion-string` contains logic which checks if the resulting > string equals the car of the completion boundary. In that case the > minibuffer is not exited. Strange, in your test case above, the minibuffer is not exited already. > I propose the following change to the existing logic: When > selecting a candidate with `choose-completion` and a suffix is present, > the minibuffer should not be exited (completion continues) and the > suffix is preserved. Here is a better patch than was posted to bug#49931 to preserve the suffix. It correctly handles at least three different cases: 1. When manually adding a suffix in the minibuffer after completions were displayed, choose-completion discards that suffix. 2. In file name completion in the above case the suffix is preserved. 3. 'M-! command filename TAB' and choosing a completion preserves both prefix and suffix. This works only after customizing 'completion-use-base-affixes' to t: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: completion-base-affixes.patch --] [-- Type: text/x-diff, Size: 5170 bytes --] diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 36b8d80841..5685f078ad 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -2227,6 +2312,8 @@ minibuffer-completion-help (let* ((last (last completions)) (base-size (or (cdr last) 0)) (prefix (unless (zerop base-size) (substring string 0 base-size))) + (base-prefix (buffer-substring (minibuffer--completion-prompt-end) (+ start base-size))) + (base-suffix (buffer-substring (point) (point-max))) (all-md (completion--metadata (buffer-substring-no-properties start (point)) base-size md @@ -2320,11 +2407,18 @@ minibuffer-completion-help ;; completion-all-completions does not give us the ;; necessary information. end)) + (setq-local completion-base-affixes (list base-prefix base-suffix)) (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 end))) (unless (or (zerop (length prefix)) (equal prefix (buffer-substring-no-properties @@ -2333,7 +2427,7 @@ minibuffer-completion-help start))) (message "*Completions* out of date")) ;; FIXME: Use `md' to do quoting&terminator here. - (completion--replace start end choice) + (completion--replace start end choice)) (let* ((minibuffer-completion-table ctable) (minibuffer-completion-predicate cpred) (completion-extra-properties cprops) diff --git a/lisp/simple.el b/lisp/simple.el index accc119e2b..52cf54c563 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -9071,6 +9075,19 @@ 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* buffers. +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.") + +(defcustom completion-use-base-affixes nil + "Non-nil means to restore original prefix and suffix in the minibuffer." + :type 'boolean + :version "29.1" + :group 'completion) + (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 @@ -9151,6 +9168,7 @@ next-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) (choice (save-excursion @@ -9184,7 +9203,8 @@ choose-completion (with-current-buffer buffer (choose-completion-string choice buffer - (or base-position + (or (and completion-use-base-affixes base-affixes) + base-position ;; If all else fails, just guess. (list (choose-completion-guess-base-position choice))) insert-function))))) @@ -9344,9 +9372,11 @@ 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)) ^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#48356: [External] : bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2022-03-13 17:56 ` Juri Linkov @ 2022-03-13 20:35 ` Drew Adams 0 siblings, 0 replies; 24+ messages in thread From: Drew Adams @ 2022-03-13 20:35 UTC (permalink / raw) To: Juri Linkov, Daniel Mendler Cc: 48356@debbugs.gnu.org, Stefan Monnier, JD Smith FWIW, my opinion (no doubt a minority of one) is that all such approaches, including what's in vanilla Emacs now (since `minibuffer.el', which I guess means Emacs 22/23), are inferior to the original vanilla behavior. Icicles uses that original behavior, in which it _makes no difference where point is_ in the minibuffer content. That is, the entire minibuffer content is the pattern to be matched (whether for completion or reading by `read-from-minibuffer' etc.). I find this more flexible & saner - doesn't matter where point is. Whether or not you've made an edit in the middle of the content, e.g. yanking or typing or deleting there, all of the current text is used. If someone really ever wants the text that follows point not to be taken into account then it's simple enough to hit a key to remove it (and it can be restored with undo etc.). You may say it's also simple enough otherwise to move point to the end of input (e.g. `M-v'). Fair enough, but I think that's more bother. It's more common, I think, to edit text in the middle somewhere, and then either keep the text that follows point or kill/delete it. There's never any need to move point just to get the pattern you want. You never need to pay any attention to point in the minibuffer. ___ [I suppose that in some sense this is kind of a bottle half-full/half-empty choice. Maybe akin to views on `delete-selection-mode': convenience of not having to first use `C-w' (to replace) versus convenience of not having to first use `C-g' (to not replace).] ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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-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 1 sibling, 1 reply; 24+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-03-14 3:10 UTC (permalink / raw) To: Daniel Mendler; +Cc: 48356, JD Smith > For example when completing a file path "~/emacs/master/li|/calc", where > "|" is the cursor, and then the candidate "lisp" is selected in the > *Completions* buffer, the result is "~/emacs/master/lisp/". The prefix > "~/emacs/master/" is prepended to the selected candidate, but the suffix > "/calc" is discarded. Yup. That's listed under "bugs" at the beginning of minibuffer.el: ;; - choose-completion can't automatically figure out the boundaries ;; corresponding to the displayed completions because we only ;; provide the start info but not the end info in ;; completion-base-position. > ... and the suffix is preserved. Of course, the suffix should *not* be preserved when the minibuffer was "r|e-buf". Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 1 reply; 24+ messages in thread From: Juri Linkov @ 2022-03-14 18:53 UTC (permalink / raw) To: 48356; +Cc: mail, monnier, jdtsmith >> For example when completing a file path "~/emacs/master/li|/calc", where >> "|" is the cursor, and then the candidate "lisp" is selected in the >> *Completions* buffer, the result is "~/emacs/master/lisp/". The prefix >> "~/emacs/master/" is prepended to the selected candidate, but the suffix >> "/calc" is discarded. > > Yup. That's listed under "bugs" at the beginning of minibuffer.el: > > ;; - choose-completion can't automatically figure out the boundaries > ;; corresponding to the displayed completions because we only > ;; provide the start info but not the end info in > ;; completion-base-position. > >> ... and the suffix is preserved. > > Of course, the suffix should *not* be preserved when the minibuffer was > "r|e-buf". IMHO, the root of the problem is in completion-all-completions that returns in the last `cdr' only the start of completions, but not the end of completions. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-03-14 20:55 UTC (permalink / raw) To: Juri Linkov; +Cc: mail, 48356, jdtsmith > IMHO, the root of the problem is in completion-all-completions > that returns in the last `cdr' only the start of completions, > but not the end of completions. Agreed. Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 1 reply; 24+ messages in thread From: Daniel Mendler @ 2022-03-15 2:14 UTC (permalink / raw) To: Stefan Monnier, Juri Linkov; +Cc: 48356, jdtsmith On 3/14/22 21:55, Stefan Monnier wrote: >> IMHO, the root of the problem is in completion-all-completions >> that returns in the last `cdr' only the start of completions, >> but not the end of completions. > > Agreed. If I recall correctly, someone wrote a patch which fixed the root cause. Daniel ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2022-03-15 2:14 ` Daniel Mendler @ 2022-03-15 7:53 ` Juri Linkov 2022-03-20 20:34 ` Juri Linkov 0 siblings, 1 reply; 24+ messages in thread From: Juri Linkov @ 2022-03-15 7:53 UTC (permalink / raw) To: Daniel Mendler; +Cc: 48356, Stefan Monnier, JD Smith >>> IMHO, the root of the problem is in completion-all-completions >>> that returns in the last `cdr' only the start of completions, >>> but not the end of completions. >> >> Agreed. > > If I recall correctly, someone wrote a patch which fixed the root cause. The only patch that removes these comments ;; FIXME: We need to additionally return the info needed for the ;; second part of completion-base-position. ;; FIXME: We should pay attention to completion ;; boundaries here, but currently ;; completion-all-completions does not give us the ;; necessary information. is https://lists.gnu.org/archive/html/emacs-devel/2021-08/msg00412.html in bug#47711 and bug#48841. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2022-03-15 7:53 ` Juri Linkov @ 2022-03-20 20:34 ` Juri Linkov 2024-04-08 21:59 ` Dmitry Gutov 0 siblings, 1 reply; 24+ messages in thread From: Juri Linkov @ 2022-03-20 20:34 UTC (permalink / raw) To: Daniel Mendler; +Cc: 48356, Stefan Monnier, JD Smith >>>> IMHO, the root of the problem is in completion-all-completions >>>> that returns in the last `cdr' only the start of completions, >>>> but not the end of completions. >>> >>> Agreed. >> >> If I recall correctly, someone wrote a patch which fixed the root cause. > > The only patch that removes these comments > > ;; FIXME: We need to additionally return the info needed for the > ;; second part of completion-base-position. > > ;; FIXME: We should pay attention to completion > ;; boundaries here, but currently > ;; completion-all-completions does not give us the > ;; necessary information. > > is https://lists.gnu.org/archive/html/emacs-devel/2021-08/msg00412.html > in bug#47711 and bug#48841. I wonder what is the fate of this patch? There was a long discussion without a clear outcome. Maybe it's possible to split the patch to smaller parts where a separate patch would add the feature needed here to return the end position of the completion boundaries? ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 2 replies; 24+ messages in thread From: Dmitry Gutov @ 2024-04-08 21:59 UTC (permalink / raw) To: Juri Linkov, Daniel Mendler; +Cc: 48356, Stefan Monnier, JD Smith [-- Attachment #1: Type: text/plain, Size: 2989 bytes --] On 20/03/2022 22:34, Juri Linkov wrote: >>>>> IMHO, the root of the problem is in completion-all-completions >>>>> that returns in the last `cdr' only the start of completions, >>>>> but not the end of completions. >>>> Agreed. >>> If I recall correctly, someone wrote a patch which fixed the root cause. >> The only patch that removes these comments >> >> ;; FIXME: We need to additionally return the info needed for the >> ;; second part of completion-base-position. >> >> ;; FIXME: We should pay attention to completion >> ;; boundaries here, but currently >> ;; completion-all-completions does not give us the >> ;; necessary information. >> >> ishttps://lists.gnu.org/archive/html/emacs-devel/2021-08/msg00412.html >> in bug#47711 and bug#48841. > I wonder what is the fate of this patch? There was a long discussion > without a clear outcome. Maybe it's possible to split the patch > to smaller parts where a separate patch would add the feature needed here > to return the end position of the completion boundaries? It seems there is a range of solutions we could take here. On the one side would be a replacement of the -all-completions API with something like completion-filter-completions as proposed by Daniel. It's still a reasonable choice, but a rather breaking one, and the patch would need to be majorly rewritten anyway, given the amount of changes in the area since it was submitted. On the other, we could preserve the current convention again, and add a new dynamic variable which would be assigned in every relevant completion style, to store the rightmost-position (the length of suffix). Then the callers would fetch it from that variable. Similar to what we do with completion-lazy-hilit-fn now. Or some variations of that. But what I don't quite see yet, is why wouldn't the caller be able to compute the bounds cheaply enough? We could offer an accessor function. See the new logic in the attached patch (but imagine it extracted to a named function). Note that it doesn't work too well now, because in the example like ~/v/e-|/src the completions include the trailing slash. And base-suffix includes a starting slash as well (according to boundaries returned by the completion table). So when I choose one of the completions using minibuffer-next-completion, the minibuffer contents look like ~/v/emacs-master//src ...which translates to "/" because of the double slash -- the filesystem root directory (*). But that's the same data which would be used by any other proposed solution, too. So maybe it should be either be fixed in the completion table (avoid adding trailing slash when the last boundary is already followed by slash?), or the insertion code should do some additional post-processing of the completion string. (*) And then, when I press tab while point is between slashes -- /|/ -- it jumps to the beginning of the input, but that's a secondary problem. [-- Attachment #2: base-suffix.diff --] [-- Type: text/x-patch, Size: 1515 bytes --] diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 0a844c538b4..33c175aa3c6 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -2582,16 +2582,13 @@ minibuffer-completion-help (minibuffer-completion-base (substring string 0 base-size)) (base-prefix (buffer-substring (minibuffer--completion-prompt-end) (+ start base-size))) - (base-suffix - (if (or (eq (alist-get 'category (cdr md)) 'file) - completion-in-region-mode-predicate) - (buffer-substring - (save-excursion - (if completion-in-region-mode-predicate - (point) - (or (search-forward "/" nil t) (point-max)))) - (point-max)) - "")) + (base-suffix (let ((suffix (buffer-substring (point) end))) + (substring + suffix + (cdr (completion-boundaries string + minibuffer-completion-table + minibuffer-completion-predicate + suffix))))) (all-md (completion--metadata (buffer-substring-no-properties start (point)) base-size md ^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 1 sibling, 0 replies; 24+ messages in thread From: Dmitry Gutov @ 2024-04-08 22:27 UTC (permalink / raw) To: Juri Linkov, Daniel Mendler; +Cc: 48356, Stefan Monnier, JD Smith On 09/04/2024 00:59, Dmitry Gutov wrote: > But what I don't quite see yet, is why wouldn't the caller be able to > compute the bounds cheaply enough? We could offer an accessor function. Reading completion-pcm--find-all-completions, it seems like the case where this wouldn't work is "The prefix has no completions at all ...". What scenarios does this correspond to? If they are limited to the cases where the point jumps to a different field (and then completion has to be repeated), then maybe my patch would still be fine. Otherwise, perhaps one of the other approaches is the way to go. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 1 sibling, 1 reply; 24+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-08 23:50 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 48356, Daniel Mendler, JD Smith, Juri Linkov > ...which translates to "/" because of the double slash -- the filesystem > root directory (*). But that's the same data which would be used by any > other proposed solution, too. More or less, tho the "ideal" solution is to do that in the completion-style code, which has a bit more knowledge about it. > So maybe it should be either be fixed in the > completion table (avoid adding trailing slash when the last boundary is > already followed by slash?), or the insertion code should do some > additional post-processing of the completion string. I think you can fix it in the same ad-hoc way we use elsewhere: compare the first char after the boundary with the last char of the completion and drop one of the two if they're the same. > + (base-suffix (let ((suffix (buffer-substring (point) end))) > + (substring > + suffix > + (cdr (completion-boundaries string > + minibuffer-completion-table > + minibuffer-completion-predicate > + suffix))))) I think you want to be careful to pass (buffer-substring start (point)) rather than `string` to `completion-boundaries`. In theory this approach can "do the wrong thing" with some completion styles, but AFAIK they haven't been written yet. 🙂 Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 1 reply; 24+ messages in thread From: Dmitry Gutov @ 2024-04-10 1:33 UTC (permalink / raw) To: Stefan Monnier; +Cc: 48356, Daniel Mendler, JD Smith, Juri Linkov [-- Attachment #1: Type: text/plain, Size: 2322 bytes --] Hi Stefan, On 09/04/2024 02:50, Stefan Monnier wrote: >> ...which translates to "/" because of the double slash -- the filesystem >> root directory (*). But that's the same data which would be used by any >> other proposed solution, too. > > More or less, tho the "ideal" solution is to do that in the > completion-style code, which has a bit more knowledge about it. Doing it in completion-style, though, would either require a relatively awkward change in most/all styles (e.g. the "new dynamic variable" route), or a more straightforward change in styles together with an incompatible change in completion-all-completions. So on balance, would you say it's a good idea to a) use this approach in minibuffer-completion-help, b) create a named function for it, for other callers to take advantage of it as well? IIUC Vertico (and other minibuffer completion UIs) might be interested. >> So maybe it should be either be fixed in the >> completion table (avoid adding trailing slash when the last boundary is >> already followed by slash?), or the insertion code should do some >> additional post-processing of the completion string. > > I think you can fix it in the same ad-hoc way we use elsewhere: compare > the first char after the boundary with the last char of the completion > and drop one of the two if they're the same. Looks like completion--merge-suffix is the helper to use. >> + (base-suffix (let ((suffix (buffer-substring (point) end))) >> + (substring >> + suffix >> + (cdr (completion-boundaries string >> + minibuffer-completion-table >> + minibuffer-completion-predicate >> + suffix))))) > > I think you want to be careful to pass (buffer-substring start (point)) > rather than `string` to `completion-boundaries`. Thanks, see the update attached. > In theory this approach can "do the wrong thing" with some completion > styles, but AFAIK they haven't been written yet. 🙂 So you figure that such theoretical style would return adjusted base-suffix in -all-completions method, not just in -try-completion? [-- Attachment #2: base-suffix-v2.diff --] [-- Type: text/x-patch, Size: 2456 bytes --] diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 41b20174be1..0cccc46448b 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -2586,16 +2586,13 @@ minibuffer-completion-help (minibuffer-completion-base (substring string 0 base-size)) (base-prefix (buffer-substring (minibuffer--completion-prompt-end) (+ start base-size))) - (base-suffix - (if (or (eq (alist-get 'category (cdr md)) 'file) - completion-in-region-mode-predicate) - (buffer-substring - (save-excursion - (if completion-in-region-mode-predicate - (point) - (or (search-forward "/" nil t) (point-max)))) - (point-max)) - "")) + (base-suffix (let ((suffix (buffer-substring (point) end))) + (substring + suffix + (cdr (completion-boundaries (buffer-substring start (point)) + minibuffer-completion-table + minibuffer-completion-predicate + suffix))))) (all-md (completion--metadata (buffer-substring-no-properties start (point)) base-size md @@ -2697,7 +2694,11 @@ minibuffer-completion-help (delete-minibuffer-contents) (insert start choice) ;; Keep point after completion before suffix - (save-excursion (insert end))) + (save-excursion (insert + (completion--merge-suffix + choice + (1- (length choice)) + end)))) (unless (or (zerop (length prefix)) (equal prefix (buffer-substring-no-properties ^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-10 2:38 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 48356, Daniel Mendler, JD Smith, Juri Linkov >>> ...which translates to "/" because of the double slash -- the filesystem >>> root directory (*). But that's the same data which would be used by any >>> other proposed solution, too. >> More or less, tho the "ideal" solution is to do that in the >> completion-style code, which has a bit more knowledge about it. > Doing it in completion-style, though, would either require a relatively > awkward change in most/all styles (e.g. the "new dynamic variable" route), > or a more straightforward change in styles together with an incompatible > change in completion-all-completions. Yup, hence the quotes around "ideal". > So on balance, would you say it's a good idea to a) use this approach in > minibuffer-completion-help, b) create a named function for it, for other > callers to take advantage of it as well? Yes, while waiting for a new API it seems like a good stop gap. > Looks like completion--merge-suffix is the helper to use. Yup. >> In theory this approach can "do the wrong thing" with some completion >> styles, but AFAIK they haven't been written yet. 🙂 > So you figure that such theoretical style would return adjusted base-suffix > in -all-completions method, not just in -try-completion? A completion style could make use of (and list) things after the boundary. E.g. completing a file name like foo/ba<|>ar/baz could decide to list all the files that match *f*o*o*/*b*a*a*r*/*b*a*z* in which case the "end boundary" of the `completion-all-completions` output should not be the `cdr` of the `completion-boundaries`. I wouldn't worry about it, tho. Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 1 reply; 24+ messages in thread From: Dmitry Gutov @ 2024-04-11 1:00 UTC (permalink / raw) To: Stefan Monnier; +Cc: 48356, Daniel Mendler, JD Smith, Juri Linkov On 10/04/2024 05:38, Stefan Monnier wrote: >>>> ...which translates to "/" because of the double slash -- the filesystem >>>> root directory (*). But that's the same data which would be used by any >>>> other proposed solution, too. >>> More or less, tho the "ideal" solution is to do that in the >>> completion-style code, which has a bit more knowledge about it. >> Doing it in completion-style, though, would either require a relatively >> awkward change in most/all styles (e.g. the "new dynamic variable" route), >> or a more straightforward change in styles together with an incompatible >> change in completion-all-completions. > > Yup, hence the quotes around "ideal". > >> So on balance, would you say it's a good idea to a) use this approach in >> minibuffer-completion-help, b) create a named function for it, for other >> callers to take advantage of it as well? > > Yes, while waiting for a new API it seems like a good stop gap. > >> Looks like completion--merge-suffix is the helper to use. > > Yup. > >>> In theory this approach can "do the wrong thing" with some completion >>> styles, but AFAIK they haven't been written yet. 🙂 >> So you figure that such theoretical style would return adjusted base-suffix >> in -all-completions method, not just in -try-completion? > > A completion style could make use of (and list) things after the > boundary. E.g. completing a file name like > > foo/ba<|>ar/baz > > could decide to list all the files that match > > *f*o*o*/*b*a*a*r*/*b*a*z* > > in which case the "end boundary" of the `completion-all-completions` > output should not be the `cdr` of the `completion-boundaries`. > > I wouldn't worry about it, tho. All right, please see the new function completion-base-suffix added in 0288bc6c949. Any docstring improvements (and others) are welcome. I guess we should also mention it in NEWS... ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 0 siblings, 2 replies; 24+ messages in thread From: Eli Zaretskii @ 2024-04-11 6:55 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 48356, juri, monnier, jdtsmith, mail > Cc: 48356@debbugs.gnu.org, Daniel Mendler <mail@daniel-mendler.de>, > JD Smith <jdtsmith@gmail.com>, Juri Linkov <juri@linkov.net> > Date: Thu, 11 Apr 2024 04:00:35 +0300 > From: Dmitry Gutov <dmitry@gutov.dev> > > All right, please see the new function completion-base-suffix added in > 0288bc6c949. Any docstring improvements (and others) are welcome. I tried to do that. Is there any reason why this function shouldn't be called completion-boundary-suffix instead? > I guess we should also mention it in NEWS... Yes, please. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2024-04-11 6:55 ` Eli Zaretskii @ 2024-04-11 10:36 ` Dmitry Gutov 2024-04-11 21:59 ` Dmitry Gutov 1 sibling, 0 replies; 24+ messages in thread From: Dmitry Gutov @ 2024-04-11 10:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 48356, juri, monnier, jdtsmith, mail On 11/04/2024 09:55, Eli Zaretskii wrote: >> Cc: 48356@debbugs.gnu.org, Daniel Mendler <mail@daniel-mendler.de>, >> JD Smith <jdtsmith@gmail.com>, Juri Linkov <juri@linkov.net> >> Date: Thu, 11 Apr 2024 04:00:35 +0300 >> From: Dmitry Gutov <dmitry@gutov.dev> >> >> All right, please see the new function completion-base-suffix added in >> 0288bc6c949. Any docstring improvements (and others) are welcome. > > I tried to do that. > > Is there any reason why this function shouldn't be called > completion-boundary-suffix instead? For such a name, I would probably expect the return value to be (buffer-substring (point) (cdr (completion-boundaries ...))) Opinions welcome. >> I guess we should also mention it in NEWS... > > Yes, please. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 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 1 sibling, 1 reply; 24+ messages in thread From: Dmitry Gutov @ 2024-04-11 21:59 UTC (permalink / raw) To: Eli Zaretskii; +Cc: mail, juri, 48356, monnier, jdtsmith, Visuwesh [-- Attachment #1: Type: text/plain, Size: 1539 bytes --] On 11/04/2024 09:55, Eli Zaretskii wrote: >> Cc: 48356@debbugs.gnu.org, Daniel Mendler <mail@daniel-mendler.de>, >> JD Smith <jdtsmith@gmail.com>, Juri Linkov <juri@linkov.net> >> Date: Thu, 11 Apr 2024 04:00:35 +0300 >> From: Dmitry Gutov <dmitry@gutov.dev> >> >> All right, please see the new function completion-base-suffix added in >> 0288bc6c949. Any docstring improvements (and others) are welcome. > > I tried to do that. > > Is there any reason why this function shouldn't be called > completion-boundary-suffix instead? > >> I guess we should also mention it in NEWS... > > Yes, please. Sorry about the trouble, here is the next patch on top which essentially had to change the function's semantics to match the name above, except it needed just the length. Since that made it a very thin wrapper, I'm inlining the code back, no docstring or announcement needed. What else this patch does: * Removes the variables completion-use-base-affixes and completion-base-affixes, just using the completion-base-position variable, although with a marker for the field end. * Changes 'completion--replace' to preserve the said marker. The result is that the text outside of the current field boundaries is maintained for both minibuffer and in-buffer completion (in particular, the suffix). As one downside, it brings back behavior described in https://debbugs.gnu.org/34517#14. That doesn't seem too critical to me, but opinions might vary. So more feedback welcome. Also Cc'ing Visuwesh who filed bug#49931 (related). [-- Attachment #2: base-suffix-v3.diff --] [-- Type: text/x-patch, Size: 10246 bytes --] diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index ad6a0928cda..56827f509aa 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,12 +2584,15 @@ 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)))) + (field-end + (save-excursion + (forward-char + (cdr (completion-boundaries (buffer-substring start (point)) + minibuffer-completion-table + minibuffer-completion-predicate + (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 @@ -2687,38 +2676,25 @@ 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 (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) @@ -4877,8 +4853,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 +4891,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)) ^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2024-04-11 21:59 ` Dmitry Gutov @ 2024-04-14 16:44 ` Juri Linkov 2024-04-14 23:55 ` Dmitry Gutov 0 siblings, 1 reply; 24+ messages in thread From: Juri Linkov @ 2024-04-14 16:44 UTC (permalink / raw) To: Dmitry Gutov Cc: Spencer Baugh, mail, 48356, monnier, jdtsmith, Visuwesh, Eli Zaretskii > As one downside, it brings back behavior described in > https://debbugs.gnu.org/34517#14. That doesn't seem too critical to me, but > opinions might vary. Sadly, this is quite an important case. Recently Spencer implemented a way to deselect a candidate in the visible completions list (minibuffer-visible-completions=t) when the user starts typing in the minibuffer. But then the user could change the mind and still select a candidate. This would interfere with the contents of the minibuffer. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2024-04-14 16:44 ` Juri Linkov @ 2024-04-14 23:55 ` Dmitry Gutov 2024-04-18 14:25 ` Spencer Baugh 0 siblings, 1 reply; 24+ messages in thread From: Dmitry Gutov @ 2024-04-14 23:55 UTC (permalink / raw) To: Juri Linkov Cc: Spencer Baugh, mail, 48356, monnier, jdtsmith, Visuwesh, Eli Zaretskii Hi Juri, On 14/04/2024 19:44, Juri Linkov wrote: >> As one downside, it brings back behavior described in >> https://debbugs.gnu.org/34517#14. That doesn't seem too critical to me, but >> opinions might vary. > Sadly, this is quite an important case. Recently Spencer implemented > a way to deselect a candidate in the visible completions list > (minibuffer-visible-completions=t) when the user starts typing > in the minibuffer. I think the (admittedly pretty cool) minibuffer-visible-completions feature is orthogonal: the scenarios I'm considering and trying to fix here also involve selecting a completion from *Completions* in some way (e.g. using M-up or M-down followed by M-RET, in default configuration). And this is currently working worse for in-buffer completion than for minibuffer completion WRT keeping the suffix around. > 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? The problem here, from my POV, is that we currently have a solution which matches the above goal, but which only makes sense for minibuffer (I think you wouldn't store the before/after buffer contents in separate string variables the same way). Whereas the API used is the same, so it would really make sense to minimize the differences in behavior between minibuffer and in-buffer completion. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2024-04-14 23:55 ` Dmitry Gutov @ 2024-04-18 14:25 ` Spencer Baugh 2024-04-20 0:12 ` Dmitry Gutov 0 siblings, 1 reply; 24+ messages in thread From: Spencer Baugh @ 2024-04-18 14:25 UTC (permalink / raw) To: Dmitry Gutov Cc: mail, Juri Linkov, 48356, monnier, jdtsmith, Visuwesh, Eli Zaretskii Dmitry Gutov <dmitry@gutov.dev> writes: > Hi Juri, > > On 14/04/2024 19:44, Juri Linkov wrote: >>> As one downside, it brings back behavior described in >>> https://debbugs.gnu.org/34517#14. That doesn't seem too critical to me, but >>> opinions might vary. >> Sadly, this is quite an important case. Recently Spencer implemented >> a way to deselect a candidate in the visible completions list >> (minibuffer-visible-completions=t) when the user starts typing >> in the minibuffer. > > I think the (admittedly pretty cool) minibuffer-visible-completions > feature is orthogonal: the scenarios I'm considering and trying to fix > here also involve selecting a completion from *Completions* in some > way (e.g. using M-up or M-down followed by M-RET, in default > configuration). And this is currently working worse for in-buffer > completion than for minibuffer completion WRT keeping the suffix > around. Yes, e.g.: 1. emacs -Q 2. M-x shell 3. mkdir -p dir1 dir2 && touch dir1/foo dir2/foo 4. cat dir/foo TAB 5. *Completions* contains: 2 possible completions: dir1/ dir2/ 6. Click "dir1/" 7. The buffer now contains "cat dir1/", the "foo" has been deleted This is a bug, but moreover it's also inconsistent with minibuffer completion, e.g.: 8. C-x C-f dir/foo TAB 9. *Completions* contains: 2 possible completions: dir1/ dir2/ 10. Click "dir1/" 11. The minibuffer now contains "dir1/foo" We should make them both behave the same way. >> 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2024-04-18 14:25 ` Spencer Baugh @ 2024-04-20 0:12 ` Dmitry Gutov 2024-05-04 2:23 ` Dmitry Gutov 0 siblings, 1 reply; 24+ messages in thread From: Dmitry Gutov @ 2024-04-20 0:12 UTC (permalink / raw) To: Spencer Baugh Cc: mail, Juri Linkov, 48356, monnier, jdtsmith, Visuwesh, Eli Zaretskii [-- 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)) ^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2024-04-20 0:12 ` Dmitry Gutov @ 2024-05-04 2:23 ` Dmitry Gutov 2024-05-09 2:33 ` Dmitry Gutov 0 siblings, 1 reply; 24+ messages in thread From: Dmitry Gutov @ 2024-05-04 2:23 UTC (permalink / raw) To: Spencer Baugh Cc: mail, Juri Linkov, 48356, monnier, jdtsmith, Visuwesh, Eli Zaretskii On 20/04/2024 03:12, Dmitry Gutov wrote: > 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. Unless anybody has objections, I'd like to install this patch soon-ish. I think it addresses all the problem scenarios that had been brought up so far. ^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary 2024-05-04 2:23 ` Dmitry Gutov @ 2024-05-09 2:33 ` Dmitry Gutov 0 siblings, 0 replies; 24+ messages in thread From: Dmitry Gutov @ 2024-05-09 2:33 UTC (permalink / raw) To: Spencer Baugh Cc: mail, Juri Linkov, 48356, monnier, jdtsmith, Visuwesh, Eli Zaretskii On 04/05/2024 05:23, Dmitry Gutov wrote: > On 20/04/2024 03:12, Dmitry Gutov wrote: >> 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. > > Unless anybody has objections, I'd like to install this patch soon-ish. > > I think it addresses all the problem scenarios that had been brought up > so far. Now installed. Please report any new problems. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2024-05-09 2:33 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2024-05-04 2:23 ` Dmitry Gutov 2024-05-09 2:33 ` Dmitry Gutov
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).