GNU bug report logs - #78719
30.1; [PATCH] Add functions `string-common-prefix' and `string-try-completion'

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Phil Sainty <psainty <at> orcon.net.nz>
Cc: mail <at> daniel-mendler.de, juri <at> linkov.net, monnier <at> iro.umontreal.ca,
 drew.adams <at> oracle.com, 78719 <at> debbugs.gnu.org
Subject: Re: bug#78719: 30.1; [PATCH] Add functions `string-common-prefix'
 and `string-try-completion'
Date: Sun, 29 Jun 2025 09:05:55 +0300

> Date: Sun, 29 Jun 2025 15:45:14 +1200
> From: Phil Sainty <psainty <at> orcon.net.nz>
> Cc: monnier <at> iro.umontreal.ca, mail <at> daniel-mendler.de, 78719 <at> debbugs.gnu.org,
>  drew.adams <at> oracle.com, juri <at> linkov.net
> 
> On 2025-06-11 03:32, Eli Zaretskii wrote:
> >> (string-common-prefix COLLECTION &optional IGNORE-CASE STRING
> >> REGEXP-LIST PREDICATE)
> > 
> > I think STRING should come before IGNORE-CASE.  Otherwise, LGTM,
> > thanks.
> 
> I've pushed the revisions to scratch/string-common-prefix
> 
> After fetching, the revised changes will be:
> 
> git diff master...origin/scratch/string-common-prefix
> 
> 
> My remaining niggle about having STRING before IGNORE-CASE is that
> STRING, REGEXP-LIST, and PREDICATE all fall under the same category
> of "filtering the COLLECTION prior to determining a common prefix",
> so this order interrupts that logical grouping.
> 
> I've described the three filtering arguments together as a group in
> the documentation, for better readability.  I don't mind switching
> the order in the code to match if you change your mind about that;
> otherwise I think this is ready to go, if you don't spot any issues.

If you want to move STRING after IGNORE-CASE, you should rename it to
something like MIN-PREFIX.

Btw, the changes for the manual have the argument list wrong.

Also, this part:

  If COLLECTION contains exactly one string, return that string.

left me wondering whether this will happen even if that single string
doesn't satisfy the filtering criteria.

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.  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?

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?

Thanks.

Previous Next

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