unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#38219: Error on leaving Ediff after killing vital buffer
@ 2019-11-15 13:50 Richard Copley
  2019-11-15 16:35 ` martin rudalics
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Copley @ 2019-11-15 13:50 UTC (permalink / raw)
  To: 38219, juri

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

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 <juri@linkov.net>
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)

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#38219: Error on leaving Ediff after killing vital buffer
  2019-11-15 13:50 bug#38219: Error on leaving Ediff after killing vital buffer Richard Copley
@ 2019-11-15 16:35 ` martin rudalics
  2019-11-16 20:18   ` Juri Linkov
  0 siblings, 1 reply; 6+ messages in thread
From: martin rudalics @ 2019-11-15 16:35 UTC (permalink / raw)
  To: Richard Copley, 38219, juri

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

 > 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

[-- Attachment #2: subr.el.diffs --]
[-- Type: text/plain, Size: 645 bytes --]

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)))
 
 \f

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#38219: Error on leaving Ediff after killing vital buffer
  2019-11-15 16:35 ` martin rudalics
@ 2019-11-16 20:18   ` Juri Linkov
  2019-11-17  9:02     ` martin rudalics
  0 siblings, 1 reply; 6+ messages in thread
From: Juri Linkov @ 2019-11-16 20:18 UTC (permalink / raw)
  To: martin rudalics; +Cc: Richard Copley, 38219

> '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)))
>  
>  \f





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#38219: Error on leaving Ediff after killing vital buffer
  2019-11-16 20:18   ` Juri Linkov
@ 2019-11-17  9:02     ` martin rudalics
  2019-11-17 21:28       ` Juri Linkov
  0 siblings, 1 reply; 6+ messages in thread
From: martin rudalics @ 2019-11-17  9:02 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Richard Copley, 38219

 > 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





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#38219: Error on leaving Ediff after killing vital buffer
  2019-11-17  9:02     ` martin rudalics
@ 2019-11-17 21:28       ` Juri Linkov
  2019-11-18 21:53         ` Juri Linkov
  0 siblings, 1 reply; 6+ messages in thread
From: Juri Linkov @ 2019-11-17 21:28 UTC (permalink / raw)
  To: martin rudalics; +Cc: Richard Copley, 38219

> 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)





^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#38219: Error on leaving Ediff after killing vital buffer
  2019-11-17 21:28       ` Juri Linkov
@ 2019-11-18 21:53         ` Juri Linkov
  0 siblings, 0 replies; 6+ messages in thread
From: Juri Linkov @ 2019-11-18 21:53 UTC (permalink / raw)
  To: martin rudalics; +Cc: Richard Copley, 38219-done

>> 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. 





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-18 21:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-15 13:50 bug#38219: Error on leaving Ediff after killing vital buffer Richard Copley
2019-11-15 16:35 ` martin rudalics
2019-11-16 20:18   ` Juri Linkov
2019-11-17  9:02     ` martin rudalics
2019-11-17 21:28       ` Juri Linkov
2019-11-18 21:53         ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).