GNU bug report logs -
#77718
31.0.50; completion styles substring and flex are broken
Previous Next
Reported by: Stephen Berman <stephen.berman <at> gmx.net>
Date: Thu, 10 Apr 2025 22:23:02 UTC
Severity: normal
Found in version 31.0.50
Done: Stefan Monnier <monnier <at> iro.umontreal.ca>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
On Tue, 24 Jun 2025 16:00:51 -0400 Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>> Does the attached patch go in the direction you intended with your
>> comments?
>
> Yes, tho I think it's not quite right yet.
>
>> From my initial testing it gives the same results as my patch
>> with hardcoded "/", and I haven't seen bad results yet; however, I don't
>> have a good understanding of `completion-boundaries' and may well have
>> not used it as you meant. Also, even if it does do what you suggested,
>> since it in effect reverts the substring completion display and behavior
>> involving directories to what it was before Spencer's changes, I guess
>> he wouldn't accept it, just as he rejected my first patch.
>
> I can't remember what his change was trying to do, so I won't try to
> guess what would be his opinion.
>
>> That was why I proposed to make the behavior configurable, but you
>> don't like that; why not?
>
> I don't think Spencer specifically wants `/usr/` to complete to
> `/usr//`.
I understood him to be saying "/usr//" is the correct substring
completion (when all completions are directories):
>>> On Tue, 17 Jun 2025 10:59:56 -0400 Spencer Baugh <sbaugh <at> janestreet.com> wrote:
> [...]
>> This is the behavior of the substring completion style. When there's a
>> common substring between all the completions,
>> completion-substring-try-completion inserts that substring. Since the
>> substring is a common suffix, it positions point before that common
>> suffix.
> [...]
>> To have only "substring" configured as a completion style implies you
>> want the behavior of the "substring" completion style, and only that
>> behavior. You are indeed getting that behavior. What's the problem?
>> substring has always behaved like this, so improved consistency in edge
>> cases like this one should not be a surprise.
> He's probably interested in a more general case. So I don't
> think the two desires are fundamentally incompatible.
If they're compatible, then of course I have no objection.
>> @@ -4613,16 +4613,22 @@ completion-pcm--merge-completions
>> ;; If prefix is one of the completions, there's no
>> ;; suffix left to find.
>> (not (assoc-string prefix comps t)))
>> - (let ((suffix
>> - (completion--common-suffix
>> - (if (zerop (length prefix)) comps
>> - ;; Ignore the chars in the common prefix, so we
>> - ;; don't merge '("abc" "abbc") as "ab*bc".
>> - (let ((skip (length prefix)))
>> - (mapcar (lambda (str) (substring str skip))
>> - comps))))))
>> + (let* ((suffix
>> + (completion--common-suffix
>> + (if (zerop (length prefix)) comps
>> + ;; Ignore the chars in the common prefix, so we
>> + ;; don't merge '("abc" "abbc") as "ab*bc".
>> + (let ((skip (length prefix)))
>> + (mapcar (lambda (str) (substring str skip))
>> + comps)))))
>> + (bounds (completion-boundaries
>> + prefix comps minibuffer-completion-predicate
>> + suffix))
>> + (sep (substring suffix (max 0 (1- (cdr bounds)))
>> + (cdr bounds))))
>> (cl-assert (stringp suffix))
>> - (unless (equal suffix "")
>> + (unless (or (equal suffix "")
>> + (and (equal prefix "") (equal suffix sep)))
>> (push suffix res))))
>
> IIRC `prefix` here is just the longest common prefixes among the found
> candidates for this specific part of the input pattern. E.g. for `/u
> TAB`, `prefix` could be just "s" if there's `/usr/` and `/users/` among
> the candidates. So testing if `prefix` is "" doesn't actually test
> whether the text before `suffix` is another boundary separator
> (e.g. "/"). For that you'd need to look also at `res` (which is where
> the "/u" part would be stashed, tho possibly in pieces, with other
> (non-string) elements interspersed).
>
> Similarly, calling `completion-boundaries` on just `prefix` doesn't make
> much sense. You'd want to call it on the combination of `prefix` and
> `res` (i.e. on something like `(completion-pcm--pattern->string (reverse
> (cons prefix res)))`).
Thanks again for the guidance, which I again attempted to follow in the
attached patch, which from my initial testing appears to give the
results I want.
> BTW, if we want to fix the problem with a user-config, there is one part
> that would make sense to be user-configurable, which is whether
> `substring` completion should try and complete suffixes at all:
> Depending on your habits and typing style, having a suffix inserted
> after point can be a hindrance rather than a help because it can be
> easier for you to type that suffix than to skip over it.
> The "right" way to make this configurable (IMO) would be to have a new
> completion style that's like `substring` but without the insertion of
> common suffixes.
> [ In my ideal UI, completion styles would not be limited to symbols, so
> you could have `(substring nil)` and `(substring t)` be the two
> alternatives, where `substring` would be a style that takes an
> (optional) `inhibit-common-suffixes` arg. ]
I would be happy with that.
Steve Berman
[Message part 2 (text/x-patch, attachment)]
This bug report was last modified 11 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.