bug#19328: [PATCH] Add mechanism to prompt about unsaved customizations

[Cameron Desautels]

Got it.  Thank you for the clarification, I think I see where our
thinking has diverged: `customize-unsaved' is the existing mechanism
to pop up a customize buffer displaying unsaved customization options.
I've put a thin layer around this which provides a yes / no prompt to
the user before (conditionally) invoking `customized-unsaved'. This is
in the general spirit of hooks that run when killing Emacs ("Modified
buffers exist", "Active processes exist", etc.).

> If you want a named function and no error handling, then, well, try
> `customize-unsaved'.

`customize-unsaved' is not suitable for use in
`kill-emacs-query-functions` because it signals an error if nothing
has been customized, meaning that it will block the user from quitting
Emacs if there are no unsaved customizations, and it will invoke the
customization mechanism there are any.  Ergo, they can never quit.
(Drew: I'm sure you're aware of this given the way you've wrapped the
call to `customize-unsaved', but I'm spelling it out for the benefit
of others.)

That said, the lambda you've added to `kill-emacs-query-functions'
doesn't function properly either (at least not on my machine).  It
returns `t' in the no-unsaved-customizations case (after catching the
error thrown my `customize-unsaved'), and it returns a (truthy)
string, courtesy of `customize-unsaved', in the
have-unsaved-custoizations case.  Truthy values returned by the
functions in `kill-emacs-query-functions' mean "continue with quitting
Emacs", ergo the user is never prompted about unsaved customizations.

Of course you can fix this easily by making sure to return nil in the
happy path, but you're working up to exactly what my patch does...

The one behavioral difference between the two is how we prompt: in the
patched version the user is prompted in the minibuffer: "Some
customized options have not been saved; Examine?", and shown a
customization buffer only if they say yes.

In yours, the customization buffer is automatically shown, without
explanation, and attempts to quit the editor will continue to fail
until the user finds either the `Revert...` or the `Apply and Save`
button.

I believe the patched version is a superior user experience.  Have I
managed to convince you? :)

-- 
Cameron Desautels <camdez@gmail.com>




> On 2014-12-09, at 10:24 AM, Drew Adams <drew.adams@oracle.com> wrote:
> 
>>> Why is this in etc/TODO?
>> 
>> If you're asking me in particular, I don't know. I just found it
>> there and thought I'd help out.
> 
> Thanks for helping out!  The question is for Emacs Dev in general,
> i.e., for this bug thread.
> 
>>> This already exists, no?  I've been using this for years:
>>> 
>>> (add-hook 'kill-emacs-query-functions
>>>         (lambda ()
>>>           (condition-case nil
>>>               (customize-unsaved)
>>>             (error t))))
>> 
>> A named function is probably more suitable for an end user than an
>> arbitrary lambda, no?  Plus using error handling for control flow is
>> generally not considered a best practice, especially when it's as
>> broad in scope as `(condition-case nil ...)`.
> 
> That's what *I* use.  I'm not saying that everyone should use it.
> My .emacs is used for multiple Emacs versions.  You might prefer
> `ignore-errors', which does not exist in older versions.  Or you
> might prefer not to ignore errors...
> 
> If you want a named function and no error handling, then, well,
> try `customize-unsaved'.
> 
> My point was that `customized-unsaved' already exists.  It is
> precisely a "mechanism to prompt about unsaved customizations".
> Unless I'm missing something.  AFAICT, it is exactly what is
> described in that TODO item:
> 
>   A function to check for customizable options that have been
>   set but not saved, and ask the user whether to save them.
>   This could go in kill-emacs-query-functions, to remind people
>   to save their changes. If the user says yes, show them
>   in a Custom buffer using customize-customized.