On Tue, 24 Jun 2025 16:00:51 -0400 Stefan Monnier <monnier@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@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