bug#30205: 27.0.50; Minor mode commands enable the minor mode even if the body fails

[Lars Ingebrigtsen]

Philipp Stephani <p.stephani2@gmail.com> writes:

> In *scratch*:
>
> (define-minor-mode foo-mode nil nil nil nil (error "what"))
>
> C-h v foo-mode shows that foo-mode is nil, as it should be.
> But after M-x foo-mode, it is t, even though the mode command failed.
> This can be confusing for mode commands that can conditionally fail,
> e.g. depending on some external property.

Hm...  OK, this is the function run when saying M-x foo-mode:

       (defun ,modefun (&optional arg ,@extra-args)
         ,(easy-mmode--mode-docstring doc pretty-name keymap-sym)
         ;; Use `toggle' rather than (if ,mode 0 1) so that using
         ;; repeat-command still does the toggling correctly.
         (interactive (list (or current-prefix-arg 'toggle)))
         (let ((,last-message (current-message)))
           (,@setter
            (if (eq arg 'toggle)
                (not ,getter)
              ;; A nil argument also means ON now.
              (> (prefix-numeric-value arg) 0)))
           ,@body
           ;; The on/off hooks are here for backward compatibility only.
           (run-hooks ',hook (if ,getter ',hook-on ',hook-off))

So `setter' is the thing that sets the mode variable, and then body is
run.

I agree with you that it would be better that the mode variable remains
unchanged if `body' fails.  But I `body' is likely to need to have that
set to work.  We could roll back the value to the previous value on
errors?

Hm.  On the other hand, if `body' has done most of the stuff it needs to
do and fails "late" in the process, then the mode will be in effect even
if it failed, and in that case it would be wrong to roll back.

So I don't know.  Does anybody have an opinion?

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