GNU bug report logs - #24514
24.5; [WIP][PATCH] Lispy backtraces

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24514 in the body.
You can then email your comments to 24514 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 01:14:47 +0200

[Message part 1 (text/plain, inline)]

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.

[0001-Make-backtraces-great-again.patch (text/x-diff, attachment)]

Message #8 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
To: Clément Pit--Claudel <clement.pit <at> gmail.com>,
 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 09:51:16 +0200

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

Message #11 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit--Claudel <clement.pit <at> gmail.com>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Thu, 22 Sep 2016 22:22:13 -0400

[Message part 1 (text/plain, inline)]

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

[signature.asc (application/pgp-signature, attachment)]

Message #14 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
To: Clément Pit--Claudel <clement.pit <at> gmail.com>,
 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 10:12:35 +0200

[Message part 1 (text/plain, inline)]

Ouch, forgot the attachment.

[0001-Make-backtraces-great-again.patch (text/x-diff, attachment)]

Message #17 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Clément Pit--Claudel <clement.pit <at> gmail.com>
Cc: v.schneidermann <at> gmail.com
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 12:44:37 +0300

> From: Clément Pit--Claudel <clement.pit <at> 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.

Message #20 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, 24515 <at> debbugs.gnu.org
Cc: Clément Pit--Claudel <clement.pit <at> gmail.com>
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 11:55:41 +0200

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

Message #23 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
Cc: 24515 <at> debbugs.gnu.org, clement.pit <at> gmail.com
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 13:06:56 +0300

> Date: Fri, 23 Sep 2016 11:55:41 +0200
> From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
> Cc: Clément Pit--Claudel <clement.pit <at> 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.

Message #26 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit--Claudel <clement.pit <at> gmail.com>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>, 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 09:22:15 -0400

[Message part 1 (text/plain, inline)]

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.

[signature.asc (application/pgp-signature, attachment)]

Message #29 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit--Claudel <clement.pit <at> gmail.com>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>, 24515 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 09:25:07 -0400

[Message part 1 (text/plain, inline)]

On 2016-09-23 05:55, Eli Zaretskii <eliz <at> 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.

[signature.asc (application/pgp-signature, attachment)]

Message #32 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
Cc: 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: 23 Sep 2016 18:47:59 -0000

In article <mailman.2864.1474586229.22741.bug-gnu-emacs <at> 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).

Message #35 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
Cc: 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 23 Sep 2016 16:43:18 -0400

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

Message #38 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
To: 24514 <at> debbugs.gnu.org
Subject: Re: 24.5; [WIP][PATCH] Lispy backtraces
Date: Tue, 27 Sep 2016 21:16:03 +0200

[Message part 1 (text/plain, inline)]

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?

[0001-Make-backtraces-great-again.patch (text/x-diff, attachment)]

Message #41 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
Cc: 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Wed, 28 Sep 2016 18:28:00 +0300

> Date: Tue, 27 Sep 2016 21:16:03 +0200
> From: Vasilij Schneidermann <v.schneidermann <at> 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.

Message #44 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 30 Sep 2016 12:29:56 +0200

[Message part 1 (text/plain, inline)]

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

[0001-Add-new-debugger-stack-frame-as-list-option.patch (text/x-diff, attachment)]

Message #47 received at 24514 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
Cc: 24514 <at> debbugs.gnu.org
Subject: Re: bug#24514: 24.5; [WIP][PATCH] Lispy backtraces
Date: Fri, 30 Sep 2016 16:26:03 +0300

> Date: Fri, 30 Sep 2016 12:29:56 +0200
> From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
> Cc: 24514 <at> 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.

Message #52 received at 24514-done <at> debbugs.gnu.org (full text, mbox):

From: Vasilij Schneidermann <v.schneidermann <at> gmail.com>
To: 24514-done <at> debbugs.gnu.org
Subject: Re: 24.5; [WIP][PATCH] Lispy backtraces
Date: Wed, 12 Oct 2016 17:34:42 +0200

Feature was fully implemented and committed to the respective branch.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.