Subject: bug#31311: 27.0; doc of `pcase'

From: Drew Adams <[email-masked]>
Date: Sun, 29 Apr 2018 19:47:08 -0700 (PDT)

Hi Michael,

> I've read the docstring again.  There are indeed many things left
> unclear.  I think a prior version already had been clearer, though, much
> also more brief.

If it was both clearer and more brief, let's consider
resurrecting it. ;-)

> > 2. Don't use ATOM unless you mean any atom, even if you explain
> >    subsequently that you really mean a self-evaluating atom.
> 
> The best part of the doc string is the "complete list of patterns".
> What is says is right: "ATOM can be a keyword, an integer, or a string".
> In particular, floats are excluded.  nil and t are also excluded.
> "KEYWORD-OR-INTEGER-OR-STRING" is not a good name, however.

It may not be a good name, but it is not so misleading.
And it's not used in a zillion places, so it's not a
big deal if the name is long: INTEGER-STRING-OR-KEYWORD
or even perhaps INTEGER|STRING|KEYWORD.  We have much
longer stuff in the description of font-lock stuff
and elsewhere.  Complex things need careful breakdowns
and named parts.

"Atom", without that qualification, includes lots of
other things, including vectors and, in particular,
symbols.

Maybe it should say something about _why_ keywords,
integers, and strings are allowed here, but not
vectors (since strings are OK) or other numbers.
What is it about those 3 types that make them
special in this regard?

Oh, and do the other two occurrences of ATOM -
those in the description of Q-patterns - mean the
same thing: keyword, string, or integer?  Nothing
said about that; not clear.

A user really does seem better off, in the case of
this feature, reading the source code and ignoring
the doc.  The doc confuses more than helps.  It
raises more questions that it answers, I think.

> > 3. The doc string (but not the Elisp manual, node `Pattern matching
> case
> >    statement') refers to "the object" without ever specifying what it
> >    is.  This makes all of the specifications that use "the object"
> >    meaningless: 'VAL, (pred FUN), and (app FUN PAT).
> >
> >    The manual uses "the value being matched" instead, which is OK
> >    (understandable).
> 
> Yes, that's what I would prefer.
> 
> >  Still, it would be clearer to give a name to "the value being
> >  matched" at the beginning: EXPVAL, for example, at the place where
> >  you say, "based on the value of the expression".
> 
> That's not necessarily true, however.  For example, in
> (pcase .9 ((let (pred integerp) 1) t))
> (which evals to t) "(pred integerp)" is matched against 1, not the
> original expression (which is ignored by the pattern in this case).

Then such exceptions need to be pointed out.  Just
saying (over and over, BTW) "the value being matched",
let alone "the object", does not make that clear.

There is not a good, comprehensive presentation of
just what _matching_ involves, it seems.

And that's the _whole point_ of something like `pcase':
what it's matching does; how it works.  If a user can't
get a good mental model of this feature from reading
the doc then the doc hasn't done its job.

The behavior of such a `let' pattern is not obvious,
even with the mini-example presented.

As a first approximation, and as the overall
description and name, EXPVAL is OK, I think, as is
"the value of EXPRESSION".

The first paragraph makes it clear, even if it is not
100% true, that each PATTERN describes EXPRESSION's
value in some sense (typically its structure).

We have to have some basis for talking about the parts
and pieces.  Especially when things are complicated is
it helpful or necessary to name things and be precise
about which ones we're talking about.

> > 5. Don't use "QPattern" and "UPattern".  Use "Q pattern" and "U
> >    pattern", or (probably better) "Q-pattern" and "U-pattern".
> 
> I would rather like to get rid from these old names completely.

OK.  What new names would you propose?

But I think I agree with you.  Seems like just PATTERN
would make sense where we use both QPATTERN and UPATTERN.
The backquote char is anyway not part of a QPATTERN; it
precedes it.

Which points to another thing that's not clear: the
description, under QPATTERN of ",UPATTERN":

  "Matches if the corresponding element of the value
   being matched matches the specified UPATTERN."

Once again we're lost.  What on earth is "the
_corresponding element_ of the value being matched"?
What are the elements of the value being matched,
and how does one of them correspond to a ",UPATTERN"?

> > 6. This part of the description of `guard' is unclear, to me:
> >
> >    For example ... and let-binds the variable 'x' to that number.
> >
> >    I see nothing in the descriptions of `guard' and `and' that
> >    indicates why `x' would be let-bound in this example.
> 
> It's just the SYMBOL case: "SYMBOL matches anything and binds it to
> SYMBOL."  And "(and PAT...) matches if all the patterns match."

OK.  So x is bound to the value of EXPRESSION, and the
whole pattern matches if x is non-nil and < 10.  Wouldn't
it be better as something like this?

  (and x (guard (and (numberp x) (< x 10))))

The description says that (and x (guard (< x 10))) matches
any number smaller than 10.  But what happens if what x
matches and is thus bound to is not a number at all?

In fact, that's another thing that's unclear to me.  What
happens during "matching" that would normally raise an
error?  Matching involves some kind of comparison (even
unification matching does).  What happens when things
that get compared during matching are incomparable?

Then, the following sentence seems not always to hold or,
rather, it seems meaningless.  How can an _occurrence_ of
a symbol become an eq test?

 "If a SYMBOL is used twice in the same pattern the second
  occurrence becomes an 'eq'uality test."

That doc-string statement is missing from the manual,
AFAICT.  What is it really trying to say?  Where's the
eq test in this: (and x (guard (< x 10)))?

I almost feel like the more closely I read this doc the
less clear it gets.

I'm serious when I say that it seems like a blog about
using `pcase' would be helpful.  But that wouldn't
obviate coming up with clear and complete doc for the
manual, and clear doc for the doc string.