unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
@ 2016-09-22 23:14 Vasilij Schneidermann
  2016-09-23  2:22 ` Clément Pit--Claudel
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Vasilij Schneidermann @ 2016-09-22 23:14 UTC (permalink / raw)
  To: 24514

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

I wrote a minimal patch that increases the overall consistency in a
backtrace buffer by printing the call stack frames as S-Expressions.

Before:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
  +(1 t)
  eval((+ 1 t) nil)
  eval-expression((+ 1 t) nil)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)

After:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
  (debug error (wrong-type-argument number-or-marker-p t))
  (+ 1 t)
  (eval (+ 1 t) nil)
  (eval-expression (+ 1 t) nil)
  (funcall-interactively eval-expression (+ 1 t) nil)
  (call-interactively eval-expression nil nil)
  (command-execute eval-expression)

Now, this patch isn't perfect.  For some reason there's an extra debug
line in the second version, I've yet to investigate into the reason for
this.  The other problem is that while I can't imagine any reason to go
back to the original view of the backtrace, I cannot rule out that this
change might break other tools relying on it.  I'd appreciate any
feedback on this.

[-- Attachment #2: 0001-Make-backtraces-great-again.patch --]
[-- Type: text/x-diff, Size: 1342 bytes --]

From 232cb613a83f128d8ee90d7a52fcbde06fd29766 Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <v.schneidermann@gmail.com>
Date: Thu, 22 Sep 2016 23:01:21 +0200
Subject: [PATCH] Make backtraces great again

---
 lisp/emacs-lisp/debug.el | 2 +-
 src/eval.c               | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 22a3f39..4020620 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -279,7 +279,7 @@ That buffer should be current already."
   (goto-char (point-min))
   (delete-region (point)
 		 (progn
-		   (search-forward "\n  debug(")
+		   (search-forward "\n  (debug")
 		   (forward-line (if (eq (car args) 'debug)
                                      ;; Remove debug--implement-debug-on-entry
                                      ;; and the advice's `apply' frame.
diff --git a/src/eval.c b/src/eval.c
index 72facd5..e32e7a1 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3409,8 +3409,9 @@ Output stream used is value of `standard-output'.  */)
       else
 	{
 	  tem = backtrace_function (pdl);
-	  Fprin1 (tem, Qnil);	/* This can QUIT.  */
 	  write_string ("(");
+	  Fprin1 (tem, Qnil);	/* This can QUIT.  */
+	  write_string (" ");
 	  {
 	    ptrdiff_t i;
 	    for (i = 0; i < backtrace_nargs (pdl); i++)
-- 
2.9.3


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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-22 23:14 bug#24514: 24.5; [WIP][PATCH] Lispy backtraces Vasilij Schneidermann
@ 2016-09-23  2:22 ` Clément Pit--Claudel
  2016-09-23  7:51   ` Vasilij Schneidermann
                     ` (2 more replies)
       [not found] ` <mailman.2864.1474586229.22741.bug-gnu-emacs@gnu.org>
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 17+ messages in thread
From: Clément Pit--Claudel @ 2016-09-23  2:22 UTC (permalink / raw)
  To: Vasilij Schneidermann


[-- Attachment #1.1: Type: text/plain, Size: 1706 bytes --]

This looks great! I love it.  And the patch looks very clean, too.

But it scares me a bit.  Some tools do depend on e.g. trimming a backtrace after printing it.  Does edebug work with your patch, for example?

I'm not sure what the right way to transition is.  Maybe Emacs should let Lisp programs access the backtraces in a structured way, and then backtrace printing would only be a user-facing facility (programs wouldn't use the textual representation).

Cheers,
Clément.

On 2016-09-22 19:14, Vasilij Schneidermann wrote:
> I wrote a minimal patch that increases the overall consistency in a
> backtrace buffer by printing the call stack frames as S-Expressions.
> 
> Before:
> 
> Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
>   +(1 t)
>   eval((+ 1 t) nil)
>   eval-expression((+ 1 t) nil)
>   call-interactively(eval-expression nil nil)
>   command-execute(eval-expression)
> 
> After:
> 
> Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
>   (debug error (wrong-type-argument number-or-marker-p t))
>   (+ 1 t)
>   (eval (+ 1 t) nil)
>   (eval-expression (+ 1 t) nil)
>   (funcall-interactively eval-expression (+ 1 t) nil)
>   (call-interactively eval-expression nil nil)
>   (command-execute eval-expression)
> 
> Now, this patch isn't perfect.  For some reason there's an extra debug
> line in the second version, I've yet to investigate into the reason for
> this.  The other problem is that while I can't imagine any reason to go
> back to the original view of the backtrace, I cannot rule out that this
> change might break other tools relying on it.  I'd appreciate any
> feedback on this.
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23  2:22 ` Clément Pit--Claudel
@ 2016-09-23  7:51   ` Vasilij Schneidermann
  2016-09-23 13:22     ` Clément Pit--Claudel
  2016-09-23  8:12   ` Vasilij Schneidermann
  2016-09-23  9:44   ` Eli Zaretskii
  2 siblings, 1 reply; 17+ messages in thread
