bug#31311: 27.0; doc of `pcase'

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

> It seems that no one really cares to document this facility.

I care enough about `pcase' and its doc to file this bug report.

> Maybe 'pcase' is seen as a kind of test: if you succeed in
> using and understanding it without any clear documentation,
> you are admitted to certain quarters where only wizards can enter.

If that's the approach being taken then perhaps remove all
of the doc.  If users need to resort to analyzing the code
and trying examples (tests) then perhaps no doc is better
than misleading doc.

I don't think that's the case, but if you think it is then
please remove the doc.  I think the doc is partly helpful.
But it needs fixing.  Dunno whether it is currently better
than nothing, but I think so.

> What you see in the manual is mostly the result of desperate efforts
> of a single individual to have something remotely palatable in our
> documentation.
> 
> Consequently, bug reports without patches are unlikely to yield any
> real results, IME.

Too bad.

Perhaps you can at least remove the incomprehensible and
seemingly unrelated text (I cited) that appears at the end
of the doc string, as a start.

There are also some other parts of my report that can be
acted on without any real knowledge of `pcase', I believe.

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> Date: Sun, 29 Apr 2018 10:02:16 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 31311@debbugs.gnu.org
> 
> > What you see in the manual is mostly the result of desperate efforts
> > of a single individual to have something remotely palatable in our
> > documentation.
> > 
> > Consequently, bug reports without patches are unlikely to yield any
> > real results, IME.
> 
> Too bad.
> 
> Perhaps you can at least remove the incomprehensible and
> seemingly unrelated text (I cited) that appears at the end
> of the doc string, as a start.
> 
> There are also some other parts of my report that can be
> acted on without any real knowledge of `pcase', I believe.

Sorry, I refuse to invest any more efforts in documenting this.  If
there's someone else out there who cares, let them show their resolve.

I'm sorry I need to choose this form of protest, but I tried
everything to cause those who know about this more than I do to write
some decent documentation, and I'm out of ideas.  Shame on me.

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> Perhaps you can at least remove the incomprehensible and
> seemingly unrelated text (I cited) that appears at the end
> of the doc string, as a start.

This is not unrelated: These are the docs for additional pcase patterns
defined elsewhere: pcase is extensible, and the pattern listing part in
the docstring is generated dynamically.  As a result of that conception
it may not read very fluently as a whole, but it's definitely nothing we
want to remove.

We clearly have some deficiencies, but the situation IMHO really is not
as horrible as Eli's sarcasm suggests.


Michael.

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

> > Perhaps you can at least remove the incomprehensible and
> > seemingly unrelated text (I cited) that appears at the end
> > of the doc string, as a start.
> 
> This is not unrelated: These are the docs for additional pcase patterns
> defined elsewhere: pcase is extensible, and the pattern listing part in
> the docstring is generated dynamically.  As a result of that conception
> it may not read very fluently as a whole, but it's definitely nothing we
> want to remove.

Sorry, but that extra (dynamically generated?) text at the
end makes no sense at all to me.  I don't have a clue what
it's trying to convey.  To me it's just noise that can only
confuse, not help, users.

If there is some core meaning behind it then great.  In
that case there is a potential for it to say something.

That's not happening at all now, I think.  For the time
being, i.e., until someone can translate that core meaning
(what you really expect it to be trying to say) into
understandable text, it should be removed.  Please consider
moving it to a TODO item somewhere, if you like.  But maybe
others disagree and only I have trouble seeing what that
text is all about.

Beyond that, if it is about showing examples of defining
additional `pcase' patterns in the Emacs code then I
don't think that kind of thing belongs in a doc string -
certainly not multiple such examples.

Even in the Elisp manual, I'd expect only a simple example
of defining a `pcase' pattern, in the node itself.  If it
were thought to really be helpful then there could perhaps
also be a note to see the code of this or that function.
But I really don't expect that that should be necessary.
Do we do that anywhere else?

Either it is simple to define your own `pcase' patterns
or it is not.  If it is, then no such extra examples
should be needed.  If it's not, then can't we make it
easier to define your own patterns (change the product,
not just the doc)?

From what you are saying, it sounds to me like what users
need to make sense of `pcase' is a blog article, showing
examples etc. and building up from simple to complex.
I'd rather encourage someone to write that than for us
to try to cram such info into a doc string or manual node.
 
> We clearly have some deficiencies, but the situation IMHO really is not
> as horrible as Eli's sarcasm suggests.

FWIW, I don't see what Eli wrote as sarcasm.  I do see it
as negative, nearly despondent.  But sarcasm involves some
element of humor, irony, or satire, which I don't find there.

In any case, there are two different problems being
discussed so far in this thread: (1) the doc is bad, and
(2) it has been given insufficient love.

This bug is really about #1.  But according to Eli, #1
likely won't be fixed because of #2.  Dunno whether he
is right, but #1 is the problem to report.

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> That's not happening at all now, I think.  For the time
> being, i.e., until someone can translate that core meaning
> (what you really expect it to be trying to say) into
> understandable text, it should be removed.  Please consider
> moving it to a TODO item somewhere, if you like.  But maybe
> others disagree and only I have trouble seeing what that
> text is all about.

Yes, that's the case.  It's not your fault that you don't understand
what the text is about, however.

> Beyond that, if it is about showing examples of defining
> additional `pcase' patterns in the Emacs code then I
> don't think that kind of thing belongs in a doc string -
> certainly not multiple such examples.

These are not examples.  These are the descriptions of the semantics of
additionally defined pattern types.  Yes, there's a sentence missing
saying something like "the following additional pattern types are
currently defined" or so.


Michael.

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

> Sorry, I refuse to invest any more efforts in documenting this.  If
> there's someone else out there who cares, let them show their resolve.
> 
> I'm sorry I need to choose this form of protest, but I tried
> everything to cause those who know about this more than I do to write
> some decent documentation, and I'm out of ideas.  Shame on me.

Understood.  No sorry, no shame.  Emacs doc is as good as
it is because of your efforts - thank you.

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

1. Please rename Elisp manual node `Pattern matching case statement'.
   It should use Title Case (not Sentence case), like the other nodes.
   And do not refer to "case", as the other index entries that match
   `case' have nothing to do with conditionals.  And the node name can
   be shorter, e.g., `Pattern-Matching Conditional'.

2. Don't use ATOM unless you mean any atom, even if you explain
   subsequently that you really mean a self-evaluating atom.  In
   particular, an arbitrary symbol is an atom, and (especially if ATOM
   is described before SYMBOL) ATOM would seem to cover the SYMBOL case,
   which it does not.  Just use "CONSTANT" instead of "ATOM".  AFAIK,
   every self-evaluating sexp in Emacs Lisp satisfies `atomp' (but not
   everything that satisfies `atomp' is a self-evaluating sexp).

   The doc should also make clear whether the ATOM clause covers a
   symbol defined using `defconst'.  The answer seems to be no; it
   seems to be covered instead by the SYMBOL clause.

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).  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 EXPRESSION".

   Similarly, replace "value of the EXPRESSION that is the first
   argument of 'pcase'" by the name you give that (e.g. EXPVAL).

4. The doc string seems to be describing a different animal from the
   manual.  Try to harmonize the two descriptions, including wrt the
   order of pattern presentation.

5. Don't use "QPattern" and "UPattern".  Use "Q pattern" and "U
   pattern", or (probably better) "Q-pattern" and "U-pattern".

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.

7. The doc-string descriptions do not correspond to those in the manual,
   in several cases.  E.g. (app FUN PAT) is different from (app FUNCTION
   UPATTERN).  Is the pattern necessarily a U-pattern?  Similary for
   `or' etc.

8. Please check for typos.  E.g., "Matches if one the argument UPatterns
   matches."

9. This is not clear to m: "For this reason, if any of the UPatterns
   let-bind symbols to the matched value, they should all bind the same
   symbols."  Should it instead say that they should all bind the same
   symbols to the matched value"?  Also, does a similar thing need to be
   said for `and'?

