bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Tino Calancha]

I often kills the *Backtrace* buffer; then, errors won't show
*Backtrace* again in this session.
Could *Backtrace* be guarded with something like `edebug-kill-buffer'
in kill-buffer-hook?

emacs -Q
;; eval following form:
(error "hi") ; This jump to *Backtrace*
q ; Back to top level

;; Eval it again:
(error "hi") ; This time kill *Backtrace*
C-x k RET

;; Eval it once more:
(error "hi")
;; 'hi' is printed in echo area, but *Backtrace* is not shown.

In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-04-25 built
Repository revision: 5b0fdefb4ca54b0d3dac3047ac1e4b380beb6ba7

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[npostavs]

severity 26650 minor
quit

Tino Calancha <tino.calancha@gmail.com> writes:

> (error "hi") ; This time kill *Backtrace*
> C-x k RET
>
> ;; Eval it once more:
> (error "hi")
> ;; 'hi' is printed in echo area, but *Backtrace* is not shown.

That's because you're still in the recursive edit, it's like evaluating
'(error "hi")' before hitting 'q' in the *Backtrace* buffer.  You can
use C-M-c (exit-recursive-edit) to get back to normal.  I think it would
make sense to do that automatically if you kill the *Backtrace* buffer.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Tino Calancha]

On Tue, 25 Apr 2017, npostavs@users.sourceforge.net wrote:

> severity 26650 minor
> quit
>
> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> (error "hi") ; This time kill *Backtrace*
>> C-x k RET
>>
>> ;; Eval it once more:
>> (error "hi")
>> ;; 'hi' is printed in echo area, but *Backtrace* is not shown.
>
> That's because you're still in the recursive edit, it's like evaluating
> '(error "hi")' before hitting 'q' in the *Backtrace* buffer.  You can
> use C-M-c (exit-recursive-edit) to get back to normal.  I think it would
> make sense to do that automatically if you kill the *Backtrace* buffer.
Yes, it would be useful.  I don't know how, though.
Sometimes i kill *Bactrace* from *Ibuffer*/*Buffer List* with the 
recursive edit still ongoing.  It's a classical mistake.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[npostavs]

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

tags 26650 patch
quit

Tino Calancha <tino.calancha@gmail.com> writes:

>>
>> That's because you're still in the recursive edit, it's like evaluating
>> '(error "hi")' before hitting 'q' in the *Backtrace* buffer.  You can
>> use C-M-c (exit-recursive-edit) to get back to normal.  I think it would
>> make sense to do that automatically if you kill the *Backtrace* buffer.
> Yes, it would be useful.  I don't know how, though.
> Sometimes i kill *Bactrace* from *Ibuffer*/*Buffer List* with the
> recursive edit still ongoing.  It's a classical mistake.

Actually, the obvious way of adding `top-level' (that's what 'q' does)
to `kill-buffer-hook' does protect *Backtrace* from killing.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 888 bytes --]

From 9e2f66a300cc0c94fb1369cbf3721d41d98c2d94 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Tue, 25 Apr 2017 08:39:17 -0400
Subject: [PATCH v1] Protect *Backtrace* from being killed (Bug#26650)

* lisp/emacs-lisp/debug.el (debugger-mode): Call `top-level' in
`kill-buffer-hook'.
---
 lisp/emacs-lisp/debug.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 94b683dcb9..0aef94f1d2 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -753,6 +753,8 @@ debugger-mode
 \\{debugger-mode-map}"
   (setq truncate-lines t)
   (set-syntax-table emacs-lisp-mode-syntax-table)
+  (add-hook 'kill-buffer-hook
+            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
   (use-local-map debugger-mode-map))
 \f
 (defcustom debugger-record-buffer "*Debugger-record*"
-- 
2.11.1

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Tino Calancha]

On Tue, 25 Apr 2017, npostavs@users.sourceforge.net wrote:

> tags 26650 patch
> quit
>
> Tino Calancha <tino.calancha@gmail.com> writes:
>
>>>
>>> That's because you're still in the recursive edit, it's like evaluating
>>> '(error "hi")' before hitting 'q' in the *Backtrace* buffer.  You can
>>> use C-M-c (exit-recursive-edit) to get back to normal.  I think it would
>>> make sense to do that automatically if you kill the *Backtrace* buffer.
>> Yes, it would be useful.  I don't know how, though.
>> Sometimes i kill *Bactrace* from *Ibuffer*/*Buffer List* with the
>> recursive edit still ongoing.  It's a classical mistake.
>
> Actually, the obvious way of adding `top-level' (that's what 'q' does)
> to `kill-buffer-hook' does protect *Backtrace* from killing.
Good!  It solves the issue.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Date: Tue, 25 Apr 2017 08:50:00 -0400
> Cc: 26650@debbugs.gnu.org
> 
> diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
> index 94b683dcb9..0aef94f1d2 100644
> --- a/lisp/emacs-lisp/debug.el
> +++ b/lisp/emacs-lisp/debug.el
> @@ -753,6 +753,8 @@ debugger-mode
>  \\{debugger-mode-map}"
>    (setq truncate-lines t)
>    (set-syntax-table emacs-lisp-mode-syntax-table)
> +  (add-hook 'kill-buffer-hook
> +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)

This will throw to top-level when _any_ buffer is killed, as long as
we are in recursive-edit, no?  Is that a good idea?  Shouldn't this
hook do that only for *Backtrace*?

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Tino Calancha]

On Wed, 26 Apr 2017, Eli Zaretskii wrote:

>> From: npostavs@users.sourceforge.net
>> Date: Tue, 25 Apr 2017 08:50:00 -0400
>> Cc: 26650@debbugs.gnu.org
>>
>> diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
>> index 94b683dcb9..0aef94f1d2 100644
>> --- a/lisp/emacs-lisp/debug.el
>> +++ b/lisp/emacs-lisp/debug.el
>> @@ -753,6 +753,8 @@ debugger-mode
>>  \\{debugger-mode-map}"
>>    (setq truncate-lines t)
>>    (set-syntax-table emacs-lisp-mode-syntax-table)
>> +  (add-hook 'kill-buffer-hook
>> +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
>
> This will throw to top-level when _any_ buffer is killed, as long as
> we are in recursive-edit, no?
No, because `add-hook' is called with non-nil LOCAL arg.
> Shouldn't this hook do that only for *Backtrace*?
We can sandwich add-hook inside:
(when (eq (current-buffer) (get-buffer "*Backtrace*"))
   (add-hook ...
)

Then would be less general: just protect *Backtrace* for accidental 
killing.  So far that was my request so i'm fine with that.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > From: npostavs@users.sourceforge.net
> > Date: Tue, 25 Apr 2017 08:50:00 -0400
> > Cc: 26650@debbugs.gnu.org
> > 
> > diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
> > index 94b683dcb9..0aef94f1d2 100644
> > --- a/lisp/emacs-lisp/debug.el
> > +++ b/lisp/emacs-lisp/debug.el
> > @@ -753,6 +753,8 @@ debugger-mode
> >  \\{debugger-mode-map}"
> >    (setq truncate-lines t)
> >    (set-syntax-table emacs-lisp-mode-syntax-table)
> > +  (add-hook 'kill-buffer-hook
> > +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
>
> This will throw to top-level when _any_ buffer is killed, as long as
> we are in recursive-edit, no?  Is that a good idea?  Shouldn't this
> hook do that only for *Backtrace*?

Did you notice that the lambda is added to the buffer-local binding of
the hook?  If you did, I don't understand why this would happen.  If you
didn't, maybe a line break should be added after the lambda expression?

Michael.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Wed, 26 Apr 2017 19:36:54 +0900 (JST)
> cc: npostavs@users.sourceforge.net, Tino Calancha <tino.calancha@gmail.com>, 
>     26650@debbugs.gnu.org
> 
> >> +  (add-hook 'kill-buffer-hook
> >> +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
> >
> > This will throw to top-level when _any_ buffer is killed, as long as
> > we are in recursive-edit, no?
> No, because `add-hook' is called with non-nil LOCAL arg.

Right, sorry for not paying attention.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> >> +  (add-hook 'kill-buffer-hook
>> >> +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
>> >
>> > This will throw to top-level when _any_ buffer is killed, as long as
>> > we are in recursive-edit, no?
>> No, because `add-hook' is called with non-nil LOCAL arg.
>
> Right, sorry for not paying attention.

Maybe I should have written

    (add-hook 'kill-buffer-hook (lambda ...) nil 'local)

to emphasize this?  I'm still undecided on the general style question of
whether to use t or '<symbol> in these cases.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Tino Calancha]

On Wed, 26 Apr 2017, npostavs@users.sourceforge.net wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>>>> +  (add-hook 'kill-buffer-hook
>>>>> +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
>>>>
>>>> This will throw to top-level when _any_ buffer is killed, as long as
>>>> we are in recursive-edit, no?
>>> No, because `add-hook' is called with non-nil LOCAL arg.
>>
>> Right, sorry for not paying attention.
>
> Maybe I should have written
>
>    (add-hook 'kill-buffer-hook (lambda ...) nil 'local)
>
> to emphasize this?  I'm still undecided on the general style question of
> whether to use t or '<symbol> in these cases.
This reminds me the discussion here:
https://github.com/bbatsov/emacs-lisp-style-guide/issues/30

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: Tino Calancha <tino.calancha@gmail.com>,  26650@debbugs.gnu.org
> Date: Wed, 26 Apr 2017 08:52:26 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> >> +  (add-hook 'kill-buffer-hook
> >> >> +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
> >> >
> >> > This will throw to top-level when _any_ buffer is killed, as long as
> >> > we are in recursive-edit, no?
> >> No, because `add-hook' is called with non-nil LOCAL arg.
> >
> > Right, sorry for not paying attention.
> 
> Maybe I should have written
> 
>     (add-hook 'kill-buffer-hook (lambda ...) nil 'local)
> 
> to emphasize this?

It's up to you, but the fact that I didn't see it is not a reason good
enough to change anything, IMO.

bug#26650: 26.0.50; Protect *Backtrace* from accidental killing

[npostavs]

tags 26650 fixed
close 26650 26.1
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> >> >> +  (add-hook 'kill-buffer-hook
>> >> >> +            (lambda () (if (> (recursion-depth) 0) (top-level))) nil t)
>> >> >
>> >> > This will throw to top-level when _any_ buffer is killed, as long as
>> >> > we are in recursive-edit, no?
>> >> No, because `add-hook' is called with non-nil LOCAL arg.
>> >
>> > Right, sorry for not paying attention.
>> 
>> Maybe I should have written
>> 
>>     (add-hook 'kill-buffer-hook (lambda ...) nil 'local)
>> 
>> to emphasize this?
>
> It's up to you, but the fact that I didn't see it is not a reason good
> enough to change anything, IMO.

Coming back to this, I found having the "... nil t)" on the same line as
the lambda confusing to read.  So I moved them to the next line; I
didn't change the "t" to "local" [1: ef9f5c672a].

[1: ef9f5c672a]: 2017-05-23 20:11:23 -0400
  Protect *Backtrace* from being killed (Bug#26650)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ef9f5c672a8e248dd7bd682101c03feb2e527340