From: Vasilij Schneidermann @ 2016-09-23  7:51 UTC (permalink / raw)
  To: Clément Pit--Claudel, 24514

> This looks great! I love it.  And the patch looks very clean, too.

Thanks!  I've updated the patch after a bit more testing.  The extra
debug line happened because the search in debug.el did actually search
for a `debug' call, so `(search-forward "\  (debug")` would match a call
to `debug-foo` as well.  Assuming there will always be extra args, this
can be solved by appending a space to the search string.  In case this
is not true, I'll have to go for `re-search-forward` to handle a
`(debug)` as well.  The other problem was that there were extraneous
spaces at times, this can be handled by making the line that prepends an
argument with a space unconditional.

> But it scares me a bit.  Some tools do depend on e.g. trimming a
> backtrace after printing it.  Does edebug work with your patch, for
> example?

Yes, it does.  However, you're raising a good point here.  I would
expect such a change to neither break Emacs core nor any of the bundled
Emacs Lisp code.  Guarantees about popular external code are hard to
make, but communicating the change with the respective authors and
maintainers should do the trick.  What I've initially asked for is
whether there might be any good reason for *not* doing things this way.

I've grepped the 24.5 sources and most usage of `backtrace' appears to
be of diagnostic nature.  The only thing other than `debug.el`
manipulating them is `edebug-backtrace' which I've never heard of
before.  I'll take a better look at its sources for my next revision of
the patch.

> I'm not sure what the right way to transition is.  Maybe Emacs should
> let Lisp programs access the backtraces in a structured way, and then
> backtrace printing would only be a user-facing facility (programs
> wouldn't use the textual representation).

There is actually a way to do this, simply use `backtrace-frame' with an
increasing integer argument until it returns nil:

    (let ((frame t) ; dummy value to kick-start the loop
          (i 0))
      (while frame
        (setq frame (backtrace-frame i))
        (message "%S" frame)
        (setq i (1+ i))))

This feature is used in `ert.el` to save a backtrace and reconstruct it.
Perhaps all current users of `backtrace' that rely on its current
representation should be using `backtrace-frame' instead?





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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23  2:22 ` Clément Pit--Claudel
  2016-09-23  7:51   ` Vasilij Schneidermann
@ 2016-09-23  8:12   ` Vasilij Schneidermann
  2016-09-23  9:44   ` Eli Zaretskii
  2 siblings, 0 replies; 17+ messages in thread
From: Vasilij Schneidermann @ 2016-09-23  8:12 UTC (permalink / raw)
  To: Clément Pit--Claudel, 24514

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

Ouch, forgot the attachment.