10. This text: "The function calls used in the 'pred' and 'app'
    UPatterns can have one of the following forms" seems wrong.
    Presumably what it is trying to say is that PREDFUN and FUNCTION
    "can have one of the following forms".  Those forms are not
    necessarily function calls.  Similarly, for the text "The FUNCTION
    call can use one of the forms described below" - just say "FUNCTION
    can use...", not "The FUNCTION call can use...".

11. Do not say "lambda-function".  Say either "lambda expression" or
    "anonymous function".  There is no such thing as a lambda function.
    You might also want to xref node `Lambda Expressions'.

12. "Here is an illustrative example" -> "Here is an example".

13. "you can use backquoted patterns that are more powerful" -> "you can
    use backquoted patterns, which that are more powerful".  The comma
    is important.  This part of the node is not "in addition" to some
    previous description of backquoted patterns; this _is_ the
    description of backquoted patterns, i.e., Q-patterns.  What came
    before was the description of unquoted patterns, i.e., U-patterns.

14. "(note that this example requires lexical binding, *note Lexical
    Binding::)".  Say _why_ this is the case, not just that it is true.
    Why does it require lexical binding?

    Also, the code needs wrapping; two of the lines are too long:

    (funcall (evaluate fun env) (evaluate arg env)) ->
    (funcall (evaluate fun env)
             (evaluate arg env))

    (lambda (val)
      (evaluate body (cons (cons arg val) env))) ->

    (lambda (val)
      (evaluate body
                (cons (cons arg val) env)))

15. Break up the paragraph that begins "Here '`(add ,x ,y)' is a..."

16. All of the following text in the _doc string_ is pretty much
   incomprehensible, to me.  It's not clear what it's trying to say or
   even, in some cases, how it relates to `pcase' at all.  And most of
   it seems to have no correspondence in the manual.

   -- (radix-tree-leaf VPAT)

   Not documented.

   -- (cl-struct TYPE &rest FIELDS)

   Pcase patterns to match cl-structs.
   Elements of FIELDS can be of the form (NAME PAT) in which case the
   contents of field NAME is matched against PAT, or they can be of the
   form NAME which is a shorthand for (NAME NAME).

   -- (seq &rest PATTERNS)

   Build a 'pcase' pattern that matches elements of SEQUENCE.

   The 'pcase' pattern will match each element of PATTERNS against the
   corresponding element of SEQUENCE.

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

   -- (eieio &rest FIELDS)

   Pcase patterns to match EIEIO objects.
   Elements of FIELDS can be of the form (NAME PAT) in which case the
   contents of field NAME is matched against PAT, or they can be of the
   form NAME which is a shorthand for (NAME NAME).

   -- `QPAT

   Backquote-style pcase patterns.
   QPAT can take the following forms:
     (QPAT1 . QPAT2)       matches if QPAT1 matches the car and QPAT2 the cdr.
     [QPAT1 QPAT2..QPATn]  matches a vector of length n and QPAT1..QPATn match
                              its 0..(n-1)th elements, respectively.
     ,PAT                  matches if the pcase pattern PAT matches.
     ATOM                  matches if the object is 'equal' to ATOM.
                              ATOM can be a symbol, an integer, or a string.

In GNU Emacs 27.0.50 (build 3, x86_64-w64-mingw32)
 of 2018-03-21
Repository revision: e70d0c9e66d7a8609450b2889869d16aeb0363b5
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install -C 'CFLAGS=-O2 -static -g3''

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

It seems that no one really cares to document this facility.  Maybe
'pcase' is seen as a kind of test: if you succeed in using and
understanding it without any clear documentation, you are admitted to
certain quarters where only wizards can enter.

What you see in the manual is mostly the result of desperate efforts
of a single individual to have something remotely palatable in our
documentation.

Consequently, bug reports without patches are unlikely to yield any
real results, IME.

TIA

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> It seems that no one really cares to document this facility.  Maybe
> 'pcase' is seen as a kind of test: if you succeed in using and
> understanding it without any clear documentation, you are admitted to
> certain quarters where only wizards can enter.
>
> What you see in the manual is mostly the result of desperate efforts
> of a single individual to have something remotely palatable in our
> documentation.

My impression was more that there were diametral conceptions of how a
good documentation would look like.  And there _has_ been some
improvements of the original (sparse) docs, I now find the docstring of
the macro itself quite good (and complete).  The texts in the manual are
not so good for otoh, I agree.


Michael.

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Drew Adams <drew.adams@oracle.com>,  31311@debbugs.gnu.org
> Date: Sun, 29 Apr 2018 20:31:59 +0200
> 
> > What you see in the manual is mostly the result of desperate efforts
> > of a single individual to have something remotely palatable in our
> > documentation.
> 
> My impression was more that there were diametral conceptions of how a
> good documentation would look like.

There's nothing special about this feature that should present
conceptual problems for documenting it well, any more than any other
advanced feature of which we have dozens in the ELisp manual.

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> There's nothing special about this feature that should present
> conceptual problems for documenting it well, any more than any other
> advanced feature of which we have dozens in the ELisp manual.

From the beginning people were divided about the usefulness and
conception of the thing, and people discussed exceptionally emotionally
and destructively.


Michael.

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: drew.adams@oracle.com,  31311@debbugs.gnu.org
> Date: Sun, 29 Apr 2018 22:05:55 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > There's nothing special about this feature that should present
> > conceptual problems for documenting it well, any more than any other
> > advanced feature of which we have dozens in the ELisp manual.
> 
> >From the beginning people were divided about the usefulness and
> conception of the thing, and people discussed exceptionally emotionally
> and destructively.

Irrelevant.  We are talking about documentation, not about the feature
itself.  The concept of how such complex features should be documented
is well established in Emacs, and we have a wealth of examples in the
ELisp manual of how it should be and is done.

bug#31311: 27.0; doc of `pcase'

[Noam Postavsky]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Michael Heerdegen <michael_heerdegen@web.de>
>> Cc: drew.adams@oracle.com,  31311@debbugs.gnu.org
>> Date: Sun, 29 Apr 2018 22:05:55 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > There's nothing special about this feature that should present
>> > conceptual problems for documenting it well, any more than any other
>> > advanced feature of which we have dozens in the ELisp manual.
>> 
>> From the beginning people were divided about the usefulness and
>> conception of the thing, and people discussed exceptionally emotionally
>> and destructively.
>
> Irrelevant.  We are talking about documentation, not about the feature
> itself.  The concept of how such complex features should be documented
> is well established in Emacs, and we have a wealth of examples in the
> ELisp manual of how it should be and is done.

It's relevant if potential contributors turn away/tune out/ignore
the whole thing due to the unpleasant discussion.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Noam Postavsky <npostavs@gmail.com>
() Mon, 30 Apr 2018 07:20:23 -0400

   It's relevant if potential contributors turn away/tune
   out/ignore the whole thing due to the unpleasant discussion.

No worries from my part.  Unpleasant discussion is better than
no discussion, IMHO.  Anyway, to start transforming discussion
energy (pleasant or otherwise) into repo commits, i've just
pushed branch ‘fix/bug-31311-pcase-doc’, w/ these commits:

 4ad382d 2018-04-30 add blank line before each @item in @table
 8fccdaa 2018-04-30 rewrite @item @var{atom}
 afb5aa4 2018-04-30 move UPattern and QPattern tables outside of @defmac pcase
 0fa3538 2018-04-30 add blank line before each @item in @table

Don't worry about the informal tone; i intend to eventually
squash this branch to a (single) commit, and normalize the
commit message to Emacs standards, etc.

