bug#38219: Error on leaving Ediff after killing vital buffer

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#38219: Error on leaving Ediff after killing vital buffer

Richard Copley-2
If you kill one of Ediff's vital buffers, then try to leave Ediff (by typing q in the control window), there is an error.

The error is:
ediff-visible-region: You have killed a vital Ediff buffer---you must leave Ediff now!

Recipe from 'emacs -Q' (Windows GUI build):

* Visit a file under version control with unstaged changes.
* [M-x ediff-revision RET RET RET RET]
* In the main frame, kill one of the buffers being diffed
* In the Ediff control window frame, type [q y]

Bisected to this commit:

a26a8cc1c85f29fb11209c16d53a8ae4e4ab7ced
Author: Juri Linkov <[hidden email]>
Date: Sun Nov 10 00:04:13 2019 +0200

'y-or-n-p' now uses the minibuffer to read 'y' or 'n' answer (bug#38076)
Reply | Threaded
Open this post in threaded view
|

bug#38219: Error on leaving Ediff after killing vital buffer

martin rudalics
 > If you kill one of Ediff's vital buffers, then try to leave Ediff (by
 > typing q in the control window), there is an error.
 >
 > The error is:
 > ediff-visible-region: You have killed a vital Ediff buffer---you must leave
 > Ediff now!

'y-or-n-p' mangles 'this-command'.  Something like the attached patch
should fix that.

martin

subr.el.diffs (882 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#38219: Error on leaving Ediff after killing vital buffer

Juri Linkov-2
> 'y-or-n-p' mangles 'this-command'.  Something like the attached patch
> should fix that.

Can the same be said about read-from-minibuffer?
Shouldn't read-from-minibuffer mangle 'this-command'?
What it some command wants to check if 'this-command'
is 'exit-minibuffer' afterwards?  Shouldn't this change better
to be localized to callers in ediff, instead of adding such hack?

> diff --git a/lisp/subr.el b/lisp/subr.el
> index eaec223585..68e25c96d9 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -2828,7 +2828,8 @@ y-or-n-p
>      (concat prompt
>      (if (or (zerop l) (eq ?\s (aref prompt (1- l))))
>   "" " ")
> -    (if dialog "" "(y or n) "))))))
> +    (if dialog "" "(y or n) ")))))
> +        (old-this-command this-command))
>      (cond
>       (noninteractive
>        (setq prompt (funcall padded prompt))
> @@ -2858,6 +2859,7 @@ y-or-n-p
>      (let ((ret (eq answer 'act)))
>        (unless noninteractive
>          (message "%s%c" prompt (if ret ?y ?n)))
> +      (setq this-command old-this-command)
>        ret)))
>  
>  



Reply | Threaded
Open this post in threaded view
|

bug#38219: Error on leaving Ediff after killing vital buffer

martin rudalics
 > Can the same be said about read-from-minibuffer?
 > Shouldn't read-from-minibuffer mangle 'this-command'?
 > What it some command wants to check if 'this-command'
 > is 'exit-minibuffer' afterwards?  Shouldn't this change better
 > to be localized to callers in ediff, instead of adding such hack?

You could argue that 'ediff' already breaks

(defalias 'y-or-n-p 'yes-or-no-p)

They would probably say that consulting 'this-command' after a
'y-or-n-p' "has worked ever since".  Guess whose argument wins.

martin



Reply | Threaded
Open this post in threaded view
|

bug#38219: Error on leaving Ediff after killing vital buffer

Juri Linkov-2
> You could argue that 'ediff' already breaks
>
> (defalias 'y-or-n-p 'yes-or-no-p)
>
> They would probably say that consulting 'this-command' after a
> 'y-or-n-p' "has worked ever since".  Guess whose argument wins.

Good example.  This means that 'ediff' is broken, here is the fix:

diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
index a481defe29..c85241b2ea 100644
--- a/lisp/vc/ediff-util.el
+++ b/lisp/vc/ediff-util.el
@@ -1038,6 +1038,7 @@ ediff-toggle-read-only
  (format
   "File %s is under version control.  Check it out? "
   (ediff-abbreviate-file-name file))))
+                   (setq this-command 'ediff-toggle-read-only)
    ;; if we checked the file out, we should also change the
    ;; original state of buffer-read-only to nil.  If we don't
    ;; do this, the mode line will show %%, since the file was
@@ -2379,6 +2380,7 @@ ediff-quit
       " & show containing session group" "")))
  (progn
   (message "")
+          (setq this-command 'ediff-quit)
   (set-buffer ctl-buf)
   (ediff-really-quit reverse-default-keep-variants))
       (select-frame ctl-frm)



Reply | Threaded
Open this post in threaded view
|

bug#38219: Error on leaving Ediff after killing vital buffer

Juri Linkov-2
>> You could argue that 'ediff' already breaks
>>
>> (defalias 'y-or-n-p 'yes-or-no-p)
>>
>> They would probably say that consulting 'this-command' after a
>> 'y-or-n-p' "has worked ever since".  Guess whose argument wins.
>
> Good example.  This means that 'ediff' is broken, here is the fix:

Now pushed and closed.