Re: [Emacs-diffs] scratch/bug19328/custom-prompt-customize-unsaved-options-rc2 e9e8f01: Provide custom-prompt-customize-unsaved-options.

Re: [Emacs-diffs] scratch/bug19328/custom-prompt-customize-unsaved-options-rc2 e9e8f01: Provide custom-prompt-customize-unsaved-options.

[Stefan Monnier]

Looks good, with the following nitpicks

>     * lisp/cus-edit.el (custom-prompt-customize-unsaved-options): Add a
>     mechanism for prompting user about unsaved customizations.

These two lines mean "change the custom-prompt-customize-unsaved-options
by adding a mechanism for ...", which is not what the patch does.
Instead, they should just say

  * lisp/cus-edit.el (custom-prompt-customize-unsaved-options): New function.

Additionally, this text fails to mention custom-unsaved-options or the
fact that customize-unsaved has been modified, so I'd write:

  * lisp/cus-edit.el (custom-unsaved-options): New function, extracted
  from customize-unsaved.
  (custom-unsaved): Use it.
  (custom-prompt-customize-unsaved-options): New function.

Feel free to install it, thank you,


        Stefan

Re: [Emacs-diffs] scratch/bug19328/custom-prompt-customize-unsaved-options-rc2 e9e8f01: Provide custom-prompt-customize-unsaved-options.

[Ted Zlatanov]

On Sun, 14 Dec 2014 09:36:00 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

SM> Looks good, with the following nitpicks
>> * lisp/cus-edit.el (custom-prompt-customize-unsaved-options): Add a
>> mechanism for prompting user about unsaved customizations.

SM> These two lines mean "change the custom-prompt-customize-unsaved-options
SM> by adding a mechanism for ...", which is not what the patch does.
SM> Instead, they should just say

SM>   * lisp/cus-edit.el (custom-prompt-customize-unsaved-options): New function.

SM> Additionally, this text fails to mention custom-unsaved-options or the
SM> fact that customize-unsaved has been modified, so I'd write:

SM>   * lisp/cus-edit.el (custom-unsaved-options): New function, extracted
SM>   from customize-unsaved.
SM>   (custom-unsaved): Use it.
SM>   (custom-prompt-customize-unsaved-options): New function.

OK, I installed it earlier and have now added a commit to just fix that
ChangeLog entry.

FWIW, I think your version does not explain what the change provides,
only that it was made (it basically restates the diff).  I don't know if
that's what you intended.

Ted

Re: [Emacs-diffs] scratch/bug19328/custom-prompt-customize-unsaved-options-rc2 e9e8f01: Provide custom-prompt-customize-unsaved-options.

[Stefan Monnier]

> FWIW, I think your version does not explain what the change provides,
> only that it was made (it basically restates the diff).  I don't know if
> that's what you intended.

The standard ChangeLog format is meant indeed to describe what the diff
is intended to change in the code.  The explanation of why we might want
this change can be placed in the code (best choice when possible), or
otherwise it can be placed just before the ChangeLog-formatted text
(e.g. in the Summary line or right below it).


        Stefan

Re: [Emacs-diffs] scratch/bug19328/custom-prompt-customize-unsaved-options-rc2 e9e8f01: Provide custom-prompt-customize-unsaved-options.

[Ted Zlatanov]

On Sun, 14 Dec 2014 18:36:18 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> FWIW, I think your version does not explain what the change provides,
>> only that it was made (it basically restates the diff).  I don't know if
>> that's what you intended.

SM> The standard ChangeLog format is meant indeed to describe what the diff
SM> is intended to change in the code.  The explanation of why we might want
SM> this change can be placed in the code (best choice when possible), or
SM> otherwise it can be placed just before the ChangeLog-formatted text
SM> (e.g. in the Summary line or right below it).

All right, I'll keep that in mind.  Thanks for explaining.

Ted