bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Chunyang Xu]

It looks like, to me, the structure of the BODY arg of these two should
be the same (accepting a list froms).

(pcase-let ((a 1))
  (incf a)
  a)
     => 2

(pcase-let* ((a 1))
  (incf a)
  a)
     => 2

but the docstrings are using the different words.

(pcase-let BINDINGS &rest BODY)

Like `let' but where you can use `pcase' patterns for bindings.
BODY should be a list of expressions, and BINDINGS should be a list of bindings
               ^^^^^^^^^^^^^^^^^^^^^
of the form (PAT EXP).

(pcase-let* BINDINGS &rest BODY)

Like `let*' but where you can use `pcase' patterns for bindings.
BODY should be an expression, and BINDINGS should be a list of bindings
               ^^^^^^^^^^^^^
of the form (PAT EXP).

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Robert Cochran]

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

Chunyang Xu <xuchunyang.me@gmail.com> writes:

> It looks like, to me, the structure of the BODY arg of these two should
> be the same (accepting a list froms).
>
> (pcase-let ((a 1))
>   (incf a)
>   a)
>      => 2
>
> (pcase-let* ((a 1))
>   (incf a)
>   a)
>      => 2

It appears to be so. Here's what I did:

(pcase-let* ((a 1)
	     (b 2))
  (message "%d" a)
  (message "%d" b))

and ended up with

1
2

in my *Messages* buffer.

> but the docstrings are using the different words.
>
> (pcase-let BINDINGS &rest BODY)
>
> Like `let' but where you can use `pcase' patterns for bindings.
> BODY should be a list of expressions, and BINDINGS should be a list of bindings
>                ^^^^^^^^^^^^^^^^^^^^^
> of the form (PAT EXP).
>
> (pcase-let* BINDINGS &rest BODY)
>
> Like `let*' but where you can use `pcase' patterns for bindings.
> BODY should be an expression, and BINDINGS should be a list of bindings
>                ^^^^^^^^^^^^^
> of the form (PAT EXP).

This patch changes the pcase-let* docstring to match the pcase-let
docstring, but IMO I don't think that the phrase 'a list of expressions'
is exactly the right term for this. That (to me) implies that we are
wrapping the whole body in a list, which you don't.

For example -

; What that phrase implies to me:
(pcase-let ((a 1))
  ((incf a)
   a))

But I have no idea how to better phrase this. For certain, though, they
ought to match because they both behave the same way.

-----


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix docstring of pcase-let* to accurately reflect what BODY can be --]
[-- Type: text/x-patch, Size: 1179 bytes --]

From c41219fcbbb01b5a219733ac54ad2cade438513b Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Wed, 14 Sep 2016 15:52:29 -0700
Subject: [PATCH] Fix docstring of pcase-let*

pcase-let*'s docstring could have been taken to mean that the BODY
parameter can only be a single expression, but it can be any numbers of
expressions.  Fix it to be more accurate.

* lisp/emacs-lisp/pcase.el (pcase-let*): Fix docstring to be more
accurate about what the BODY parameter can be.
---
 lisp/emacs-lisp/pcase.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 0b8dddf..2d61642 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -261,7 +261,7 @@ pcase--let*
 ;;;###autoload
 (defmacro pcase-let* (bindings &rest body)
   "Like `let*' but where you can use `pcase' patterns for bindings.
-BODY should be an expression, and BINDINGS should be a list of bindings
+BODY should be a list of expressions, and BINDINGS should be a list of bindings
 of the form (PAT EXP)."
   (declare (indent 1)
            (debug ((&rest (pcase-PAT &optional form)) body)))
-- 
2.7.4


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

-----

HTH,
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Michael Heerdegen]

Robert Cochran <robert-emacs@cochranmail.com> writes:

> This patch changes the pcase-let* docstring to match the pcase-let
> docstring, but IMO I don't think that the phrase 'a list of
> expressions' is exactly the right term for this. That (to me) implies
> that we are wrapping the whole body in a list, which you don't.

Isn't this just normal "Elisp speak" for a &rest parameter?  Sure, the
value of the parameter doesn't appear in the code (only "spliced in") -
but we use this wording all the time.

> diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
> index 0b8dddf..2d61642 100644
> --- a/lisp/emacs-lisp/pcase.el
> +++ b/lisp/emacs-lisp/pcase.el
> @@ -261,7 +261,7 @@ pcase--let*
>  ;;;###autoload
>  (defmacro pcase-let* (bindings &rest body)
>    "Like `let*' but where you can use `pcase' patterns for bindings.
> -BODY should be an expression, and BINDINGS should be a list of bindings
> +BODY should be a list of expressions, and BINDINGS should be a list of bindings

Looks ok to me.


Michael.

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Robert Cochran]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Robert Cochran <robert-emacs@cochranmail.com> writes:
>
>> This patch changes the pcase-let* docstring to match the pcase-let
>> docstring, but IMO I don't think that the phrase 'a list of
>> expressions' is exactly the right term for this. That (to me) implies
>> that we are wrapping the whole body in a list, which you don't.
>
> Isn't this just normal "Elisp speak" for a &rest parameter?  Sure, the
> value of the parameter doesn't appear in the code (only "spliced in") -
> but we use this wording all the time.

