Re: [Emacs-diffs] master 39355bc: Revert "* lisp/subr.el (when): Use `macroexp-progn'"

Re: [Emacs-diffs] master 39355bc: Revert "* lisp/subr.el (when): Use `macroexp-progn'"

[Ken Brown]

On 11/4/2015 9:22 AM, Artur Malabarba wrote:
> branch: master
> commit 39355bc045874874eb83cecdf2e2e04c3b81d3f7
> Author: Artur Malabarba <bruce.connor.am@gmail.com>
> Commit: Artur Malabarba <bruce.connor.am@gmail.com>
>
>      Revert "* lisp/subr.el (when): Use `macroexp-progn'"
>
>      This reverts commit 8e843831eaf271801836b7a3e4dd3b4fb0bb72b8.
>      It breaks bootstrapping (duh).
> ---
>   lisp/subr.el |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lisp/subr.el b/lisp/subr.el
> index 91647a6..ea926ae 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -179,7 +179,7 @@ value of last one, or nil if there are none.
>
>   \(fn COND BODY...)"
>     (declare (indent 1) (debug t))
> -  (list 'if cond (macroexp-progn body)))
> +  (list 'if cond (cons 'progn body)))
>
>   (defmacro unless (cond &rest body)
>     "If COND yields nil, do BODY, else return nil.

You only reverted part of the commit, leaving the new test in place. 
Was that intentional?

Ken

Re: [Emacs-diffs] master 39355bc: Revert "* lisp/subr.el (when): Use `macroexp-progn'"

[Artur Malabarba]

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

> You only reverted part of the commit, leaving the new test in place. Was
that intentional?

Yes. No reason to delete the test. :-)

[-- Attachment #2: Type: text/html, Size: 169 bytes --]

Re: [Emacs-diffs] master 39355bc: Revert "* lisp/subr.el (when): Use `macroexp-progn'"

[Ken Brown]

On 11/5/2015 5:17 PM, Artur Malabarba wrote:
>  > You only reverted part of the commit, leaving the new test in place.
> Was that intentional?
>
> Yes. No reason to delete the test. :-)

This part fails:

   (should (equal (macroexpand-all '(when a b))
                  '(if a b)))

It should be

   (should (equal (macroexpand-all '(when a b))
                  '(if a (progn b))))

Ken

Re: [Emacs-diffs] master 39355bc: Revert "* lisp/subr.el (when): Use `macroexp-progn'"

[Artur Malabarba]

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

On 5 Nov 2015 10:44 pm, "Ken Brown" <kbrown@cornell.edu> wrote:
>
> On 11/5/2015 5:17 PM, Artur Malabarba wrote:
>>
>>  > You only reverted part of the commit, leaving the new test in place.
>> Was that intentional?
>>
>> Yes. No reason to delete the test. :-)
>
>
> This part fails:
>
>   (should (equal (macroexpand-all '(when a b))
>                  '(if a b)))
>
> It should be
>
>   (should (equal (macroexpand-all '(when a b))
>                  '(if a (progn b))))

Yep. Thanks for catching that.

[-- Attachment #2: Type: text/html, Size: 802 bytes --]