Clarify `pcase' `rx' pattern doc

Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

Hello,

I had problems to understand the documentation of the `pcase' `rx'
pattern defined in "rx.el", though I'm familiar with (and user of) both
pcase and rx.  So I looked at the code and tried to tweak the docstring.
(I also removed the EXPVAL reference.)

Opinions?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clarify-pcase-rx-pattern-documentation.patch --]
[-- Type: text/x-diff, Size: 2158 bytes --]

From 0d56315ad2470e127f5f0b464872216e7ceb6caa Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Wed, 13 Jun 2018 01:42:34 +0200
Subject: [PATCH] Clarify `pcase' `rx' pattern documentation

---
 lisp/emacs-lisp/rx.el | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index 85e74f28ef..643ee6949d 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -1183,24 +1183,25 @@ rx
 
 
 (pcase-defmacro rx (&rest regexps)
-  "Build a `pcase' pattern matching `rx' regexps.
-The REGEXPS are interpreted as by `rx'.  The pattern matches if
-the regular expression so constructed matches EXPVAL, as if
-by `string-match'.
+  "`pcase' pattern matching strings using `rx' REGEXPS.
+The REGEXPS are interpreted as by `rx'.  The pattern matches any
+string matched by the regular expression so constructed, as if by
+`string-match'.
 
 In addition to the usual `rx' constructs, REGEXPS can contain the
 following constructs:
 
-  (let VAR FORM...)  creates a new explicitly numbered submatch
-                     that matches FORM and binds the match to
-                     VAR.
-  (backref VAR)      creates a backreference to the submatch
-                     introduced by a previous (let VAR ...)
-                     construct.
+  (let VAR REGEXPS...)  creates a new explicitly numbered
+                        submatch that matches the `rx' REGEXPS
+                        and binds the match to VAR.
+  (backref VAR-OR-NBR)  creates a backreference to the submatch
+                        introduced by a previous (let VAR ...)
+                        construct; VAR-OR-NBR is either a symbol
+                        VAR or a submatch number.  It matches the
+                        exact previous submatch.
 
 The VARs are associated with explicitly numbered submatches
-starting from 1.  Multiple occurrences of the same VAR refer to
-the same submatch.
+starting from 1.
 
 If a case matches, the match data is modified as usual so you can
 use it in the case body, but you still have to pass the correct
-- 
2.17.1


[-- Attachment #3: Type: text/plain, Size: 21 bytes --]



Thanks,

Michael.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hello,

I added another sentence saying that it is an error if the target is not
a string.

Should we also add an example?


diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index 85e74f28ef..fa2a0898d6 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -1183,24 +1183,25 @@ rx
 
 
 (pcase-defmacro rx (&rest regexps)
-  "Build a `pcase' pattern matching `rx' regexps.
-The REGEXPS are interpreted as by `rx'.  The pattern matches if
-the regular expression so constructed matches EXPVAL, as if
-by `string-match'.
+  "`pcase' pattern matching strings using `rx' REGEXPS.
+The REGEXPS are interpreted as by `rx'.  The pattern matches any
+string matched by the regular expression so constructed, as if by
+`string-match'.  Raises an error if the target is not a string.
 
 In addition to the usual `rx' constructs, REGEXPS can contain the
 following constructs:
 
-  (let VAR FORM...)  creates a new explicitly numbered submatch
-                     that matches FORM and binds the match to
-                     VAR.
-  (backref VAR)      creates a backreference to the submatch
-                     introduced by a previous (let VAR ...)
-                     construct.
+  (let VAR REGEXPS...)  creates a new explicitly numbered
+                        submatch that matches the `rx' REGEXPS
+                        and binds the match to VAR.
+  (backref VAR-OR-NBR)  creates a backreference to the submatch
+                        introduced by a previous (let VAR ...)
+                        construct; VAR-OR-NBR is either a symbol
+                        VAR or a submatch number.  It matches the
+                        exact previous submatch.
 
 The VARs are associated with explicitly numbered submatches
-starting from 1.  Multiple occurrences of the same VAR refer to
-the same submatch.
+starting from 1.
 
 If a case matches, the match data is modified as usual so you can
 use it in the case body, but you still have to pass the correct
-- 
2.17.1



Michael.

Re: Clarify `pcase' `rx' pattern doc

[Andreas Schwab]

On Jun 13 2018, Michael Heerdegen <michael_heerdegen@web.de> wrote:

> diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
> index 85e74f28ef..fa2a0898d6 100644
> --- a/lisp/emacs-lisp/rx.el
> +++ b/lisp/emacs-lisp/rx.el
> @@ -1183,24 +1183,25 @@ rx
>  
>  
>  (pcase-defmacro rx (&rest regexps)
> -  "Build a `pcase' pattern matching `rx' regexps.
> -The REGEXPS are interpreted as by `rx'.  The pattern matches if
> -the regular expression so constructed matches EXPVAL, as if
> -by `string-match'.
> +  "`pcase' pattern matching strings using `rx' REGEXPS.

This sentence no verb.  What's wrong with the original version?  At
least it should start with a capitalized word.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Andreas Schwab <schwab@suse.de> writes:

> On Jun 13 2018, Michael Heerdegen <michael_heerdegen@web.de> wrote:
>
> > diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
> > index 85e74f28ef..fa2a0898d6 100644
> > --- a/lisp/emacs-lisp/rx.el
> > +++ b/lisp/emacs-lisp/rx.el
> > @@ -1183,24 +1183,25 @@ rx
> >  
> >  
> >  (pcase-defmacro rx (&rest regexps)
> > -  "Build a `pcase' pattern matching `rx' regexps.
> > -The REGEXPS are interpreted as by `rx'.  The pattern matches if
> > -the regular expression so constructed matches EXPVAL, as if
> > -by `string-match'.
> > +  "`pcase' pattern matching strings using `rx' REGEXPS.
>
> This sentence no verb.  At least it should start with a capitalized
> word.

I can arrange that.  There is a verb, but it is elided ;-)  It is
arguable if it is not a sentence, I guess.


> What's wrong with the original version?

I found some things confusing; and some information is also missing:

The original version says "build a pcase pattern".  This text appears in
C-h f pcase, where it sounds strange.

It says it matches rx regexps, I find it more concrete to say it matches
strings using rx regexps.  It doesn't say that it raises an error if
matched against something that is not a string.

The `let' part doc speaks of a FORM where it means an rx regexp.

The `backref' part fails to say that it also accepts a positive integer
as argument, and what "backreference" actually means (is it a reference
to a pattern or to a match?).


Michael.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> > > +  "`pcase' pattern matching strings using `rx' REGEXPS.
> >
> > This sentence no verb.  At least it should start with a capitalized
> > word.
>
> I can arrange that.  There is a verb, but it is elided ;-)  It is
> arguable if it is not a sentence, I guess.

BTW this aspect concerns more or less all pcase pattern definitions: The
first sentence either is not a sentence in the strict sense, or says
"Build a pcase pattern matching..."  We should probably unify the
language used.


Michael.

Re: Clarify `pcase' `rx' pattern doc

[Yuri Khan]

On Wed, Jun 13, 2018 at 3:22 PM Michael Heerdegen
<michael_heerdegen@web.de> wrote:

> BTW this aspect concerns more or less all pcase pattern definitions: The
> first sentence either is not a sentence in the strict sense, or says
> "Build a pcase pattern matching..."  We should probably unify the
> language used.

The standard patterns are documented in this form:

    <pattern> matches <something in some conditions>.

It would be nice if extended patterns defined using pcase-defmacro
also followed this syntax:

    -- (seq &rest PATTERNS)

    Matches a SEQUENCE such that each element of PATTERNS matches a
corresponding element of SEQUENCE.

    Extra elements of the sequence are ignored if fewer PATTERNS are
given, and the match does not fail.

In other words, for the user, it is not important that underlying the
‘(seq …)’ is a ‘seq--pcase-macroexpander’ function that builds a pcase
pattern out of simpler constructs. From the user’s viewpoint, ‘(seq
…)’ *is* the pattern.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Opinions?

For now, I would like to install the following, slightly modified
version (I left the first sentence unchanged so that we can care about
that systematically in a later commit) into emacs-26.  Is that ok?


Thanks,

Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Tweak-pcase-rx-pattern-documentation.patch --]
[-- Type: text/x-diff, Size: 2263 bytes --]

From c03375d0e086c3bc729f9a8b22eccd88804b6548 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Wed, 13 Jun 2018 01:42:34 +0200
Subject: [PATCH] Tweak 'pcase' 'rx' pattern documentation

* lisp/emacs-lisp/rx.el (rx): Improve pattern docstring.
---
 lisp/emacs-lisp/rx.el | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index 85e74f28ef..6c01536380 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -1183,24 +1183,25 @@ rx
 
 
 (pcase-defmacro rx (&rest regexps)
-  "Build a `pcase' pattern matching `rx' regexps.
-The REGEXPS are interpreted as by `rx'.  The pattern matches if
-the regular expression so constructed matches EXPVAL, as if
-by `string-match'.
+  "Build a `pcase' pattern matching with `rx' REGEXPS.
+The REGEXPS are interpreted as by `rx'.  The pattern matches any
+string matched by the regular expression so constructed, as if by
+`string-match'.  An error is raised if the target is not a string.
 
 In addition to the usual `rx' constructs, REGEXPS can contain the
 following constructs:
 
-  (let VAR FORM...)  creates a new explicitly numbered submatch
-                     that matches FORM and binds the match to
-                     VAR.
-  (backref VAR)      creates a backreference to the submatch
-                     introduced by a previous (let VAR ...)
-                     construct.
+  (let VAR REGEXPS...)  creates a new explicitly numbered
+                        submatch that matches the `rx' REGEXPS
+                        and binds the match to VAR.
+  (backref VAR-OR-NBR)  creates a backreference to the submatch
+                        introduced by a previous (let VAR ...)
+                        construct; VAR-OR-NBR is either a symbol
+                        VAR or a submatch number.  It matches the
+                        exact previous submatch.
 
 The VARs are associated with explicitly numbered submatches
-starting from 1.  Multiple occurrences of the same VAR refer to
-the same submatch.
+starting from 1.
 
 If a case matches, the match data is modified as usual so you can
 use it in the case body, but you still have to pass the correct
-- 
2.17.1

Re: Clarify `pcase' `rx' pattern doc

[Stefan Monnier]

> +`string-match'.  An error is raised if the target is not a string.

BTW, usually, pcase patterns simply don't match (rather than signal an
error) if the type is not appropriate (and they may signal an error when
used in pcase-let since pcase-let will then eliminate the test under the
assumption that the pattern does match).


        Stefan

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > +`string-match'.  An error is raised if the target is not a string.
>
> BTW, usually, pcase patterns simply don't match (rather than signal an
> error) if the type is not appropriate [...]

Hmm, yes, `rx' seems to be the only exception.  I'll drop that sentence
for now, and we can change the definition of the `rx' pattern later in
master.


Thanks,

Michael.

Re: Clarify `pcase' `rx' pattern doc

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Mon, 18 Jun 2018 14:34:50 +0200
> 
> > Opinions?
> 
> For now, I would like to install the following, slightly modified
> version (I left the first sentence unchanged so that we can care about
> that systematically in a later commit) into emacs-26.  Is that ok?

Thanks, I have a few minor comments regarding the style:

>  (pcase-defmacro rx (&rest regexps)
> -  "Build a `pcase' pattern matching `rx' regexps.
> -The REGEXPS are interpreted as by `rx'.  The pattern matches if
> -the regular expression so constructed matches EXPVAL, as if
> -by `string-match'.
> +  "Build a `pcase' pattern matching with `rx' REGEXPS.

I don't like calling this "regexp".  Elsewhere in rx documentation we
say either "regexps in sexp form" or just "form".  Using "regexp"
might confuse the reader to think these are the "normal" regexp
strings.

> +`string-match'.  An error is raised if the target is not a string.
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Please avoid the passive tense.  In this case, I'd rephrase:

  If the target is not a string, this macro signals an error.

> -  (let VAR FORM...)  creates a new explicitly numbered submatch
> -                     that matches FORM and binds the match to
> -                     VAR.
> -  (backref VAR)      creates a backreference to the submatch
> -                     introduced by a previous (let VAR ...)
> -                     construct.
> +  (let VAR REGEXPS...)  creates a new explicitly numbered
> +                        submatch that matches the `rx' REGEXPS
> +                        and binds the match to VAR.

IMO, this change is for the worse: the original clearly indicated that
FORM is the rx-style regexp, whereas the new text blurs this
indication.

> +  (backref VAR-OR-NBR)  creates a backreference to the submatch
> +                        introduced by a previous (let VAR ...)
> +                        construct; VAR-OR-NBR is either a symbol
> +                        VAR or a submatch number.  It matches the
> +                        exact previous submatch.

I'd use just REF here instead of VAR-OR-NBR, which is quite a
mouthful, and doesn't help in understanding the semantics until one
reads the explanation.

This is OK for the emacs-26 branch, once we agree on the above nits.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:


> >  (pcase-defmacro rx (&rest regexps)
> > -  "Build a `pcase' pattern matching `rx' regexps.
> > -The REGEXPS are interpreted as by `rx'.  The pattern matches if
> > -the regular expression so constructed matches EXPVAL, as if
> > -by `string-match'.
> > +  "Build a `pcase' pattern matching with `rx' REGEXPS.
>
> I don't like calling this "regexp".  Elsewhere in rx documentation we
> say either "regexps in sexp form" or just "form".  Using "regexp"
> might confuse the reader to think these are the "normal" regexp
> strings.

Wound "rx-form" be acceptable?  Also see below.

> > +`string-match'.  An error is raised if the target is not a string.
>                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Please avoid the passive tense.

I've dropped that sentence after Stefan's comment.

> > -  (let VAR FORM...)  creates a new explicitly numbered submatch
> > -                     that matches FORM and binds the match to
> > -                     VAR.
> > -  (backref VAR)      creates a backreference to the submatch
> > -                     introduced by a previous (let VAR ...)
> > -                     construct.
> > +  (let VAR REGEXPS...)  creates a new explicitly numbered
> > +                        submatch that matches the `rx' REGEXPS
> > +                        and binds the match to VAR.
>
> IMO, this change is for the worse: the original clearly indicated that
> FORM is the rx-style regexp, whereas the new text blurs this
> indication.

The problem here: we usually call an expression (something to be
evaluated) a FORM.  And we have a `let' pattern in `pcase', and its
second argument is really a form.  This is super confusing and the main
reason why I didn't understand how `let' and `backref' work in this
pattern after reading the sources.  So again: is something like
"RX-FORM" acceptable, or something similar?


> > +  (backref VAR-OR-NBR)  creates a backreference to the submatch
> > +                        introduced by a previous (let VAR ...)
> > +                        construct; VAR-OR-NBR is either a symbol
> > +                        VAR or a submatch number.  It matches the
> > +                        exact previous submatch.
>
> I'd use just REF here instead of VAR-OR-NBR, which is quite a
> mouthful, and doesn't help in understanding the semantics until one
> reads the explanation.

Ok, I'll do this.


Thanks,

Michael.

Re: Clarify `pcase' `rx' pattern doc

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: emacs-devel@gnu.org
> Date: Mon, 18 Jun 2018 19:22:01 +0200
> 
> > I don't like calling this "regexp".  Elsewhere in rx documentation we
> > say either "regexps in sexp form" or just "form".  Using "regexp"
> > might confuse the reader to think these are the "normal" regexp
> > strings.
> 
> Wound "rx-form" be acceptable?

We never use rx-form in any other doc string in rx.el.  We use these:

  . regular expression FORM
  . regular expression in sexp form
  . sequence of (regular expression) forms

etc.  I think we should stick to the same nomenclature in this macro.

> > > -  (let VAR FORM...)  creates a new explicitly numbered submatch
> > > -                     that matches FORM and binds the match to
> > > -                     VAR.
> > > -  (backref VAR)      creates a backreference to the submatch
> > > -                     introduced by a previous (let VAR ...)
> > > -                     construct.
> > > +  (let VAR REGEXPS...)  creates a new explicitly numbered
> > > +                        submatch that matches the `rx' REGEXPS
> > > +                        and binds the match to VAR.
> >
> > IMO, this change is for the worse: the original clearly indicated that
> > FORM is the rx-style regexp, whereas the new text blurs this
> > indication.
> 
> The problem here: we usually call an expression (something to be
> evaluated) a FORM.  And we have a `let' pattern in `pcase', and its
> second argument is really a form.

As long as there's no other reference to "form" in this doc string,
the ambiguity does no harm, since you describe what FORM is right
away.  FORM and "form" are not the same thing, so the fact that 'let'
uses a "form" has no real bearing on this doc string.

> This is super confusing and the main reason why I didn't understand
> how `let' and `backref' work in this pattern after reading the
> sources.  So again: is something like "RX-FORM" acceptable, or
> something similar?

I admit really don't understand the source of your confusion.  I think
you are biased by uses of FORM in entirely unrelated contexts, which
is a mistake: the word "form" has many uses, and we shouldn't allow
the shadows of those uses to haunt us.

What other alternatives would be acceptable to you, if we exclude
RX-FORM?  Again, it's best to use something used elsewhere in the
rx.el doc strings: consistency in documentation of a feature is a Good
Thing.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> >  (pcase-defmacro rx (&rest regexps)
> > -  "Build a `pcase' pattern matching `rx' regexps.
> > -The REGEXPS are interpreted as by `rx'.  The pattern matches if
> > -the regular expression so constructed matches EXPVAL, as if
> > -by `string-match'.
> > +  "Build a `pcase' pattern matching with `rx' REGEXPS.
>
> I don't like calling this "regexp".  Elsewhere in rx documentation we
> say either "regexps in sexp form" or just "form".  Using "regexp"
> might confuse the reader to think these are the "normal" regexp
> strings.

But hey, ehm - I didn't change this, I just upcased the argument name.

> > -  (let VAR FORM...)  creates a new explicitly numbered submatch
> > -                     that matches FORM and binds the match to
> > -                     VAR.
> > -  (backref VAR)      creates a backreference to the submatch
> > -                     introduced by a previous (let VAR ...)
> > -                     construct.
> > +  (let VAR REGEXPS...)  creates a new explicitly numbered
> > +                        submatch that matches the `rx' REGEXPS
> > +                        and binds the match to VAR.
>
> IMO, this change is for the worse: the original clearly indicated that
> FORM is the rx-style regexp, whereas the new text blurs this
> indication.

And here I just used the same name for the `let' argument, since it's of
exactly the same type as the argument of the `rx' pattern.

Even the normal `rx' macro (not the pcase macro) names its &rest
argument "REGEXPS" - so I think now what I suggested was just
consistent.  Of cause could we change all occurrences of "REGEXPS" to
"FORMS" or something better, but I think this is not in the scope of my
suggested commit.


Michael.

Re: Clarify `pcase' `rx' pattern doc

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: emacs-devel@gnu.org
> Date: Thu, 21 Jun 2018 13:13:58 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > >  (pcase-defmacro rx (&rest regexps)
> > > -  "Build a `pcase' pattern matching `rx' regexps.
> > > -The REGEXPS are interpreted as by `rx'.  The pattern matches if
> > > -the regular expression so constructed matches EXPVAL, as if
> > > -by `string-match'.
> > > +  "Build a `pcase' pattern matching with `rx' REGEXPS.
> >
> > I don't like calling this "regexp".  Elsewhere in rx documentation we
> > say either "regexps in sexp form" or just "form".  Using "regexp"
> > might confuse the reader to think these are the "normal" regexp
> > strings.
> 
> But hey, ehm - I didn't change this, I just upcased the argument name.

That's true, but your goal was to improve the existing doc string,
right?  I'm saying that using "REGEXP" doesn't improve it.

> > > -  (let VAR FORM...)  creates a new explicitly numbered submatch
> > > -                     that matches FORM and binds the match to
> > > -                     VAR.
> > > -  (backref VAR)      creates a backreference to the submatch
> > > -                     introduced by a previous (let VAR ...)
> > > -                     construct.
> > > +  (let VAR REGEXPS...)  creates a new explicitly numbered
> > > +                        submatch that matches the `rx' REGEXPS
> > > +                        and binds the match to VAR.
> >
> > IMO, this change is for the worse: the original clearly indicated that
> > FORM is the rx-style regexp, whereas the new text blurs this
> > indication.
> 
> And here I just used the same name for the `let' argument, since it's of
> exactly the same type as the argument of the `rx' pattern.
> 
> Even the normal `rx' macro (not the pcase macro) names its &rest
> argument "REGEXPS" - so I think now what I suggested was just
> consistent.  Of cause could we change all occurrences of "REGEXPS" to
> "FORMS" or something better, but I think this is not in the scope of my
> suggested commit.

Could you please elaborate on what you didn't like or found confusing
in the original text, and why?  Then perhaps I could suggest how to
modify the text to satisfy us both.  OK?

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> Could you please elaborate on what you didn't like or found confusing
> in the original text, and why?  Then perhaps I could suggest how to
> modify the text to satisfy us both.  OK?

As I said - the second arg of `let' in the `rx' pattern is of the same
type as the argument of `rx' - a formal regexp.  But because it is named
differently ("FORM"), I assumed it would be something different - and
because it is named form, I thought, well, it should be a form.

So there are two things: (1) the arguments of the `rx' macro, the `rx'
pcase macro, and the first arg of the `let' subpattern in `rx' should
have the same name, and (2) we need to go away from the argument name
"REGEXP" since we agree that it's a confusing name, but it's used in
many places, the `rx' macro among them.


Michael.

Re: Clarify `pcase' `rx' pattern doc

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: emacs-devel@gnu.org
> Date: Thu, 21 Jun 2018 17:13:51 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Could you please elaborate on what you didn't like or found confusing
> > in the original text, and why?  Then perhaps I could suggest how to
> > modify the text to satisfy us both.  OK?
> 
> As I said - the second arg of `let' in the `rx' pattern is of the same
> type as the argument of `rx' - a formal regexp.  But because it is named
> differently ("FORM"), I assumed it would be something different - and
> because it is named form, I thought, well, it should be a form.
> 
> So there are two things: (1) the arguments of the `rx' macro, the `rx'
> pcase macro, and the first arg of the `let' subpattern in `rx' should
> have the same name, and (2) we need to go away from the argument name
> "REGEXP" since we agree that it's a confusing name, but it's used in
> many places, the `rx' macro among them.

How about the below?

diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index 30bb129..dee734c 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -1181,20 +1181,25 @@ rx
 
 
 (pcase-defmacro rx (&rest regexps)
-  "Build a `pcase' pattern matching `rx' regexps.
-The REGEXPS are interpreted as by `rx'.  The pattern matches if
-the regular expression so constructed matches EXPVAL, as if
-by `string-match'.
+  "Build a `pcase' pattern matching `rx' REGEXPS in sexp form.
+The REGEXPS are interpreted as in `rx'.  The pattern matches any
+string that is a match for the regular expression so constructed,
+as if by `string-match'.  If the target is not a string, signal
+an error.
 
 In addition to the usual `rx' constructs, REGEXPS can contain the
 following constructs:
 
-  (let VAR FORM...)  creates a new explicitly numbered submatch
-                     that matches FORM and binds the match to
-                     VAR.
+  (let VAR SEXP...)  creates a new explicitly numbered submatch
+                     that matches regular expressions SEXP, and
+                     binds the match to VAR.
   (backref VAR)      creates a backreference to the submatch
                      introduced by a previous (let VAR ...)
-                     construct.
+                     construct.  VAR can be the same symbol
+                     in the first argument of the corresponding
+                     (let VAR ...) construct, or it can be a
+                     submatch number.  It matches the referenced
+                     submatch.
 
 The VARs are associated with explicitly numbered submatches
 starting from 1.  Multiple occurrences of the same VAR refer to

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> How about the below?

> + [...]  If the target is not a string, signal an error.

We want to change that, so I think you can drop that sentence.

 
>  In addition to the usual `rx' constructs, REGEXPS can contain the
>  following constructs:
>  
> -  (let VAR FORM...)  creates a new explicitly numbered submatch
> -                     that matches FORM and binds the match to
> -                     VAR.
> +  (let VAR SEXP...)  creates a new explicitly numbered submatch
> +                     that matches regular expressions SEXP, and
> +                     binds the match to VAR.
>    (backref VAR)      creates a backreference to the submatch
>                       introduced by a previous (let VAR ...)
> -                     construct.
> +                     construct.  VAR can be the same symbol
> +                     in the first argument of the corresponding
> +                     (let VAR ...) construct, or it can be a
> +                     submatch number.  It matches the referenced
> +                     submatch.
>  

Looks ok to me.  But can we remove the sentence saying "Multiple
occurrences of the same VAR refer to the same submatch."?  It's
completely redundant IMHO.

Then, can you please install it (maybe with my issues fixed)?


Thanks,

Michael.

Re: Clarify `pcase' `rx' pattern doc

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: emacs-devel@gnu.org
> Date: Fri, 06 Jul 2018 19:57:29 +0200
> 
> > + [...]  If the target is not a string, signal an error.
> 
> We want to change that, so I think you can drop that sentence.

Shouldn't it be dropped when that change is committed?

> But can we remove the sentence saying "Multiple occurrences of the
> same VAR refer to the same submatch."?  It's completely redundant
> IMHO.

Is it redundant even when VAR is not a submatch number, but a symbol?

Btw, in this part:

> +  (let VAR SEXP...)  creates a new explicitly numbered submatch
> +                     that matches regular expressions SEXP, and
> +                     binds the match to VAR.

Does "explicitly numbered" mean that VAR must be a number?  If it can
be something else, perhaps "explicitly named" is better?

> Then, can you please install it (maybe with my issues fixed)?

Sure.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > > + [...]  If the target is not a string, signal an error.
> > 
> > We want to change that, so I think you can drop that sentence.
>
> Shouldn't it be dropped when that change is committed?

Then we would break documented behavior then.  What would be the gain?

> > But can we remove the sentence saying "Multiple occurrences of the
> > same VAR refer to the same submatch."?  It's completely redundant
> > IMHO.
>
> Is it redundant even when VAR is not a submatch number, but a symbol?

I think so.  A symbol VAR references the explicit binding created with
(let VAR ...), i.e. a submatch, just like in the number case.

> Btw, in this part:
>
> > +  (let VAR SEXP...)  creates a new explicitly numbered submatch
> > +                     that matches regular expressions SEXP, and
> > +                     binds the match to VAR.
>
> Does "explicitly numbered" mean that VAR must be a number?  If it can
> be something else, perhaps "explicitly named" is better?

AFAIK, in `let' VAR must be a symbol, but it seems the submatch is also
numbered as side effect, e.g.

(pcase "Hala"
  ((rx "H" (let x "a") (regex ".*") (backref 1)) x))
==> "a"

In `backref' the argument can be a number or a symbol VAR.  That's why I
would prefer a different argument name in the docstring, "REF" maybe.  I
had done that in my suggested patch.


Michael.

Re: Clarify `pcase' `rx' pattern doc

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: emacs-devel@gnu.org
> Date: Sat, 07 Jul 2018 15:36:04 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > > > + [...]  If the target is not a string, signal an error.
> > > 
> > > We want to change that, so I think you can drop that sentence.
> >
> > Shouldn't it be dropped when that change is committed?
> 
> Then we would break documented behavior then.  What would be the gain?

Sorry, I must be confused about this.  You said "we want to change
that", so I interpreted that to mean that such a change was not yet
done and will be done shortly.  IMO, changes in documentation should
be committed together with code changes that modify the behavior
described in the documentation, thus my question.  Did I
misunderstand?

If you fear that having this sentence in the doc string will somehow
preclude us from making the code change, or make it harder, then the
doc string already says that, so if there's a problem, it is already
with us, no?

> > > But can we remove the sentence saying "Multiple occurrences of the
> > > same VAR refer to the same submatch."?  It's completely redundant
> > > IMHO.
> >
> > Is it redundant even when VAR is not a submatch number, but a symbol?
> 
> I think so.  A symbol VAR references the explicit binding created with
> (let VAR ...), i.e. a submatch, just like in the number case.

I'm just asking whether it will be immediately clear to users that any
reference to VAR refers to the same submatch.  I'm okay with removing
it if you say so.

> > Btw, in this part:
> >
> > > +  (let VAR SEXP...)  creates a new explicitly numbered submatch
> > > +                     that matches regular expressions SEXP, and
> > > +                     binds the match to VAR.
> >
> > Does "explicitly numbered" mean that VAR must be a number?  If it can
> > be something else, perhaps "explicitly named" is better?
> 
> AFAIK, in `let' VAR must be a symbol, but it seems the submatch is also
> numbered as side effect, e.g.
> 
> (pcase "Hala"
>   ((rx "H" (let x "a") (regex ".*") (backref 1)) x))
> ==> "a"

So you agree that "explicitly named" is a better wording?

> In `backref' the argument can be a number or a symbol VAR.  That's why I
> would prefer a different argument name in the docstring, "REF" maybe.  I
> had done that in my suggested patch.

I'm okay with using REF there.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > > > > + [...]  If the target is not a string, signal an error.
> > > > 
> > > > We want to change that, so I think you can drop that sentence.
> > >
> > > Shouldn't it be dropped when that change is committed?
> > 
> > Then we would break documented behavior then.  What would be the gain?
>
> Sorry, I must be confused about this.  You said "we want to change
> that", so I interpreted that to mean that such a change was not yet
> done and will be done shortly.  IMO, changes in documentation should
> be committed together with code changes that modify the behavior
> described in the documentation, thus my question.  Did I
> misunderstand?

No, I don't think so.

> If you fear that having this sentence in the doc string will somehow
> preclude us from making the code change, or make it harder, then the
> doc string already says that, so if there's a problem, it is already
> with us, no?

No, my fear is that users or package maintainers rely on what we
document now although we surely know what we add to the docs won't hold
later.  IOW, what you suggest to document is a misfeature we want to get
rid of very soon.  I intend to do this right after this commit we speak
about here (but in master).  The main effect would be a merge conflict.

> > AFAIK, in `let' VAR must be a symbol, but it seems the submatch is also
> > numbered as side effect, e.g.
> > 
> > (pcase "Hala"
> >   ((rx "H" (let x "a") (regex ".*") (backref 1)) x))
> > ==> "a"
>
> So you agree that "explicitly named" is a better wording?

I think it would be an improvement, yes.


Michael.

Re: Clarify `pcase' `rx' pattern doc

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: emacs-devel@gnu.org
> Date: Sat, 07 Jul 2018 16:35:32 +0200
> 
> > If you fear that having this sentence in the doc string will somehow
> > preclude us from making the code change, or make it harder, then the
> > doc string already says that, so if there's a problem, it is already
> > with us, no?
> 
> No, my fear is that users or package maintainers rely on what we
> document now although we surely know what we add to the docs won't hold
> later.  IOW, what you suggest to document is a misfeature we want to get
> rid of very soon.  I intend to do this right after this commit we speak
> about here (but in master).  The main effect would be a merge conflict.
> 
> > > AFAIK, in `let' VAR must be a symbol, but it seems the submatch is also
> > > numbered as side effect, e.g.
> > > 
> > > (pcase "Hala"
> > >   ((rx "H" (let x "a") (regex ".*") (backref 1)) x))
> > > ==> "a"
> >
> > So you agree that "explicitly named" is a better wording?
> 
> I think it would be an improvement, yes.

OK, fixed as discussed and pushed to the emacs-26 branch.

Thanks.

Re: Clarify `pcase' `rx' pattern doc

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> OK, fixed as discussed and pushed to the emacs-26 branch.

Thanks, Eli.


Michael.