Subject: Re: bug#78719: 30.1; [PATCH] Add functions `string-common-prefix' and `string-try-completion'

From: Eli Zaretskii <[email-masked]>
Date: Sat, 05 Jul 2025 16:36:50 +0300

> Date: Sun, 06 Jul 2025 00:05:01 +1200
> From: Phil Sainty <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked],
>  [email-masked], [email-masked]
> 
> > My single high-level gripe about this changeset is that it ended up
> > to be too tied up with completion.  AFAIU, the original intent was
> > to provide an API for finding the longest common prefix of a list of
> > strings.  With that concept in sight, STRING should have been a
> > mandatory argument (perhaps an empty string), and the fact that this
> > is based on the completion machinery should have been an
> > implementation detail.
> 
> I don't think STRING should be mandatory.  If you're wanting the
> longest prefix from an unfiltered collection (which I think will be a
> very common use-case for the function), it's unnecessarily awkward to
> need to also pass an explicit empty string argument when it could be
> left as an optional one.  Indeed, I'd say that not *needing* that
> argument is one of the benefits of the new function, vs using
> `try-completion' (which does require it).
> 
> 
> > The REGEXP-LIST and PREDICATE would not be needed, either.  By
> > contrast, what we have instead is a variant of try-completion, and
> > both the documentation and the API itself are advertising the
> > completion-based nature of the function too loudly to my palate,
> > starting from the argument named COLLECTION.  Why would we need yet
> > another variant of try-completion?
> 
> If `string-common-prefix' was not based on `try-completion' and we
> needed to write all of the functionality from scratch, then I would
> not be pushing to include and document the additional features.
> 
> It is based on `try-completion' though (for the good reason that
> `try-completion' is already extremely efficient at performing this
> task), and that has some ramifications for the new function:
> 
> 1. It will support the same COLLECTION data types as `try-collection'.
> 
> 2. All of the filtering features of `try-collection' are available.
> 
> I just see no reason to hide or suppress any of these things.

The reason is simplicity.  Right now, the API is intimidating, IMO.

> I feel the documentation should reflect the functionality, and the
> functionality in this case is dictated by `try-completion', so I don't
> see a way to avoid mentioning it.

Indeed, my main argument is about the API itself, not about the
documentation.  The documentation is complex because the API is
complex.

> > So maybe we should make this API simpler, so it could do its main
> > job without requiring such a large and complicated doc string, and
> > leave the rest to try-completion?
> 
> Again, my personal feeling is that the filtering is reasonable, so
> I'm in favour of keeping it.
> 
> I don't like "leave the rest to try-completion" because try-completion
> is really awkward to use outside of a completion context.

How can you say that, when the body of the new function is just this:

  (let* ((completion-ignore-case ignore-case)
         (completion-regexp-list regexp-list)
         (prefix (try-completion (or string "") collection predicate)))
    (if (stringp prefix)
        prefix
      (if (eq t prefix)
          string
        "")))

AFAIU, the single most important argument for having a new function
(instead of telling people to do the above inline) is to simplify the
calling sequence when all a program wants is a common prefix of a set
of strings.  If all the facilities provided by try-completion need to
be exposed by this new function, doesn't that make our rationale
pretty much void?  Because the above "complexity" pales near the need
to understand the role and effects of each of the arguments.

> The reasons for adding the new function were:
> 
> 1. Discoverability; `try-completion' is a very non-obvious name for
> anyone working with strings rather than completion, whereas the
> `string-common-prefix' name will be very easily found.

This could be solved by a defalias.

> 2. Ease of use: `try-completion' requires you to jump through hoops
> in order to deal with its non-string return values, whereas the new
> function deals with those edge cases to return what's useful for
> working with strings.

I disagree that the above code is anywhere near "jumping through
hoops".  We could even show it in the manual if that is a serious
problem, and have this issue solved.