In the meantime, i invite people to push freely to it.  Many of
the suggestions i've seen in this thread have merit at first
blush.  Let's try to realize them (or rationale alternatives),
together, and see how the concrete result stands.  In this way,
we do a kind of "git rebase changes discussion", hopefully
improve the discussion (too!), and most importantly, take
forward action on this important (i'm a big fan of ‘pcase’)
task.

"But ttn, you sound like one of those manglement types!"

Yes, but hopefully i act better than most of them out there, by
doing and encouraging others to do.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

Thanks for working on this, ttn.

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Thien-Thi Nguyen <ttn@gnu.org> writes:

>  4ad382d 2018-04-30 add blank line before each @item in @table
>  8fccdaa 2018-04-30 rewrite @item @var{atom}
>  afb5aa4 2018-04-30 move UPattern and QPattern tables outside of @defmac pcase
>  0fa3538 2018-04-30 add blank line before each @item in @table

Thanks for the initiative.

One comment:

+@item @var{keyword}
+@itemx @var{number}
+@itemx @var{string}
+Matches if the value is @code{equal} to the literal.
+This is a special case of @code{'@var{val}}, above,
+possible because these types are self-quoting.

Not all numbers are allowed; only integers - for example

(pcase .9
  (.9 t))

|-- Unknown pattern `0.9'

See the definition of `pcase--self-quoting-p', it's intentional.


Michael.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Michael Heerdegen <michael_heerdegen@web.de>
() Mon, 30 Apr 2018 20:04:12 +0200

   Not all numbers are allowed; only integers - for example

   (pcase .9
     (.9 t))

   |-- Unknown pattern `0.9'

   [...] `pcase--self-quoting-p', it's intentional.

Thanks.  I've made the change.  Doing so, i realized i've
already foolishly goofed by branching off of ‘master’ instead of
‘emacs-26’.  So, apologies for the confusion to anyone who might
have already pulled ‘fix/bug-31311-pcase-doc’.  I've deleted
that branch and recreated it, this time from ‘emacs-26’.  Here
are the current list of commits:

 46d58247f1  add blank line before each @item in @table
 32eee97a74  rewrite @item @var{atom}
 f870e5e766  move UPattern and QPattern tables outside of @defmac pcase
 702131a8b2  add blank line before each @item in @table
 892995705f  explode first ‘@def pcase’ para
 e8dd2a7724  s/number/integer/ per feedback from Michael Heerdegen <874ljso9k3.fsf@web.de>

Note that commit 892995705f includes some stylized comments.  I
invite interested people to include discussion of those in this
thread.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Thien-Thi Nguyen <ttn@gnu.org>
> Date: Mon, 30 Apr 2018 15:35:32 +0200
> Cc: 31311@debbugs.gnu.org
> 
> Anyway, to start transforming discussion energy (pleasant or
> otherwise) into repo commits, i've just pushed branch
> ‘fix/bug-31311-pcase-doc’, w/ these commits:
> 
>  4ad382d 2018-04-30 add blank line before each @item in @table
>  8fccdaa 2018-04-30 rewrite @item @var{atom}
>  afb5aa4 2018-04-30 move UPattern and QPattern tables outside of @defmac pcase
>  0fa3538 2018-04-30 add blank line before each @item in @table

Thanks!

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Eli Zaretskii <eliz@gnu.org>
() Mon, 30 Apr 2018 22:31:14 +0300

   Thanks!

Trundling along, so far so good.  I wish to call attention to
(i.e., explicitly request input for) these two recent commits:

* c6e9b975f  add some Issue comments; nfc

  This first set of ponderables revolves around the precise
  semantics of the SYMBOL let-binding feature.

* b93c07ef0  (docstring) add inline Issue re sub-patterns of structural patterns

  This is a writing question, mostly, i think.

Additionally, related to SYMBOL binding, i see (in *scratch*):

  a.  (pcase t   ('t   'true))  => true
  b.  (pcase nil ('nil 'false)) => false
  c.  (pcase t   (t    'true))  => true
  d.  (pcase nil (nil  'false)) |= ERROR: Unknown pattern ‘nil’

and wonder if/how to approach this corner case in the docs.  I
suppose before anything, i'd like to confirm that c. and d. are
correct and intended (i believe a. and b. to be correct and
intended, but maybe i'm missing something there, too!).

For anyone just joining, these commits are on the branch
‘fix/bug-31311-pcase-doc’ (off of ‘emacs-26’).  Feedback on
anything there is most welcome!

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Thien-Thi Nguyen <ttn@gnu.org> writes:

>   a.  (pcase t   ('t   'true))  => true
>   b.  (pcase nil ('nil 'false)) => false
>   c.  (pcase t   (t    'true))  => true
>   d.  (pcase nil (nil  'false)) |= ERROR: Unknown pattern ‘nil’
>
> and wonder if/how to approach this corner case in the docs.  I
> suppose before anything, i'd like to confirm that c. and d. are
> correct and intended (i believe a. and b. to be correct and
> intended, but maybe i'm missing something there, too!).

That's all correct and intended - see 3ef31167.  t was originally
intended as catchall pattern equivalent to _ but has been deprecated
(since it was confusing).  Try e.g.

  (pcase nil
   (t t))

You get a message when you use it, but when evaluating and not when
compiling (so you must look in the *Messages* buffer - I don't think
this is intended...).  A catch-nothing pattern is not useful, so nil is
just invalid.

I once asked Stefan if t and nil could be handled like the ATOM case,
but he didn't want to do this (backward compatibility, don't recall).


Michael.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Michael Heerdegen <michael_heerdegen@web.de>
() Sat, 12 May 2018 15:54:18 +0200

   >   a.  (pcase t   ('t   'true))  => true
   >   b.  (pcase nil ('nil 'false)) => false
   >   c.  (pcase t   (t    'true))  => true
   >   d.  (pcase nil (nil  'false)) |= ERROR: Unknown pattern ‘nil’

   That's all correct and intended - see 3ef31167.  t was
   originally intended as catchall pattern equivalent to _ but
   has been deprecated (since it was confusing).  Try e.g.

     (pcase nil
      (t t))

   You get a message when you use it, but when evaluating and
   not when compiling (so you must look in the *Messages* buffer
   - I don't think this is intended...).  A catch-nothing
   pattern is not useful, so nil is just invalid.

Thanks.  I've added a blurb suggesting to avoid both ‘t’ and
‘nil’ (commit 5fc0f24c).  Here's the text:

 Two symbols to avoid are @code{t}, which behaves like @code{_}
 (above) and is deprecated, and @code{nil}, which signals error.
 It makes no sense to bind these (@pxref{Constant Variables}).

This is in the table, part of the @var{symbol} entry.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Thien-Thi Nguyen <ttn@gnu.org> writes:

> Thanks.  I've added a blurb suggesting to avoid both ‘t’ and
> ‘nil’ (commit 5fc0f24c).  Here's the text:
>
>  Two symbols to avoid are @code{t}, which behaves like @code{_}
>  (above) and is deprecated, and @code{nil}, which signals error.
>  It makes no sense to bind these (@pxref{Constant Variables}).

BTW, keywords are another case - it might be obvious that trying to bind
these symbols might not be a good idea, but it should be told for
completeness.

Maybe we should move the ATOM case in the pattern list of the pcase
docstring before the SYMBOL case?  IMO the listing should be in an order
so that the first fitting explanation is valid.


Michael.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Michael Heerdegen <michael_heerdegen@web.de>
() Tue, 15 May 2018 17:16:00 +0200

   BTW, keywords are another case - it might be obvious that
   trying to bind these symbols might not be a good idea, but it
   should be told for completeness.

Thanks; good catch.  See commit 2db569560c, just pushed.

   Maybe we should move the ATOM case in the pattern list of the
   pcase docstring before the SYMBOL case?

Please see these commits:
 80ed201  (docstring) explode "ATOM"; flatten; zonk "match themselves" sentence
 f927f6a  (docstring) move { 'VAL, KEYWORD, INTEGER, STRING } prior to SYMBOL

Similarly, for QPAT:
 e436b49  for structural / Q-pattern, replace "atom" w/ SYMBOL, INTEGER, STRING

The list in Texinfo has been likewise re-ordered.  (Sorry, too
much bother to dig up specific commits -- you can build .info or
.pdf and take a look for yourself.)

Change of focus: In commit 5b775cf3fc there is an Issue.  WDYT?

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Thien-Thi Nguyen <ttn@gnu.org> writes:

> Please see these commits:
>  80ed201  (docstring) explode "ATOM"; flatten; zonk "match themselves" sentence
>  f927f6a  (docstring) move { 'VAL, KEYWORD, INTEGER, STRING } prior to SYMBOL

Ok, fine.

Personally I would prefer to have KEYWORD, INTEGER and STRING in one
line, like

KEYWORD, INTEGER, STRING
                shorthand for \\='KEYWORD, \\='INTEGER,
                and \\='STRING

so that we have as least different cases as possible in the item list.

> Similarly, for QPAT:
>  e436b49  for structural / Q-pattern, replace "atom" w/ SYMBOL, INTEGER, STRING
>
> The list in Texinfo has been likewise re-ordered.  (Sorry, too
> much bother to dig up specific commits -- you can build .info or
> .pdf and take a look for yourself.)

It's ok for me.  I'll have a look at the complete change when the
details are done.

> Change of focus: In commit 5b775cf3fc there is an Issue.  WDYT?

Yes, the code seems a bit half-baked.  I don't recall why these cases
are so problematic that they are handled this way.  I think Stefan will
know.


Michael.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Michael Heerdegen <michael_heerdegen@web.de>
() Wed, 16 May 2018 17:18:14 +0200

   Personally I would prefer to have KEYWORD, INTEGER and STRING
   in one line, like

   KEYWORD, INTEGER, STRING
            shorthand for \\='KEYWORD, \\='INTEGER,
            and \\='STRING

   so that we have as least different cases as possible in the
   item list.

Yeah, that does seem a bit more pleasant.  To achieve that,
however, we would need to use ‘@table @asis’ and then manually
add @code around everything, so that the commas are not included
in @code.  Hmmm, energy seeping away...

   It's ok for me.  I'll have a look at the complete change when
   the details are done.

As of commit 7c68d9f8c7, "the details are done", but for:

   > commit 5b775cf3fc there is an Issue.  WDYT?

   Yes, the code seems a bit half-baked.  I don't recall why
   these cases are so problematic that they are handled this
   way.  I think Stefan will know.

and the other remaining Issue (commit 455f990ce), excerpt here:

 +@c Issue: Should this be split off into its own node/subsection?
 +@subheading Backquote-Style Patterns
 +@cindex backquote-style patterns

IMHO, ‘pcase’ is so complex (compared to most other Emacs Lisp
control structures), it deserves to be promoted to @‘section’,
between ‘Conditionals’ and ‘Constructs for Combining Conditions’.

What do people think?

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

Thanks for working on this, guys.

I can't see the result, but I'll see it in the next
release.  I'm sure that it's been improved quite a bit,
even just from the fact that a close look at it has
been taken and difficulties have been discussed.  Thx.

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> Date: Wed, 23 May 2018 06:55:48 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: Michael Heerdegen <michael_heerdegen@web.de>
> 
> Thanks for working on this, guys.
> 
> I can't see the result

Yes, you can: point your Web browser to this URL:

   http://git.savannah.gnu.org/cgit/emacs.git

then click on the link to the relevant branch, in this case

   http://git.savannah.gnu.org/cgit/emacs.git/log/?h=fix/bug-31311-pcase-doc-squash

and then navigate to the file you want to read.  You can also click on
the "diff" link to see only the changes.

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Thien-Thi Nguyen <ttn@gnu.org>
> Date: Sun, 20 May 2018 20:59:58 +0200
> Cc: Michael Heerdegen <michael_heerdegen@web.de>
> 
> and the other remaining Issue (commit 455f990ce), excerpt here:
> 
>  +@c Issue: Should this be split off into its own node/subsection?
>  +@subheading Backquote-Style Patterns
>  +@cindex backquote-style patterns
> 
> IMHO, ‘pcase’ is so complex (compared to most other Emacs Lisp
> control structures), it deserves to be promoted to @‘section’,
> between ‘Conditionals’ and ‘Constructs for Combining Conditions’.
> 
> What do people think?

Why is this important?  You can always add subsections,
subsubsections, etc., if you want to structure the text into more
parts than it is already.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Eli Zaretskii <eliz@gnu.org>
() Wed, 23 May 2018 18:28:19 +0300

   >  +@c Issue: Should this be split off into its own
   >  node/subsection?

   Why is this important?

It is important because ‘pcase’ is a construct that selects
based on new concepts: "pattern" and "matching".  It also
supports defining and sharing (to some extent) let-bindings.
All the other conditional constructs select on value (squashed
to boolean, that is), and don't have any let-binding support.

It's true that the macro eventually expands to a ‘let’-wrapped
‘cond’, but my understanding that this expansion is an
implementation detail -- maybe in the future it will be expanded
in another more-fitting way.  So, the new concepts stand on
their own, rather than "stylized expressions for a predicate in
the ‘cond’ CONDITION position".

Also, ‘pcase’ is the only conditional construct that takes a
first arg and evaluates it (like ‘case’).  The new text explains
its hybrid nature (‘cond’ and ‘case’) in more detail.

The last reason is that ‘pcase’ supports "sequencing patterns",
i.e., ‘(and PAT...)’ and ‘(or PAT...)’.  These are analogous to
the same-named special forms documented in "Constructs for
Combining Conditions" and the ‘pcase’ docs points that out as a
conceptual backward-direction xref.  It's nice if the back-xref
is also in the text as well.  The reader sees/thinks:
- conditionals / four, ok, simple, no prob
- combining conditions / two, old hat, no prob
- pcase "sequencing conditions" / two, xref AH! like ‘and’, ‘or’
  for patterns instead of for values, new hat but still no prob

If we were to reverse the latter two (i.e., placing "Combining"
last), the reader would encounter the xref in the forward
direction.  I feel strongly that it's desirable to minimize
forward references in documentation (or work really hard to make
them less disconcerting).

   You can always add subsections, subsubsections, etc., if you
   want to structure the text into more parts than it is
   already.

In branch ‘fix/bug-31311-pcase-doc’ (note, not -squash), commit
567342a88e53 does such a re-arranging.  If you prefer, i can
demote everything one level and move it back to "Conditionals".
I'd then need to update that node's intro para to say "five".
That might reduce the diff, too.

However, i have explained (above) why i would be against this.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Thien-Thi Nguyen <ttn@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 23 May 2018 21:16:42 +0200
> 
>    >  +@c Issue: Should this be split off into its own
>    >  node/subsection?
> 
>    Why is this important?
> 
> It is important because ‘pcase’ is a construct that selects
> based on new concepts: "pattern" and "matching".  It also
> supports defining and sharing (to some extent) let-bindings.
> All the other conditional constructs select on value (squashed
> to boolean, that is), and don't have any let-binding support.
> 
> It's true that the macro eventually expands to a ‘let’-wrapped
> ‘cond’, but my understanding that this expansion is an
> implementation detail -- maybe in the future it will be expanded
> in another more-fitting way.  So, the new concepts stand on
> their own, rather than "stylized expressions for a predicate in
> the ‘cond’ CONDITION position".

The documentation of 'pcase' is inside Conditionals not because it
expands to 'cond', but because it can be perceived as a kind-of
generalization of 'cond' (and the current text even says so
explicitly).

> The last reason is that ‘pcase’ supports "sequencing patterns",
> i.e., ‘(and PAT...)’ and ‘(or PAT...)’.  These are analogous to
> the same-named special forms documented in "Constructs for
> Combining Conditions" and the ‘pcase’ docs points that out as a
> conceptual backward-direction xref.  It's nice if the back-xref
> is also in the text as well.  The reader sees/thinks:
> - conditionals / four, ok, simple, no prob
> - combining conditions / two, old hat, no prob
> - pcase "sequencing conditions" / two, xref AH! like ‘and’, ‘or’
>   for patterns instead of for values, new hat but still no prob
> 
> If we were to reverse the latter two (i.e., placing "Combining"
> last), the reader would encounter the xref in the forward
> direction.  I feel strongly that it's desirable to minimize
> forward references in documentation (or work really hard to make
> them less disconcerting).

I think you read too much into the tree-like structure of the manual,
in particular it sounds like you assume many people will read all the
sections of this chapter in strict depth-first order.

But that is not what happens in most use cases.  People usually read
just the part(s) they need to understand the particular feature they
need to use in their programs.  When read like this, the order matters
much less.  What does matter is that details and "advanced" features
are at lower levels, so that first reading doesn't require people to
negotiate too many obstacles unnecessarily, which would prevent them
from easily grasping the higher-level picture and main ideas.

So I personally don't see too many serious reasons to promote this
subsection to the level of a section; quite the contrary.  But neither
am I willing to make yet another dispute out of a minor issue such as
this.  If you feel strongly about this, feel free to do it.

P.S.  Your messages in this thread have a Mail-Followup-To header that
points back to the bug address, 31311@debbugs.gnu.org.  This causes
Rmail to produce both To and CC headers to the bug address when I
reply, and I'm forced to manually remove one of them, which is an
annoyance.  Would it be possible for you to avoid using that header,
please?  TIA.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Eli Zaretskii <eliz@gnu.org>
() Thu, 24 May 2018 19:23:22 +0300

   The documentation of 'pcase' is inside Conditionals not
   because it expands to 'cond', but because it can be perceived
   as a kind-of generalization of 'cond' (and the current text
   even says so explicitly).

Yes.  Commit 81d1eaff54ad corrects the count of conditional
forms, mentions this relationship, and adds xref.

   > [because of "sequencing patterns", pcase
   > should follow "Combining Conditions"]

   I think you read too much into the tree-like structure of the
   manual, in particular it sounds like you assume many people
   will read all the sections of this chapter in strict
   depth-first order.

When the output format is text or PDF, depth-first is the order
of presentation.  I was thinking about that use-case primarily,
for this portion of the reasoning.

   But that is not what happens in most use cases.  People
   usually read just the part(s) they need to understand the
   particular feature they need to use in their programs.  When
   read like this, the order matters much less.  What does
   matter is that details and "advanced" features are at lower
   levels, so that first reading doesn't require people to
   negotiate too many obstacles unnecessarily, which would
   prevent them from easily grasping the higher-level picture
   and main ideas.

Agreed.  Random-access readers need less help.

   So I personally don't see too many serious reasons to promote
   this subsection to the level of a section; quite the
   contrary.  But neither am I willing to make yet another
   dispute out of a minor issue such as this.  If you feel
   strongly about this, feel free to do it.

Ack.  My feelings are strong but not validated by experience (in
this case).  If people complain, i'll gladly change it back.

   P.S.  Your messages in this thread have a Mail-Followup-To
   header that [...] causes Rmail to produce both To and CC
   headers to the bug address when I reply, and I'm forced to
   manually remove one of them, which is an annoyance.  Would it
   be possible for you to avoid using that header, please?  TIA.

Sure, no problem.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Noam Postavsky]

Thien-Thi Nguyen <ttn@gnu.org> writes:

> Also, ‘pcase’ is the only conditional construct that takes a
> first arg and evaluates it (like ‘case’).  The new text explains
> its hybrid nature (‘cond’ and ‘case’) in more detail.

The manual should refer to `cl-case', not `case'.  I don't think pcase
should be referred to as a hybrid of cond and cl-case; maybe a hybrid of
cond and cl-destructuring-bind.  Or perhaps: like cl-case, only more so.

> +These render @code{case} unsuitable for strings or compound
> +data structures (e.g., lists or vectors).
> +For that, why not use @code{cond}?  Wait, what?  @dots{}

I guess this is supposed to be funny, but I think it might confuse the
reader.

> +(defun grok/traditional (obj)
> +  (if (and (stringp obj)
> +           (string-match "^key:\\([[:digit:]]+\\)$" obj))
> +      (match-string 1 obj)
> +    (list "149" 'default)))

> +(defun grok/pcase (obj)
> +  (pcase obj
> +    ((or                                     ; @r{line 1}
> +      (and                                   ; @r{line 2}
> +       (pred stringp)                        ; @r{line 3}
> +       (pred (string-match                   ; @r{line 4}
> +              "^key:\\([[:digit:]]+\\)$"))   ; @r{line 5}
> +       (app (match-string 1)                 ; @r{line 6}

You have to pass the original string to match-string.

> +            val))                            ; @r{line 7}
> +      (let val (list "149" 'default)))       ; @r{line 8}
> +     val)))                                  ; @r{line 9}

This doesn't seem like a great example of pcase usage.  Not sure if you
want to introduce the extended `rx' pattern here, but it works better
for this, IMO:

    (pcase x
      ((and (pred stringp)
            (rx bol "key:" (let val (+ (any digit))) eol))
       val)
      (_ 'default))

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Noam Postavsky <npostavs@gmail.com>
() Thu, 24 May 2018 19:13:40 -0400

   The manual should refer to `cl-case', not `case'.

   > +These render @code{case} unsuitable for strings or compound
   > +data structures (e.g., lists or vectors).
   > +For that, why not use @code{cond}?  Wait, what?  @dots{}

   I guess this is supposed to be funny, but I think it might
   confuse the reader.

I did ‘s/case/cl-&/g’ in commit 468e82790f1, and converted the
haha attempt text to: "(And here we end up in a circle.)"

   I don't think pcase should be referred to as a
   hybrid of cond and cl-case; maybe a hybrid of
   cond and cl-destructuring-bind.  Or perhaps:
   like cl-case, only more so.

Many computer languages have an intrinsic case-ish construct, so
it was a bit surprising for me to learn that ‘case’ in Emacs
Lisp is actually ‘cl-case’, which has a second-class citizen
feel.  In using ‘cl-case’ as one of the conceptual parents of
‘pcase’, my reasoning is:

- Build on "expectatious programmer" mindset.  Programmers new
  to Emacs Lisp might feel that same surprise i felt and do what
  i did: reach for ‘cl-case’ immediately, making it a habit to
  such an extent as to consider it intrinsic (thus, familiar).

- I agree somewhat that ‘cl-destructuring-bind’ might be a
  better parent, especially for structural pattern matching, but
  it does not have widespread analogs in other programming
  languages, and its operation is not as easy to grasp as a
  case-ish construct.  Too, structural patterns are but one kind
  of pattern that ‘pcase’ supports.

So, i understand your point (i believe) but disagree.

   > +       (app (match-string 1)                 ; @r{line 6}

   You have to pass the original string to match-string.

Yes, ‘app’ handles that.  There is an explanation in one of the
paragraphs following the code.  Anyway, thanks for pointing this
out, as the paragraph in question lacked a reference "line 6"
due to oversight on my part -- fixed in commit 6aace45830.

   > +            val))                            ; @r{line 7}
   > +      (let val (list "149" 'default)))       ; @r{line 8}
   > +     val)))                                  ; @r{line 9}

   This doesn't seem like a great example of pcase usage.  Not
   sure if you want to introduce the extended `rx' pattern here,
   but it works better for this, IMO:

       (pcase x
         ((and (pred stringp)
               (rx bol "key:" (let val (+ (any digit))) eol))
          val)
         (_ 'default))

Thanks for the tip; that's certainly more elegant.  This example
is admittedly a bit contrived, but i think its merit is that it
stays w/ the basic core patterns, using them in idiomatic ways.

I think i saw in another thread (bug report?) that rx.el needs
documentation.  It would be great if those docs could xref this
example w/ the re-re-formulation you wrote.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

>    The manual should refer to `cl-case', not `case'.

I disagree.  My (unsolicited) 2 cents:

`case' is aliased to `cl-case', IF you load `cl.el' at
byte-compile time.  It has always been so, and it should
remain so.  Or the other way around: we should instead
alias `cl-case' to `case'.

Encouraging everyone to instead load `cl-lib.el' does _not_
provide `case'.  If `cl-lib.el' is to be the way to go then
it should itself provide the alias.

Dunno whether this lack was an oversight or deliberate, but
it is a step backward.  We now (rightfully) encourage users
to load `cl-lib.el' instead of `cl.el', but we no longer
bother to encourage loading `cl.el' at compile time to pick
up macros.

But no one should even need to load `cl-lib.el' (or `cl.el'
at compile time).  Emacs should itself have `case', prominently
and first class.

And it is fine if the Emacs `case' is also the Common Lisp
`case', or close to it.  The same should be true elsewhere:
Emacs appropriating Common Lisp constructs is fine and dandy.

But yes, of course, as long as `case' (aka `cl-case') is not
*exactly* the same as the Common Lisp `case' the two should
remain aliased, and any differences from Common Lisp should
be documented.

But users should first and foremost see `case', not `cl-case'.
It should be as prominent as `cond' and `if', `when', and
`unless' - ALL of which also exist in Common Lisp.  We don't
call them `cl-cond', `cl-if', `cl-when', and `cl-unless'!
And rightfully so.

Preload the `cl-case' definition in Emacs, call it `case',
and alias `cl-case' to it.

There is no reason to force or encourage people to use
`cl-case' instead of `case'.  The opposite is true - they
should be encouraged to use `case', not `cl-case'.  Emacs
deserves `case', and it has `case' - you just have to know
about jumping through a hoop to reveal it.

[We even had the extreme position a few years ago that an
eager-beaver mmaintainer forced names like `cl-caddr' on
Emacs.  Fortunately, that craziness was eventually rescinded.

http://lists.gnu.org/archive/html/emacs-devel/2016-02/msg01394.html
]

_______

On a related subject, `pcase', and especially its derivatives,
should be renamed.  These things are not (are no longer) about
a kind of `case'.

Their names should share a common root, yes.  But it should not
be just `p', and it should definitely not be `case'.  Their
names should have a root that suggests pattern matching and, at
least in some cases, binding/destructuring.  (Start a contest to
find a good root name. ;-))

> I did ‘s/case/cl-&/g’ in commit 468e82790f1

Too bad.  That's a step backward, not forward, IMHO.
 
> Many computer languages have an intrinsic case-ish construct, so
> it was a bit surprising for me to learn that ‘case’ in Emacs
> Lisp is actually ‘cl-case’, which has a second-class citizen
> feel.

Exactly.  And there is no need for that second-class status.
And it is _new_.  In the past everyone just used `case' in
Emacs.  And they still can, the same way, by loading `cl.el'
at byte-compile time, to pick up the macro.  They should not
need to do that.

> In using ‘cl-case’ as one of the conceptual parents of ‘pcase’, 

But it's not, really.  Maybe "parent" in the sense of design
ancestor, but not "parent" in the sense of current derivation
or resemblance.

> - Build on "expectatious programmer" mindset.  Programmers new
>   to Emacs Lisp might feel that same surprise i felt and do what
>   i did: reach for ‘cl-case’ immediately, making it a habit to
>   such an extent as to consider it intrinsic (thus, familiar).

Yes.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Drew Adams <drew.adams@oracle.com>
() Sat, 26 May 2018 08:26:02 -0700 (PDT)

   >    The manual should refer to `cl-case', not `case'.

   I disagree.  My (unsolicited) 2 cents:

   [why ‘case’ over ‘cl-case’]

I agree, in the long term.  However, for the scope of this bug,
it's not a crucial problem to resolve.  If the fundamental
change you seek is to transition ‘case’ into Emacs Lisp "core"
(not requiring loading cl.el etc), that's something that we can
discuss in -devel and when it occurs, the docs can be updated
accordingly.

   [We even had the extreme position a few years ago that an
   eager-beaver mmaintainer forced names like `cl-caddr' on
   Emacs.  Fortunately, that craziness was eventually rescinded.

   http://lists.gnu.org/archive/html/emacs-devel/2016-02/msg01394.html
   ]

Wow.

   On a related subject [naming of `pcase' et al].

This interesting topic is better discussed in -devel, i think.

   > In using ‘cl-case’ as one of the conceptual parents of
   > ‘pcase’,

   But it's not, really.  Maybe "parent" in the sense of design
   ancestor, but not "parent" in the sense of current derivation
   or resemblance.

Well, "design ancestor" is what i meant by "conceptual parent".
However, i did not dare use that because i myself did not design
‘pcase’.  I think "conceptual parent" accurately describes how
someone on the outside might look at things.  Pure projection on
the surface, yes, but a bit of empathy in the background, too.

Anyway, thanks to everyone who gave feedback.  With your help, i
think we are now at a good point to merge (and close the bug).

I have squashed the two following commits that pertained to
"Overhaul pcase documentation" into that one and left "Mention
pcase as a fifth conditional form" as the last one on branch
‘fix/bug-31311-pcase-doc-squash’.  I deleted and repushed that
branch; its HEAD commit is now 3a0c48aac9a.

I'd like to merge that branch into ‘emacs-26’ immediately.  Any
objections?

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)               ; (2018) Software Libero
   (pcase (context query)               ;       = Dissenso Etico
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Thien-Thi Nguyen <ttn@gnu.org>
() Sun, 27 May 2018 10:22:49 +0200

   I deleted and repushed ‘fix/bug-31311-pcase-doc-squash’;
   its HEAD commit is now 3a0c48aac9a.

Update: rebased onto ‘emacs-26’, HEAD commit now cead47ab951f.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)               ; (2018) Software Libero
   (pcase (context query)               ;       = Dissenso Etico
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

> >    The manual should refer to `cl-case', not `case'.
>    I disagree.  My (unsolicited) 2 cents: ...
> 
> I agree, in the long term.  However, for the scope of
> this bug, it's not a crucial problem to resolve.

Right.  This bug is not about that.  

> If the fundamental change you seek is to transition
> ‘case’ into Emacs Lisp "core" (not requiring loading
> cl.el etc), 

Yes - let Emacs embrace `case' as one of its own.

But I put it differently.  It's not about where the
definition lives (in which file), except in so far
as one or the other place requires users to jump
through more hoops.

And no one has ever had to (and still does not need
to) load cl.el at runtime, to get `case'.  It's a
macro, so it's enough to load it at byte-compile time.

Still, it would be better if even that were not needed.

> that's something that we can discuss in -devel

It's been raised, IIRC, but didn't go anywhere.

> and when it occurs, the docs can be updated accordingly.

Agreed.  I just jumped in here, OT, in response to
this general statement:

  The manual should refer to `cl-case', not `case'.

>    On a related subject [naming of `pcase' et al].
> 
> This interesting topic is better discussed in -devel, i think.

Yes.

> i think we are now at a good point to merge (and close the bug).

I haven't looked at the resulting doc, but I'm sure it's
a great improvement.  Thanks for all your work on this.

bug#31311: 27.0; doc of `pcase'

[Noam Postavsky]

Thien-Thi Nguyen <ttn@gnu.org> writes:

> I'd like to merge that branch into ‘emacs-26’ immediately.  Any
> objections?

Nothing more from me, except a minor note about the commit message
formatting:

    * doc/lispref/elisp.texi (Top) (detailmenu)
    [Control Structures]: Add "Pattern-Matching Conditional".
    [Conditionals]: Delete section.

Square brackets are for conditional changes [1], I think you want round
parens or possibly angle brackets [2].

[1]: https://www.gnu.org/prep/standards/html_node/Conditional-Changes.html
[2]: https://www.gnu.org/prep/standards/html_node/Indicating-the-Part-Changed.html

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Thien-Thi Nguyen <ttn@gnu.org>
> Date: Sun, 27 May 2018 10:22:49 +0200
> Cc: 31311@debbugs.gnu.org
> 
> I'd like to merge that branch into ‘emacs-26’ immediately.  Any
> objections?

The tarball for 26.1 was already made, and was tested yesterday.  I
understand it will be uploaded tomorrow.

Is this deemed important enough to delay the release and ask Nicolas
to re-tar the release after updating from Git?  If not, I guess this
merge will have to wait for a couple of days and will make it into
26.2.

I'd like to take this opportunity to thank everybody who participated
in this effort.

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> Date: Sun, 27 May 2018 19:16:40 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: Nicolas Petton <nicolas@petton.fr>, 31311@debbugs.gnu.org
> 
> > From: Thien-Thi Nguyen <ttn@gnu.org>
> > Date: Sun, 27 May 2018 10:22:49 +0200
> > Cc: 31311@debbugs.gnu.org
> > 
> > I'd like to merge that branch into ‘emacs-26’ immediately.  Any
> > objections?
> 
> The tarball for 26.1 was already made, and was tested yesterday.  I
> understand it will be uploaded tomorrow.
> 
> Is this deemed important enough to delay the release and ask Nicolas
> to re-tar the release after updating from Git?  If not, I guess this
> merge will have to wait for a couple of days and will make it into
> 26.2.

And I see that you already pushed without waiting for a response to
your question :-(

Nicolas, what does this mean for the tarball you prepared?  You didn't
yet push the 26.1 tag, so do we revert the commit, or re-do the
tarball, or do something else?

bug#31311: 27.0; doc of `pcase'

[Andreas Schwab]

On Mai 27 2018, Eli Zaretskii <eliz@gnu.org> wrote:

> Nicolas, what does this mean for the tarball you prepared?  You didn't
> yet push the 26.1 tag, so do we revert the commit, or re-do the
> tarball, or do something else?

Don't move the tag.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: ttn@gnu.org,  nicolas@petton.fr,  31311@debbugs.gnu.org
> Date: Sun, 27 May 2018 18:32:49 +0200
> 
> On Mai 27 2018, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Nicolas, what does this mean for the tarball you prepared?  You didn't
> > yet push the 26.1 tag, so do we revert the commit, or re-do the
> > tarball, or do something else?
> 
> Don't move the tag.

Sorry, I don't follow.  The 26.1 tag is not yet there, AFAICS.

bug#31311: 27.0; doc of `pcase'

[Andreas Schwab]

On Mai 27 2018, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: ttn@gnu.org,  nicolas@petton.fr,  31311@debbugs.gnu.org
>> Date: Sun, 27 May 2018 18:32:49 +0200
>> 
>> On Mai 27 2018, Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>> > Nicolas, what does this mean for the tarball you prepared?  You didn't
>> > yet push the 26.1 tag, so do we revert the commit, or re-do the
>> > tarball, or do something else?
>> 
>> Don't move the tag.
>
> Sorry, I don't follow.  The 26.1 tag is not yet there, AFAICS.

The tag should match the tarball contents, even if not yet published.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Eli Zaretskii <eliz@gnu.org>
() Sun, 27 May 2018 19:26:04 +0300

   And I see that you already pushed without waiting for a
   response to your question :-(

No worries if it misses 26.1 and goes into 26.2.  I.e., "don't
move the tag" is fine by me.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)               ; (2018) Software Libero
   (pcase (context query)               ;       = Dissenso Etico
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Nicolas Petton]

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

Eli Zaretskii <eliz@gnu.org> writes:

> And I see that you already pushed without waiting for a response to
> your question :-(
>
> Nicolas, what does this mean for the tarball you prepared?  You didn't
> yet push the 26.1 tag, so do we revert the commit, or re-do the
> tarball, or do something else?

The commit the release tarball points to is pushed, so I can still push
the the tag, which whill be one commit behind in the emacs-26 branch.

I can also rebuild the tarball, it's up to you.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#31311: 27.0; doc of `pcase'

[Nicolas Petton]

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

Nicolas Petton <nicolas@petton.fr> writes:

> The commit the release tarball points to is pushed, so I can still push
> the the tag, which whill be one commit behind in the emacs-26 branch.
>
> I can also rebuild the tarball, it's up to you.

However I set the release date to today in the tarball, so if we decide
to release the current tarball, it has to be done today.

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

On May 28, 2018 10:25:03 AM GMT+03:00, Nicolas Petton <nicolas@petton.fr> wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > And I see that you already pushed without waiting for a response to
> > your question :-(
> >
> > Nicolas, what does this mean for the tarball you prepared?  You
> didn't
> > yet push the 26.1 tag, so do we revert the commit, or re-do the
> > tarball, or do something else?
> 
> The commit the release tarball points to is pushed, so I can still
> push
> the the tag, which whill be one commit behind in the emacs-26 branch.
> 
> I can also rebuild the tarball, it's up to you.
> 
> Cheers,
> Nico

If there are no complications with the tag, let's release the tarball you have.

Thanks.

bug#31311: 27.0; doc of `pcase'

[Nicolas Petton]

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

Eli Zaretskii <eliz@gnu.org> writes:

> If there are no complications with the tag, let's release the tarball
> you have.

Ok, I'll push and announce it this afternoon!

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#31311: 27.0; doc of `pcase'

[Noam Postavsky]

Thien-Thi Nguyen <ttn@gnu.org> writes:

> * c6e9b975f  add some Issue comments; nfc
>
>   This first set of ponderables revolves around the precise
>   semantics of the SYMBOL let-binding feature.

> +@c        - This is a design decision.  Document the limitation
> +@c          (i.e., BOOLEAN-EXPRESSION SHOULD NOT be side-effecting).
> +@c        - This is an unforseen case; behavior is based on the
> +@c          implementation (‘cond’ + redundant ‘let*’ expansion)
> +@c          and is considered {undefined, wontfix, futurefix}.
> +@c          Leave undocumented for now.

I would say the behaviour of pcase when guard expressions have
side-effects is not defined, so the documentation should advise against
having side-effects in guard expressions.  By the way, the current
implementation causes a compiler warning if the variable is not used in
the body (Bug#16771), so it may well be changed if someone can figure
out how.

> Additionally, related to SYMBOL binding, i see (in *scratch*):
>
>   a.  (pcase t   ('t   'true))  => true
>   b.  (pcase nil ('nil 'false)) => false
>   c.  (pcase t   (t    'true))  => true

Yes, it looks like t is intentionally equivalent to _, cf pcase.el:

    (defconst pcase--dontcare-upats '(t _ pcase--dontcare))

Hence also:

    (pcase 42   (t    'true)) => true

>   d.  (pcase nil (nil  'false)) |= ERROR: Unknown pattern ‘nil’

In Emacs 24.5, it gives "Lisp error: (setting-constant nil)", so we can
see that this case now gives a more specific error.

> For anyone just joining, these commits are on the branch
> ‘fix/bug-31311-pcase-doc’ (off of ‘emacs-26’).  Feedback on
> anything there is most welcome!

In the docstring, you have

    The two catagories of patterns are logical and structural.

    A logical pattern expands, in essence, to a predicate function
    to call on EXPVAL. [...]

    On the other hand, a structural pattern specifies a template,
    and allows for elements of that template to be either constants
    or sub-patterns (of either category).

You grouped SYMBOL into the "logical patterns", but I don't think it
really fits into either of those.  Better to make it separate, perhaps?

I'm also not so sure it's a useful division.  At least, they shouldn't
be presented as equal categories.  The "structural" is more a short form
for particular kinds of "logical" patterns.  For example, `(1 . 2) is
equivalent to

    (and (pred consp)
         (app car 1)
         (app cdr 2))

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Noam Postavsky <npostavs@gmail.com>
() Sat, 12 May 2018 09:56:21 -0400

   I would say the behaviour of pcase when guard expressions
   have side-effects is not defined, so the documentation should
   advise against having side-effects in guard expressions.

I've done this (commit 977bd10f5e70) and extended the advice to
other places where evaluation occurs.

   [‘t’ and ‘nil’ as SYMBOL patterns]

Thanks for confirming.  I've addressed this in another post.

   > For anyone just joining, these commits are on the branch
   > ‘fix/bug-31311-pcase-doc’ (off of ‘emacs-26’).  Feedback on
   > anything there is most welcome!

   In the docstring, you have

       [...] two catagories [...] logical and structural.

   You grouped SYMBOL into the "logical patterns", but I don't
   think it really fits into either of those.  Better to make it
   separate, perhaps?

In the end, reverted this distinction, moved `QPAT material to
the ‘(pcase-defmacro \` (qpat) ...)’ form, and added a comment
in ‘pcase--make-docstring’ proposing somehow to arrange for
`QPAT docstring to be presented immediately after ‘pcase’.  More
on this in a post to emacs-devel (which is still queued for
delivery on my side at this moment...).

   I'm also not so sure it's a useful division.  At least, they
   shouldn't be presented as equal categories.  The "structural"
   is more a short form for particular kinds of "logical"
   patterns.  For example, `(1 . 2) is equivalent to

       (and (pred consp)
            (app car 1)
            (app cdr 2))

True.  However, i'm trying to avoid using `QPAT in either
docstrings or examples prior to its introduction (including
table of possible QPAT forms).  Kind of an informational
layering.  This snippet was very useful, anyway; i adapted and
expanded on it for the example illustrating "2nd SYMBOL becomes
‘eq’ test".  Thanks for posting it.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Hi,

one thing I want to improve in the current docstring is the following
detail:

FUN in `pred' and `app' can take one of the forms:
  SYMBOL  or  (lambda ARGS BODY)
     call it with one argument
[...]

This doesn't tell which binding of SYMBOL is used (value vs. function
binding), but I think it should.  AFAICT only the function binding
matters, so we just could say "FBOUND-SYMBOL" instead of "SYMBOL".  Or
something better?

Thanks, Michael.

bug#31311: 27.0; doc of `pcase'

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@gmail.com>
> Cc: Michael Heerdegen <michael_heerdegen@web.de>,  31311@debbugs.gnu.org,  drew.adams@oracle.com
> Date: Mon, 30 Apr 2018 07:20:23 -0400
> 
> It's relevant if potential contributors turn away/tune out/ignore
> the whole thing due to the unpleasant discussion.

Right.  QED.

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

1. Even just the first line of the doc string seems wrong:

 "Evaluate EXP and attempt to match it against structural
  patterns."

That should be something more like this:

 "Evaluate EXP and match _its value_ against structural
  patterns."

It does not match EXP ("it") against the patterns.  It
matches the result of evaluating EXP (against the patterns).

(And are the patterns necessarily "structural"?)

I'd say: "Evaluate EXP and match its value against CASES."

Even better would be to call the second arg PATTERNS.
 "Evaluate EXP and match its value against PATTERNS."

This is especially so because after that we talk only
about "patterns", never referring to CASES.


2. CODE is undefined (not specified).  We say only this:

 "The CODE expression corresponding to the matching
  pattern determines the return value."

So CODE must be an expression (Aha!).  That should be
said up front.

But what does "_corresponding_ to the matching pattern"
mean?

3. And just _how_ does the CODE expression that
corresponds to the matching pattern "determine the
return value"?

The rest of the doc string never mentions the return
value at all, AFAICT.  It mentions neither "return"
nor "value".

[There is one occurrence of "return", but it is about
something different: "(pred FUN) matches if FUN applied
to the object returns non-nil."]

The rest of the doc string (before the extra, unclear
generated text) just talks about the available standard
patterns and how matching is done - nothing about what
value is returned by the code resulting from `pcase'
macro expansion.

Even if we can't get someone super-knowledgable about
`pcase' to work on the doc, perhaps some rudimentary
improvement can be made.

bug#31311: 27.0; doc of `pcase'

[Noam Postavsky]

Drew Adams <drew.adams@oracle.com> writes:

> 2. CODE is undefined (not specified).  We say only this:
>
>  "The CODE expression corresponding to the matching
>   pattern determines the return value."

I guess you missed the second line of the docstring?

    CASES is a list of elements of the form (PATTERN CODE...).

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

> > 2. CODE is undefined (not specified).  We say only this:
> >
> >  "The CODE expression corresponding to the matching
> >   pattern determines the return value."
> 
> I guess you missed the second line of the docstring?
>     CASES is a list of elements of the form (PATTERN CODE...).

No, not at all.  That's where CODE is "bound" in the
description, but not where it is defined.  I'm looking
for a description of CODE.

The most one can say, apparently, is that CODE is an
expression.  What purpose the sequence of CODE expressions
serves - what is done with them (or with any of them) -
isn't clear.

It talks about "the CODE expression corresponding to
the matching pattern".  Which CODE expression of the
sequence "CODE..." is "_the_ CODE expression"?  What
does it mean for "the CODE expression" to _correspond_
to the matching pattern?  (That should presumably be
"the matching PATTERN, BTW.)

Sorry, but it's just not clear.

bug#31311: 27.0; doc of `pcase'

[Michael Heerdegen]

Hi Drew,

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.

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

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

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

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


Michael.

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

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.

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Drew Adams <drew.adams@oracle.com>
() Sun, 29 Apr 2018 19:47:08 -0700 (PDT)

   > > 2. Don't use ATOM [...]
   > 
   > "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.

Since "atom" is a shorthand for this set of types in an @item
context, another solution is to list the types explicitly:

 @item @var{integer}
 @itemx @var{string}
 @itemx @var{keyword}

 These types are self-quoting [...]

In this way, we achieve grouping through presentation and not
through coining (possibly confusing) terms.  This makes the text
longer vertically, but flattens the conceptual model (for this
particular grouping of types) for the reader, who can mentally
coin terms on their own, like "!@#$%-complicated-sh*t".  :-D

   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.

Same technique applicable, here.  Generally, it's good to use
abstractions in code (DRY principle), but for documentation, a
flatter landscape w/ simple landmarks is easier to navigate.

This is one of the sadnesses i have come to realize as a
programmer trying to communicate w/ non-programmers.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Thien-Thi Nguyen]

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


() Drew Adams <drew.adams@oracle.com>
() Sun, 29 Apr 2018 09:03:32 -0700 (PDT)

   [points 1. through 15.]

I have just pushed branches:

 fix/bug-31311-pcase-docs
 fix/bug-31311-pcase-docs-squash

that address these points (and others raised during discussion).
The latter branch is the former, squashed, and additionally
rebased off ‘emacs-26’.  Here are the four commits, briefly:

 25d3e50  Ensure pcase doc shows `QPAT first among extensions
 62b1cef  Introduce EXPVAL for pcase, pcase-defmacro docstrings
 a7bf9ce  Use EXPVAL in docstrings of patterns defined using pcase-defmacro
 2df20b3  Overhaul pcase documentation

(The first is mostly independent of this bug, and so its commit
message lacks the "(Bug#31311)" found in the others'.)

   16. All of the following text in the _doc string_ is pretty
       much incomprehensible, to me.

       [...]

Yeah, i was also flummoxed for a bit.  The third commit touches
many of these docstrings, the ones that i could decipher.  Its
changes do not improve the meaning too much, i'm afraid (but at
least, now they are more consistent in one syntax aspect).

       -- `QPAT

This particular extension is actually "built-in" in spirit.  The
first commit takes care to support that impression (somewhat).
The other commits improve its docstring, i believe.

The -squash branch is ready for merge.  I'll wait a few days for
comments and do the merge, unless someone beats me to it.

Once merged, i'd like to reserve the right to delete those
branches (personal closure :-D), if the maintainers don't mind.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)
   (pcase (context query)
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

bug#31311: 27.0; doc of `pcase'

[Lars Ingebrigtsen]

This bug report pointed out a lot of things that could be improved in
the pcase documentation.  The documentation was largely rewritten a bit
later, and skimming this thread, it seems like the major objections were
all handled, so I'm closing this bug report.

If there are any remaining issues, opening a new bug report for those
issues would be the way forward.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#31311: 27.0; doc of `pcase'

[Drew Adams]

> This bug report pointed out a lot of things that 
> could be improved in the pcase documentation.

Of course.  Glad you recognize that there are so
many things wrong with that doc - lots of things
that _need_ improving.

And you're welcome for thanking me for having
reported so many of them specifically.

> The documentation was largely rewritten a bit
> later, and skimming this thread, it seems like
> the major objections were all handled,

The point in specifying particular problems, as
opposed to just tossing out "major objections",
is to help get the doc fixed in those many ways.

> so I'm closing this bug report.

Does the doc now respond to all of the cited needs
and deficiencies?  If not, then the bug isn't fixed.

If you don't keep it open so those can be fixed,
then ... "wont fix".

> If there are any remaining issues, opening a new
> bug report for those issues would be the way forward.

Then do that.  If the bug isn't fixed then it
isn't fixed.  A new bug specifying what remains
to be fixed is a fine idea.  Please go for it.
The way forward isn't just to ignore them and
call the bug fixed.