GNU bug report logs - #74019
[PATCH] Optionally preserve selected candidate across *Completions* update

Previous Next

Full log

Message #23 received at 74019 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> linkov.net, Andrea Corallo <acorallo <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 74019 <at> debbugs.gnu.org,
 Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#74019: [PATCH] Optionally preserve selected candidate
 across *Completions* update
Date: Mon, 28 Oct 2024 09:57:38 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: Juri Linkov <juri <at> linkov.net>
>> Date: Fri, 25 Oct 2024 17:32:38 -0400
>> From:  Spencer Baugh via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>> 
>> Add completion-preserve-selection, a defcustom which allows keeping the
>> same selected candidate after *Completions* is updated by
>> minibuffer-completion-help.
>
> Shouldn't we stop complicating the completion machinery at some point?
> It is already basically impenetrable, and if someone still can reason
> about what it does in any specific case, my hat's off to them; I have
> long ago reached the point where the _only_ way of understanding
> what's going on is to step with a debugger through the code -- and it
> doesn't help that some of the code is in Lisp and some in C, so Lisp
> calls into C, which calls back into Lisp, etc., thus one needs to
> interrupt the stepping, fire up a different debugger, then go back.
>
> Stefan, WDYT?  Should we close completion to further development and
> accept only bugfixes?
>
> What do Stefan Kangas and Andrea think?

FWIW, this patch is a very surface-level improvement to the default
completion UI, it doesn't deal with the deeper complicated parts of the
completion machinery.

>> +(defcustom completion-preserve-selection nil
>> +  "If non-nil, `minibuffer-completion-help' preserves the selected completion candidate.
>
> This doc-string line is too long.
>
> I also don't like the name: "selection" could be confused to mean the
> X selection.  I'd use "selected-candidates" instead.

Will change.

>> +If non-nil, and point is on a completion candidate in the displayed
>> +*Completions* window, `minibuffer-completion-help' will put point on the
>> +same candidate after updating *Completions*."
>
> This is completely unclear: what does minibuffer-completion-help have
> to do with updating *Completions*?

That's fair.  minibuffer-completion-help is the function which updates
*Completions*, but a user doesn't necessarily know that.  Will clarify
in next version.

>> +             (current-candidate-and-offset
>> +              (when-let* ((window (get-buffer-window "*Completions*" 0)))
>> +                (with-selected-window window
>> +                  (when-let* ((beg (completions--start-of-candidate-at (point))))
>> +
>> +                    (cons (get-text-property beg 'completion--string) (- (point) beg))))))
>
> Why is this done unconditionally?  with-selected-window is not a cheap
> function.

Will avoid the with-selected-window in the next version - a
with-current-buffer is sufficient.

>> @@ -4905,8 +4928,6 @@ minibuffer-next-completion
>>    (interactive "p")
>>    (let ((auto-choose minibuffer-completion-auto-choose))
>>      (with-minibuffer-completions-window
>> -      (when completions-highlight-face
>> -        (setq-local cursor-face-highlight-nonselected-window t))
>
> Why is this being moved to a different place?

See my reply to Stefan.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.