Subject: Re: bug#34641: rx: (or ...) order unpredictable

From: Eli Zaretskii <[email-masked]>
Date: Sat, 02 Mar 2019 14:33:05 +0200

> From: Mattias Engdegård <[email-masked]>
> Date: Sun, 24 Feb 2019 22:18:28 +0100
> Cc: [email-masked]
> 
> > Your preferred solution is fine with me, FWIW.
> 
> Thank you; patch attached.

Thanks, LGTM with minor comments below.

> +The optional argument @var{noreorder}, if @code{nil}, allows the
> +returned regexp to match the strings in any order.  If non-@code{nil},
> +the regexp is equivalent to a chain of alternatives (by the @samp{\|}
> +operator) of the strings in the order given.

I find the text in NEWS much more clear regarding what happens when
the new arg is non-nil.  I think what is missing is a more explicit
description you have in NEWS:

  If the new third argument is non-nil, the match is
  guaranteed to be performed in the order given, as if the strings were
  made into a regexp by joining them with '\|'.

So I suggest to mention explicitly the "match is guaranteed to be
performed in the order given" part.

> +The optional argument NOREORDER, if nil, allows the returned

We usually say "if nil or omitted" for optional arguments.

> +(defun regexp-opt--contains-prefix (strings)
> +  "Whether a list of strings contains a proper prefix of one of its elements.
> +STRINGS must be sorted and free from duplicates."

It is usually a good idea to refer to arguments explicitly in the
first sentence of a doc string.  In this case, I think just up-casing
STRINGS there should be enough.