[-- Attachment #2: 0001-Make-backtraces-great-again.patch --]
[-- Type: text/x-diff, Size: 1444 bytes --]

From f85845921c5e06475c925508ba41b06340f4c95a Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <v.schneidermann@gmail.com>
Date: Thu, 22 Sep 2016 23:01:21 +0200
Subject: [PATCH] Make backtraces great again

---
 lisp/emacs-lisp/debug.el | 2 +-
 src/eval.c               | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 22a3f39..a1ca336 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -279,7 +279,7 @@ That buffer should be current already."
   (goto-char (point-min))
   (delete-region (point)
 		 (progn
-		   (search-forward "\n  debug(")
+		   (search-forward "\n  (debug ")
 		   (forward-line (if (eq (car args) 'debug)
                                      ;; Remove debug--implement-debug-on-entry
                                      ;; and the advice's `apply' frame.
diff --git a/src/eval.c b/src/eval.c
index 72facd5..698e0a1 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3409,13 +3409,13 @@ Output stream used is value of `standard-output'.  */)
       else
 	{
 	  tem = backtrace_function (pdl);
-	  Fprin1 (tem, Qnil);	/* This can QUIT.  */
 	  write_string ("(");
+	  Fprin1 (tem, Qnil);	/* This can QUIT.  */
 	  {
 	    ptrdiff_t i;
 	    for (i = 0; i < backtrace_nargs (pdl); i++)
 	      {
-		if (i) write_string (" ");
+		write_string (" ");
 		Fprin1 (backtrace_args (pdl)[i], Qnil);
 	      }
 	  }
-- 
2.9.3


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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23  2:22 ` Clément Pit--Claudel
  2016-09-23  7:51   ` Vasilij Schneidermann
  2016-09-23  8:12   ` Vasilij Schneidermann
@ 2016-09-23  9:44   ` Eli Zaretskii
  2016-09-23  9:55     ` bug#24515: " Vasilij Schneidermann
  2 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2016-09-23  9:44 UTC (permalink / raw)
  To: Clément Pit--Claudel; +Cc: v.schneidermann

> From: Clément Pit--Claudel <clement.pit@gmail.com>
> Date: Thu, 22 Sep 2016 22:22:13 -0400
> 
> I'm not sure what the right way to transition is.  Maybe Emacs should let Lisp programs access the backtraces in a structured way, and then backtrace printing would only be a user-facing facility (programs wouldn't use the textual representation).

How about if we install this with a user option, off by default, that
switches to the new format when turned on?  Then we could collect user
experience for some time, and make this the default if everyone likes
it.





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

* bug#24515: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23  9:44   ` Eli Zaretskii
@ 2016-09-23  9:55     ` Vasilij Schneidermann
  2016-09-23 10:06       ` Eli Zaretskii
  2016-09-23 13:25       ` Clément Pit--Claudel
  0 siblings, 2 replies; 17+ messages in thread
From: Vasilij Schneidermann @ 2016-09-23  9:55 UTC (permalink / raw)
  To: Eli Zaretskii, 24515; +Cc: Clément Pit--Claudel

> How about if we install this with a user option, off by default, that
> switches to the new format when turned on?

This sounds sensible, but I'm not sure whether it's worth complicating
the patch by adding checks for this new user option in `eval.c`,
`debug.el` and `edebug.el`.

> Then we could collect user experience for some time, and make this the
> default if everyone likes it.

How do you plan on informing and surveying users?  Would a discussion on
emacs-devel be the way to do this or do you have something different in
mind?





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

* bug#24515: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23  9:55     ` bug#24515: " Vasilij Schneidermann
@ 2016-09-23 10:06       ` Eli Zaretskii
  2016-09-23 13:25       ` Clément Pit--Claudel
  1 sibling, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2016-09-23 10:06 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 24515, clement.pit

> Date: Fri, 23 Sep 2016 11:55:41 +0200
> From: Vasilij Schneidermann <v.schneidermann@gmail.com>
> Cc: Clément Pit--Claudel <clement.pit@gmail.com>
> 
> > How about if we install this with a user option, off by default, that
> > switches to the new format when turned on?
> 
> This sounds sensible, but I'm not sure whether it's worth complicating
> the patch by adding checks for this new user option in `eval.c`,
> `debug.el` and `edebug.el`.

It's a complication, sure.  But it doesn't sound too complicated, and
given the fears of breaking someone's code, I think it's justified.

> > Then we could collect user experience for some time, and make this the
> > default if everyone likes it.
> 
> How do you plan on informing and surveying users?  Would a discussion on
> emacs-devel be the way to do this or do you have something different in
> mind?

I thought just watching the discussions and the bug tracker would be
enough.  Then, a couple of releases from now, we could ask whether
making it the default would be okay, and see how people react.





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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23  7:51   ` Vasilij Schneidermann
@ 2016-09-23 13:22     ` Clément Pit--Claudel
  0 siblings, 0 replies; 17+ messages in thread
From: Clément Pit--Claudel @ 2016-09-23 13:22 UTC (permalink / raw)
  To: Vasilij Schneidermann, 24514


[-- Attachment #1.1: Type: text/plain, Size: 1782 bytes --]

On 2016-09-23 03:51, Vasilij Schneidermann wrote:
>> This looks great! I love it.  And the patch looks very clean, too.
> 
> Thanks!  I've updated the patch after a bit more testing.  The extra 
> debug line happened because the search in debug.el did actually
> search for a `debug' call, so `(search-forward "\  (debug")` would
> match a call to `debug-foo` as well. Assuming there will always be
> extra args, this can be solved by appending a space to the search
> string.

Could it just search for the first occurence only?

>> But it scares me a bit.  Some tools do depend on e.g. trimming a 
>> backtrace after printing it.  Does edebug work with your patch,
>> for example?
> 
> Yes, it does.  …  The only thing other than `debug.el` 
> manipulating them is `edebug-backtrace' which I've never heard of 
> before. 

This is the one I was scared about. You can get a backtrace while edebugging by pressing t IIRC.  I have a faint memory of this code doing nasty things to remove edebug instrumentation from the backtrace before displaying it.

>> I'm not sure what the right way to transition is.  Maybe Emacs
>> should let Lisp programs access the backtraces in a structured way,
>> and then backtrace printing would only be a user-facing facility
>> (programs wouldn't use the textual representation).
> 
> There is actually a way to do this, simply use `backtrace-frame' with
> an increasing integer argument until it returns nil:
> 
> (let ((frame t) ; dummy value to kick-start the loop (i 0)) (while
> frame (setq frame (backtrace-frame i)) (message "%S" frame) (setq i
> (1+ i))))

Neat! Didn't know about this.  I think it would be great to add a pointer to this in the documentation of `backtrace'.

Great work :)
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* bug#24515: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23  9:55     ` bug#24515: " Vasilij Schneidermann
  2016-09-23 10:06       ` Eli Zaretskii
@ 2016-09-23 13:25       ` Clément Pit--Claudel
  2016-09-23 16:33         ` John Wiegley
  1 sibling, 1 reply; 17+ messages in thread
From: Clément Pit--Claudel @ 2016-09-23 13:25 UTC (permalink / raw)
  To: Vasilij Schneidermann, Eli Zaretskii, 24515


[-- Attachment #1.1: Type: text/plain, Size: 307 bytes --]

On 2016-09-23 05:55, Eli Zaretskii <eliz@gnu.org> wrote:
> How about if we install this with a user option, off by default, that
> switches to the new format when turned on?

I like this idea :) In that case, we should probably mention the options in NEWS, adding a note that code should be updated.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* bug#24515: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-23 13:25       ` Clément Pit--Claudel
@ 2016-09-23 16:33         ` John Wiegley
  0 siblings, 0 replies; 17+ messages in thread
From: John Wiegley @ 2016-09-23 16:33 UTC (permalink / raw)
  To: Clément Pit--Claudel; +Cc: Vasilij Schneidermann, 24515

>>>>> "CP" == Clément Pit--Claudel <clement.pit@gmail.com> writes:

PC> On 2016-09-23 05:55, Eli Zaretskii <eliz@gnu.org> wrote:
>> How about if we install this with a user option, off by default, that
>> switches to the new format when turned on?

CP> I like this idea :) In that case, we should probably mention the options
PC> in NEWS, adding a note that code should be updated.

I third the appreciation of having a user option.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2





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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
       [not found] ` <mailman.2864.1474586229.22741.bug-gnu-emacs@gnu.org>
@ 2016-09-23 18:47   ` Alan Mackenzie
  0 siblings, 0 replies; 17+ messages in thread
From: Alan Mackenzie @ 2016-09-23 18:47 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 24514

In article <mailman.2864.1474586229.22741.bug-gnu-emacs@gnu.org> you wrote:
> [-- text/plain, encoding 7bit, charset: utf-8, 30 lines --]

> I wrote a minimal patch that increases the overall consistency in a
> backtrace buffer by printing the call stack frames as S-Expressions.

> Before:

> Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
>   +(1 t)
>   eval((+ 1 t) nil)
>   eval-expression((+ 1 t) nil)
>   call-interactively(eval-expression nil nil)
>   command-execute(eval-expression)

> After:

> Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
>   (debug error (wrong-type-argument number-or-marker-p t))
>   (+ 1 t)
>   (eval (+ 1 t) nil)
>   (eval-expression (+ 1 t) nil)
>   (funcall-interactively eval-expression (+ 1 t) nil)
>   (call-interactively eval-expression nil nil)
>   (command-execute eval-expression)

I'm not sure I'm in favour of this change.  There is some tool in some
circumstances which prints the lines in the "before:" fashion
interspersed with internal forms from function which start off with "("
in column 0.  Having the distinction between lines starting with "(" and
lines starting with the function name is handy for telling them apart.

Sorry I can't be more specific about the circumstances this happens in,
but it happens relatively frequently.

> Now, this patch isn't perfect.  For some reason there's an extra debug
> line in the second version, I've yet to investigate into the reason for
> this.  The other problem is that while I can't imagine any reason to go
> back to the original view of the backtrace, I cannot rule out that this
> change might break other tools relying on it.  I'd appreciate any
> feedback on this.

-- 
Alan Mackenzie (Nuremberg, Germany).






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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-22 23:14 bug#24514: 24.5; [WIP][PATCH] Lispy backtraces Vasilij Schneidermann
  2016-09-23  2:22 ` Clément Pit--Claudel
       [not found] ` <mailman.2864.1474586229.22741.bug-gnu-emacs@gnu.org>
@ 2016-09-23 20:43 ` Richard Stallman
  2016-09-27 19:16 ` Vasilij Schneidermann
  3 siblings, 0 replies; 17+ messages in thread
From: Richard Stallman @ 2016-09-23 20:43 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 24514

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

An item in a backtrace consists of a function and ACTUAL parameters
(argument values).  If you do  (list 'a 'b), the function will be  list
and the argument values will be (a b).

(list a b) is a misleading way to represent that.  It looks like a and b
are expressions to be evaluated.
-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-22 23:14 bug#24514: 24.5; [WIP][PATCH] Lispy backtraces Vasilij Schneidermann
                   ` (2 preceding siblings ...)
  2016-09-23 20:43 ` Richard Stallman
@ 2016-09-27 19:16 ` Vasilij Schneidermann
  2016-09-28 15:28   ` Eli Zaretskii
  2016-10-12 15:34   ` Vasilij Schneidermann
  3 siblings, 2 replies; 17+ messages in thread
From: Vasilij Schneidermann @ 2016-09-27 19:16 UTC (permalink / raw)
  To: 24514

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

Hello again,

I've updated the patch as suggested to make the behavior configurable.
I'm pretty sure though that the customization option's name is terrible.
Any suggestions for a better one?

[-- Attachment #2: 0001-Make-backtraces-great-again.patch --]
[-- Type: text/x-diff, Size: 2739 bytes --]

From 7b953b3ec64b26edd812d0ad3e4e01bad4f94193 Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <v.schneidermann@gmail.com>
Date: Thu, 22 Sep 2016 23:01:21 +0200
Subject: [PATCH] Make backtraces great again

---
 lisp/emacs-lisp/debug.el  |  4 +++-
 lisp/emacs-lisp/edebug.el |  4 +++-
 src/eval.c                | 12 ++++++++++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 22a3f39..c481ee2 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -279,7 +279,9 @@ That buffer should be current already."
   (goto-char (point-min))
   (delete-region (point)
 		 (progn
-		   (search-forward "\n  debug(")
+		   (search-forward (if debugger-lispy-backtrace
+                                       "\n  (debug "
+                                     "\n  debug("))
 		   (forward-line (if (eq (car args) 'debug)
                                      ;; Remove debug--implement-debug-on-entry
                                      ;; and the advice's `apply' frame.
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index c283c16..a2e9c00 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -3797,7 +3797,9 @@ Otherwise call `debug' normally."
 	  (forward-line 1)
 	  (delete-region last-ok-point (point)))
 
-	 ((looking-at "^  edebug")
+	 ((looking-at (if debugger-lispy-backtrace
+                          "^  (edebug"
+                        "^  edebug"))
 	  (forward-line 1)
 	  (delete-region last-ok-point (point))
 	  )))
diff --git a/src/eval.c b/src/eval.c
index 72facd5..fc6e5a6 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3409,13 +3409,17 @@ Output stream used is value of `standard-output'.  */)
       else
 	{
 	  tem = backtrace_function (pdl);
+	  if (debugger_lispy_backtrace)
+	    write_string ("(");
 	  Fprin1 (tem, Qnil);	/* This can QUIT.  */
-	  write_string ("(");
+	  if (!debugger_lispy_backtrace)
+	    write_string ("(");
 	  {
 	    ptrdiff_t i;
 	    for (i = 0; i < backtrace_nargs (pdl); i++)
 	      {
-		if (i) write_string (" ");
+		if (i || debugger_lispy_backtrace)
+		  write_string(" ");
 		Fprin1 (backtrace_args (pdl)[i], Qnil);
 	      }
 	  }
@@ -3838,6 +3842,10 @@ This is nil when the debugger is called under circumstances where it
 might not be safe to continue.  */);
   debugger_may_continue = 1;
 
+  DEFVAR_BOOL ("debugger-lispy-backtrace", debugger_lispy_backtrace,
+	       doc: /* Non-nil means display call stack frames as lists. */);
+  debugger_lispy_backtrace = 0;
+
   DEFVAR_LISP ("debugger", Vdebugger,
 	       doc: /* Function to call to invoke debugger.
 If due to frame exit, args are `exit' and the value being returned;
-- 
2.9.3


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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-27 19:16 ` Vasilij Schneidermann
@ 2016-09-28 15:28   ` Eli Zaretskii
  2016-09-30 10:29     ` Vasilij Schneidermann
  2016-10-12 15:34   ` Vasilij Schneidermann
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2016-09-28 15:28 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 24514

> Date: Tue, 27 Sep 2016 21:16:03 +0200
> From: Vasilij Schneidermann <v.schneidermann@gmail.com>
> 
> I've updated the patch as suggested to make the behavior configurable.

Thanks!

> I'm pretty sure though that the customization option's name is terrible.
> Any suggestions for a better one?

How about debugger-stack-frame-as-list?

Two minor nits to make this patch perfect:

  . Add the variable to cus-start.el, so that it will be a defcustom
  . Add a NEWS entry and mention the variable in the ELisp manual

Thanks a lot for working on this.





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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-28 15:28   ` Eli Zaretskii
@ 2016-09-30 10:29     ` Vasilij Schneidermann
  2016-09-30 13:26       ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Vasilij Schneidermann @ 2016-09-30 10:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24514

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

> How about debugger-stack-frame-as-list?

Thanks, sounds good to me.

> Two minor nits to make this patch perfect:
> 
>   . Add the variable to cus-start.el, so that it will be a defcustom
>   . Add a NEWS entry and mention the variable in the ELisp manual

I've done these and changed the commit message to comply better with the
changelog style.  Let me know if there's still something off.

[-- Attachment #2: 0001-Add-new-debugger-stack-frame-as-list-option.patch --]
[-- Type: text/x-diff, Size: 5421 bytes --]

From 62d47f75adc8d319b6fc35bb79bcf316d16bec00 Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <v.schneidermann@gmail.com>
Date: Thu, 22 Sep 2016 23:01:21 +0200
Subject: [PATCH] Add new 'debugger-stack-frame-as-list' option

Allow displaying all lines in `backtrace' as lists.
* doc/lispref/debugging.texi: Document the new variable
* src/eval.c: New variable
* lisp/emacs-lisp/debug.el: Adjust backtrace processing for new variable
* lisp/emacs-lisp/edebug.el: Adjust backtrace processing for new
variable
---
 doc/lispref/debugging.texi | 29 +++++++++++++++++++++++++++++
 etc/NEWS                   |  5 +++++
 lisp/cus-start.el          |  1 +
 lisp/emacs-lisp/debug.el   |  4 +++-
 lisp/emacs-lisp/edebug.el  |  4 +++-
 src/eval.c                 | 12 ++++++++++--
 6 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
index 98c4705..15eea8a 100644
--- a/doc/lispref/debugging.texi
+++ b/doc/lispref/debugging.texi
@@ -623,6 +623,35 @@ forms are elided.
 @end smallexample
 @end deffn
 
+@defvar debugger-stack-frame-as-list
+If this variable is non-@code{nil}, every line of the backtrace is
+displayed as a list.  This aims to improve backtrace readability at
+the cost of special forms no longer being visually different from
+regular function calls.
+
+The above example would look as follows:
+@smallexample
+@group
+----------- Buffer: backtrace-output ------------
+  (backtrace)
+  (list ...computing arguments...)
+@end group
+  (progn ...)
+  (eval (progn (1+ var) (list (quote testing) (backtrace))))
+  (setq ...)
+  (save-excursion ...)
+  (let ...)
+  (with-output-to-temp-buffer ...)
+  (eval (with-output-to-temp-buffer ...))
+  (eval-last-sexp-1 nil)
+@group
+  (eval-last-sexp nil)
+  (call-interactively eval-last-sexp)
+----------- Buffer: backtrace-output ------------
+@end group
+@end smallexample
+@end defvar
+
 @defvar debug-on-next-call
 @cindex @code{eval}, and debugging
 @cindex @code{apply}, and debugging
diff --git a/etc/NEWS b/etc/NEWS
index a72be53..a88d9ec 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -179,6 +179,11 @@ questions, with a handy way to display help texts.
 +++
 ** 'switch-to-buffer-preserve-window-point' now defaults to t.
 
++++
+** The new variable 'debugger-stack-frame-as-list' allows displaying
+all call stack frames in 'backtrace' as lists.  Both debug.el and
+edebug.el have been updated accordingly.
+
 \f
 * Editing Changes in Emacs 25.2
 
diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index c830ed8..c6d0ae4 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -246,6 +246,7 @@ Leaving \"Default\" unchecked is equivalent with specifying a default of
 	     (debug-ignored-errors debug (repeat (choice symbol regexp)))
 	     (debug-on-quit debug boolean)
 	     (debug-on-signal debug boolean)
+             (debugger-stack-frame-as-list debugger boolean)
 	     ;; fileio.c
 	     (delete-by-moving-to-trash auto-save boolean "23.1")
 	     (auto-save-visited-file-name auto-save boolean)
diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 22a3f39..7d27380 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -279,7 +279,9 @@ That buffer should be current already."
   (goto-char (point-min))
   (delete-region (point)
 		 (progn
-		   (search-forward "\n  debug(")
+		   (search-forward (if debugger-stack-frame-as-list
+                                       "\n  (debug "
+                                     "\n  debug("))
 		   (forward-line (if (eq (car args) 'debug)
                                      ;; Remove debug--implement-debug-on-entry
                                      ;; and the advice's `apply' frame.
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index c283c16..0c2a8ad 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -3797,7 +3797,9 @@ Otherwise call `debug' normally."
 	  (forward-line 1)
 	  (delete-region last-ok-point (point)))
 
-	 ((looking-at "^  edebug")
+	 ((looking-at (if debugger-stack-frame-as-list
+                          "^  (edebug"
+                        "^  edebug"))
 	  (forward-line 1)
 	  (delete-region last-ok-point (point))
 	  )))
diff --git a/src/eval.c b/src/eval.c
index 72facd5..f07cc83 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3409,13 +3409,17 @@ Output stream used is value of `standard-output'.  */)
       else
 	{
 	  tem = backtrace_function (pdl);
+	  if (debugger_stack_frame_as_list)
+	    write_string ("(");
 	  Fprin1 (tem, Qnil);	/* This can QUIT.  */
-	  write_string ("(");
+	  if (!debugger_stack_frame_as_list)
+	    write_string ("(");
 	  {
 	    ptrdiff_t i;
 	    for (i = 0; i < backtrace_nargs (pdl); i++)
 	      {
-		if (i) write_string (" ");
+		if (i || debugger_stack_frame_as_list)
+		  write_string(" ");
 		Fprin1 (backtrace_args (pdl)[i], Qnil);
 	      }
 	  }
@@ -3838,6 +3842,10 @@ This is nil when the debugger is called under circumstances where it
 might not be safe to continue.  */);
   debugger_may_continue = 1;
 
+  DEFVAR_BOOL ("debugger-stack-frame-as-list", debugger_stack_frame_as_list,
+	       doc: /* Non-nil means display call stack frames as lists. */);
+  debugger_stack_frame_as_list = 0;
+
   DEFVAR_LISP ("debugger", Vdebugger,
 	       doc: /* Function to call to invoke debugger.
 If due to frame exit, args are `exit' and the value being returned;
-- 
2.10.0


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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-30 10:29     ` Vasilij Schneidermann
@ 2016-09-30 13:26       ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2016-09-30 13:26 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 24514

> Date: Fri, 30 Sep 2016 12:29:56 +0200
> From: Vasilij Schneidermann <v.schneidermann@gmail.com>
> Cc: 24514@debbugs.gnu.org
> 
> > Two minor nits to make this patch perfect:
> > 
> >   . Add the variable to cus-start.el, so that it will be a defcustom
> >   . Add a NEWS entry and mention the variable in the ELisp manual
> 
> I've done these and changed the commit message to comply better with the
> changelog style.  Let me know if there's still something off.

Thanks, pushed to master.

The log entry need some tweaking to bring it to our standards; see my
commit for what was needed.

If this bug can be closed now, please do.





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

* bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
  2016-09-27 19:16 ` Vasilij Schneidermann
  2016-09-28 15:28   ` Eli Zaretskii
@ 2016-10-12 15:34   ` Vasilij Schneidermann
  1 sibling, 0 replies; 17+ messages in thread
From: Vasilij Schneidermann @ 2016-10-12 15:34 UTC (permalink / raw)
  To: 24514-done

Feature was fully implemented and committed to the respective branch.





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

end of thread, other threads:[~2016-10-12 15:34 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-22 23:14 bug#24514: 24.5; [WIP][PATCH] Lispy backtraces Vasilij Schneidermann
2016-09-23  2:22 ` Clément Pit--Claudel
2016-09-23  7:51   ` Vasilij Schneidermann
2016-09-23 13:22     ` Clément Pit--Claudel
2016-09-23  8:12   ` Vasilij Schneidermann
2016-09-23  9:44   ` Eli Zaretskii
2016-09-23  9:55     ` bug#24515: " Vasilij Schneidermann
2016-09-23 10:06       ` Eli Zaretskii
2016-09-23 13:25       ` Clément Pit--Claudel
2016-09-23 16:33         ` John Wiegley
     [not found] ` <mailman.2864.1474586229.22741.bug-gnu-emacs@gnu.org>
2016-09-23 18:47   ` Alan Mackenzie
2016-09-23 20:43 ` Richard Stallman
2016-09-27 19:16 ` Vasilij Schneidermann
2016-09-28 15:28   ` Eli Zaretskii
2016-09-30 10:29     ` Vasilij Schneidermann
2016-09-30 13:26       ` Eli Zaretskii
2016-10-12 15:34   ` Vasilij Schneidermann

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