It may very well be. I haven't been around long enough to know. Now,
don't get me wrong, I *knew* what 'a list of expressions' had to mean in
context (it wouldn't have worked any other way), but my natural parsing
still evokes the wrong idea in my mind. If it's already established
tradition, then I have no qualms.

>> diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
>> index 0b8dddf..2d61642 100644
>> --- a/lisp/emacs-lisp/pcase.el
>> +++ b/lisp/emacs-lisp/pcase.el
>> @@ -261,7 +261,7 @@ pcase--let*
>>  ;;;###autoload
>>  (defmacro pcase-let* (bindings &rest body)
>>    "Like `let*' but where you can use `pcase' patterns for bindings.
>> -BODY should be an expression, and BINDINGS should be a list of bindings
>> +BODY should be a list of expressions, and BINDINGS should be a list of bindings
>
> Looks ok to me.

Thanks! Not that there's much to do wrong there. ;)

-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Drew Adams]

> > This patch changes the pcase-let* docstring to match the pcase-let
> > docstring, but IMO I don't think that the phrase 'a list of
> > expressions' is exactly the right term for this. That (to me) implies
> > that we are wrapping the whole body in a list, which you don't.
> 
> Isn't this just normal "Elisp speak" for a &rest parameter?  Sure, the
> value of the parameter doesn't appear in the code (only "spliced in") -
> but we use this wording all the time.

An &rest formal parameter corresponds to a list of actual arguments
in the calling sequence.

Consider (defun foo (&rest xs) (message "Args: %S" xs)).

At runtime, XS is the only argument that you can access by name -
the only argument whose value is bound to a variable.

And yes, its value at runtime, i.e., in any actual call, is a list.

On the other hand, not only (foo 1 2) but also (funcall #'foo 1 2)
treat 1 and 2 as actual arguments.  The list (1 2) does not appear
explicitly in these calling sequences.

So if you are talking about `foo's actual arguments for such a call
then you can say they are 1 and 2.  But if you are talking about the
value of the &rest argument XS for such a call then you must say
that the value is a list.

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Michael Heerdegen]

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

> So if you are talking about `foo's actual arguments for such a call
> then you can say they are 1 and 2.  But if you are talking about the
> value of the &rest argument XS for such a call then you must say that
> the value is a list.

Hah Drew!  Actually we are indeed defining the meaning of the actual
arguments, but we are describing the semantics of the value of the &rest
parameter to do so.  That's the reason why such descriptions can be a
bit confusing at first.

Michael.

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Michael Heerdegen]

Robert Cochran <robert-emacs@cochranmail.com> writes:

> This patch changes the pcase-let* docstring to match the pcase-let
> docstring [...]

Seems you did not yet install it to master.  I think the patch can be
applied.


Thanks,

Michael.

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Robert Cochran]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Seems you did not yet install it to master.  I think the patch can be
> applied.
>

I do not have commit access. Someone who does will need to do this.

TIA,
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Michael Heerdegen]

Robert Cochran <robert-emacs@cochranmail.com> writes:

> I do not have commit access. Someone who does will need to do this.

Don't you want to get commit access?


Michael.

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Robert Cochran]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Don't you want to get commit access?

Sure I would, but I figure that I don't have the required amount of
'known-ness' to bother requesting this access right now. I'd have
imagined I'd need to have a few code commits (AFAIK, only 1 commit of
mine in the master branch has anything other than documentation changes)
and be known to be generally sane.

I could be wrong about this; I just know that some people have commit
rights and others do not. I have no idea what the process is for
attaining said rights.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

bug#24362: 25.1.50; Inconsistent docstring between pcase-let and pcase-let*

[Stefan Kangas]

Chunyang Xu <xuchunyang.me@gmail.com> writes:

> It looks like, to me, the structure of the BODY arg of these two should
> be the same (accepting a list froms).
>
> (pcase-let ((a 1))
>   (incf a)
>   a)
>      => 2
>
> (pcase-let* ((a 1))
>   (incf a)
>   a)
>      => 2
>
> but the docstrings are using the different words.
>
> (pcase-let BINDINGS &rest BODY)
>
> Like `let' but where you can use `pcase' patterns for bindings.
> BODY should be a list of expressions, and BINDINGS should be a list of bindings
>                ^^^^^^^^^^^^^^^^^^^^^
> of the form (PAT EXP).
>
> (pcase-let* BINDINGS &rest BODY)
>
> Like `let*' but where you can use `pcase' patterns for bindings.
> BODY should be an expression, and BINDINGS should be a list of bindings
>                ^^^^^^^^^^^^^
> of the form (PAT EXP).

This has been changed on current master to:

    Like ‘let*’, but supports destructuring BINDINGS using ‘pcase’ patterns.
    As with ‘pcase-let’, BINDINGS are of the form (PATTERN EXP), but the
    EXP in each binding in BINDINGS can use the results of the destructuring
    bindings that precede it in BINDINGS’ order.
    
    Each EXP should match (i.e. be of compatible structure) to its
    respective PATTERN; a mismatch may signal an error or may go
    undetected, binding variables to arbitrary values, such as nil.

I'm therefore going to assume that the above is no longer an issue and
close this bug report.  If that is incorrect, please reopen.

Best regards,
Stefan Kangas