bug#6991: Please keep bytecode out of *Backtrace* buffers

bug#6991: Please keep bytecode out of *Backtrace* buffers

[jidanni]

Please keep bytecode out of *Backtrace* buffers.
* It is unreadable.
* It will cause problems when sent via email. Even if one runs col(1)
and strings(1) on it, nobody can read it anyway.
* The mountain of gobbledygook makes people reading give up on trying to help.
E.g., http://article.gmane.org/gmane.emacs.w3m/8695

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Glenn Morris]

No. Closed as wontfix.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

> No. Closed as wontfix.

Why?  No reason given.  This is a reasonable request, which would improve
correspondence about bugs and user difficulties.

Users who are aware of the problem often have to manually excise such binary
codes and replace them with `...' or nothing.  Users who are unaware of the
problem can end up sending an incomplete backtrace.

This should at least be a wishlist item, IMO.  Or at least give a convincing
argument why the request is not appropriate.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Juanma Barranquero]

On Wed, Feb 22, 2012 at 17:43, Drew Adams <drew.adams@oracle.com> wrote:

> Why?  No reason given.  This is a reasonable request, which would improve
> correspondence about bugs and user difficulties.
>
> Users who are aware of the problem often have to manually excise such binary
> codes and replace them with `...' or nothing.

Yes. Not two days ago I had to do exactly that.

> This should at least be a wishlist item, IMO.  Or at least give a convincing
> argument why the request is not appropriate.

Agreed.

    Juanma

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

> > Why?  No reason given.  This is a reasonable request, which 
> > would improve correspondence about bugs and user difficulties.

And no response to that question.  I've reopened this bug.

> > Users who are aware of the problem often have to manually 
> > excise such binary codes and replace them with `...' or nothing.
> 
> Yes. Not two days ago I had to do exactly that.

I do it quite frequently.  And I see portions of backtrace text sent by email
and posted to websites quite often - including in this bug list!

Posting a backtrace is a common and reasonable way to communicate about many
Emacs problems.  Needing to hand-sanitize a backtrace log to remove byte code
portions is silly.  That's what machines and software are for.

> > This should at least be a wishlist item, IMO.  Or at least 
> > give a convincing argument why the request is not appropriate.
> 
> Agreed.

Users should be able to control this (e.g. via an option, and perhaps even a
toggle key in buffer *Backtrace*.

The usefulness of this should be a no-brainer.

I cannot speak to the difficulty of implementating the fix.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Stefan Monnier]

>> > Why?  No reason given.  This is a reasonable request, which 
>> > would improve correspondence about bugs and user difficulties.
> And no response to that question.  I've reopened this bug.

You can try the simple patch below.  It doesn't cut it for me, and
I think the only way to make it work well would be to change the
representation of the byte-codes so that they're not just a "unibyte
string" but an object with a distinctive type: the patch only catches
the case where the byte-codes appear within a printed
byte-compiled-function, not when they're arguments to the `byte-code'
function or to the `make-byte-code' function, and I'm sure there can be
other cases.


        Stefan


=== modified file 'src/print.c'
--- src/print.c	2012-06-28 11:11:48 +0000
+++ src/print.c	2012-07-02 18:37:46 +0000
@@ -1937,8 +1937,11 @@
       else
 	{
 	  ptrdiff_t size = ASIZE (obj);
+	  int bytecode = 0;
+
 	  if (COMPILEDP (obj))
 	    {
 	      PRINTCHAR ('#');
+	      bytecode = 1;
 	      size &= PSEUDOVECTOR_SIZE_MASK;
 	    }
@@ -1978,6 +1981,9 @@
 	      {
 		if (i) PRINTCHAR (' ');
 		tem = AREF (obj, i);
+		if (bytecode && i == 1 && INTEGERP (Vprint_level))
+		  strout ("\"..<bytecode>..\"", 16, 16, printcharfun);
+		else
 		print_object (tem, printcharfun, escapeflag);
 	      }
 	    if (size < real_size)

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

> You can try the simple patch below.  It doesn't cut it for me, and
> I think the only way to make it work well would be to change the
> representation of the byte-codes so that they're not just a "unibyte
> string" but an object with a distinctive type: the patch only catches
> the case where the byte-codes appear within a printed
> byte-compiled-function, not when they're arguments to the `byte-code'
> function or to the `make-byte-code' function, and I'm sure 
> there can be other cases.

Thanks, but I don't build Emacs.  Hopefully something like this will be added to
Emacs itself, even if it is only a partial solution.

Did you mean to close the bug?  It seems to be getting closed just because (?)
6991-done is in the recipients list.

If you did not mean to close it, let's please reopen it.  Even if it is made
only a wishlist item, it is a useful enhancement request.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

> Sent: Monday, July 02, 2012 12:06 PM
>
> > You can try the simple patch below.  It doesn't cut it for me, and
> > I think the only way to make it work well would be to change the
> > representation of the byte-codes so that they're not just a "unibyte
> > string" but an object with a distinctive type: the patch 
> > only catches the case where the byte-codes appear within a printed
> > byte-compiled-function, not when they're arguments to the 
> > `byte-code' function or to the `make-byte-code' function, and I'm sure 
> > there can be other cases.
> 
> Thanks, but I don't build Emacs.  Hopefully something like 
> this will be added to Emacs itself, even if it is only a partial
> solution.
> 
> Did you mean to close the bug?  It seems to be getting closed 
> just because (?) 6991-done is in the recipients list.
> 
> If you did not mean to close it, let's please reopen it.  
> Even if it is made only a wishlist item, it is a useful enhancement
> request.

Can we please follow up on this?  The status seems to be `wishlist' but tagged
`wontfix', which doesn't make a lot of sense to me.

I cannot build Emacs to test this.  Could someone else please test it?  Or could
it please be installed without testing?  (Seriously.)

It would _really_ be helpful if there were no binary crap in Lisp backtraces.
Does that stuff actually help anyone?  If so, perhaps we can keep it as a
(non-default) option, but otherwise, can't we simply elide anything that is not
a printable character, at the least?

I mean replace it by `...', not just change a `display' property.  The problems
I encounter arise from trying to copy + paste the backtrace.

I don't understand why we even have backtraces that one cannot copy & paste
completely, into, e.g., an email.  What's the point of that?  If I try to paste
a copied backtrace I need to paste bits of it piecemeal, because the binary
parts do not paste.  That is tedious and error prone.  Many users might not even
realize that the backtrace did not get completely pasted.

Why is it so hard to advance on something like this?  Stefan provided a C patch
to test, and that was the end of the thread.

So much stuff gets added to the Emacs C sources anyway, sometimes breaking all
kinds of stuff.  Why don't you please just go ahead and install your patch,
Stefan, so we can see whether and how much it helps?

Please consider trying to do something to advance this schmilblick.  I am sure
that it will be appreciated by more than just me.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

Can we please fix this?  What about Stefan's patch?  Etc.

> Sent: Thursday, January 24, 2013 2:43 PM
> 
> > Sent: Monday, July 02, 2012 12:06 PM
> >
> > > You can try the simple patch below.  It doesn't cut it for me, and
> > > I think the only way to make it work well would be to change the
> > > representation of the byte-codes so that they're not just a "unibyte
> > > string" but an object with a distinctive type: the patch
> > > only catches the case where the byte-codes appear within a printed
> > > byte-compiled-function, not when they're arguments to the
> > > `byte-code' function or to the `make-byte-code' function, and I'm sure
> > > there can be other cases.
> >
> > Thanks, but I don't build Emacs.  Hopefully something like
> > this will be added to Emacs itself, even if it is only a partial
> > solution.
> >
> > Did you mean to close the bug?  It seems to be getting closed
> > just because (?) 6991-done is in the recipients list.
> >
> > If you did not mean to close it, let's please reopen it.
> > Even if it is made only a wishlist item, it is a useful enhancement
> > request.
> 
> Can we please follow up on this?  The status seems to be `wishlist' but
> tagged `wontfix', which doesn't make a lot of sense to me.
> 
> I cannot build Emacs to test this.  Could someone else please test it?  Or
> could it please be installed without testing?  (Seriously.)
> 
> It would _really_ be helpful if there were no binary crap in Lisp
> backtraces.  Does that stuff actually help anyone?  If so, perhaps we can
> keep it as a (non-default) option, but otherwise, can't we simply elide
> anything that is not a printable character, at the least?
> 
> I mean replace it by `...', not just change a `display' property.  The
> problems
> I encounter arise from trying to copy + paste the backtrace.
> 
> I don't understand why we even have backtraces that one cannot copy & paste
> completely, into, e.g., an email.  What's the point of that?  If I try to
> paste a copied backtrace I need to paste bits of it piecemeal, because the
> binary parts do not paste.  That is tedious and error prone.  Many users
> might not even realize that the backtrace did not get completely pasted.
> 
> Why is it so hard to advance on something like this?  Stefan provided a C
> patch to test, and that was the end of the thread.
> 
> So much stuff gets added to the Emacs C sources anyway, sometimes breaking
> all kinds of stuff.  Why don't you please just go ahead and install your
> patch, Stefan, so we can see whether and how much it helps?
> 
> Please consider trying to do something to advance this schmilblick.  I am
> sure that it will be appreciated by more than just me.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> You can try the simple patch below.  It doesn't cut it for me, and
> I think the only way to make it work well would be to change the
> representation of the byte-codes so that they're not just a "unibyte
> string" but an object with a distinctive type: the patch only catches
> the case where the byte-codes appear within a printed
> byte-compiled-function, not when they're arguments to the `byte-code'
> function or to the `make-byte-code' function, and I'm sure there can be
> other cases.

The patch doesn't seem to do what we want.

If we don't have debug-on-error, it's OK:

eval: Wrong number of arguments: #[(&optional handle) "..<bytecode>.." [article-buffer handle temp-buffer coding-system-for-read coding-system-for-write default-process-coding-system mm-dissect-buffer t buffer-name generate-new-buffer ...] 28], 2

But with debug-on-error, the backtrace is as byte-ey as ever:

Debugger entered--Lisp error: (wrong-number-of-arguments #[(&optional handle) "p	\204
\306\307!\214``}\210\212	\211@\203\245\310	@!\203\245\311\312!r
q\210\313\216\314 \210\315	@!\210\316\317	8	\211@;\203A	@\202E	A@@)\"\210\320\211\x1c\v\fB\321	A@\322\"\211\x12\203}\323\x12\324\307#\211\x12\203}\x0e\325=\204}\326\327 \x12\"\330 \210\331 \210c\210\332ed\333\324\b\324\334\335\336\337\340\337\341\342\341\343\341\344\345\346\347-\"\350\346\347.\"\341\351\352\353&\210.\a*\354 *\207" [article-buffer handle temp-buffer coding-system-for-read coding-system-for-write default-process-coding-system mm-dissect-buffer t buffer-name generate-new-buffer " *temp*" #[nil "\301\b!\205	\302\b!\207" [temp-buffer buffer-name kill-buffer] 2] mm-disable-multibyte insert-buffer-substring mm-decode-content-transfer-encoding 2 utf-8 mail-content-type-get charset mm-charset-to-coding-system nil ascii decode-coding-string buffer-string erase-buffer mm-enable-multibyte call-process-region "w3m" "-halfdump" "-no-cookie" "-I" "UTF-8" "-O" "-o" "ext_halfdump=1" "display_ins_del=2" "pre_conv=1" "-t" format "%s" "-cols" "display_image=on" "-T" "text/html" gnus-html-wash-tags tab-width gnus-html-frame-width] 28] 2)
  gnus-article-html(1 2)
  eval((gnus-article-html 1 2) nil)
  eval-expression((gnus-article-html 1 2) nil)
  funcall-interactively(eval-expression (gnus-article-html 1 2) nil)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

Why is this bug classified as "Won't Fix"?  Isn't this something
we would all like to see fixed?  Not fixing today does not mean
that it should not be fixed.

What's more, _users_ currently do the work by hand, so it must
be possible to at least partly (probably fully) get it done
by program.  If users can manually (time-consuming and error-prone)
redact the byte-code when pasting a backtrace into a mail etc.
then that can be done by program.

The fact that no one has done this yet is not a good reason
to close this as "wont-fix".

bug#6991: Please keep bytecode out of *Backtrace* buffers

[John Wiegley]

>>>>> Drew Adams <drew.adams@oracle.com> writes:

> What's more, _users_ currently do the work by hand, so it must be possible
> to at least partly (probably fully) get it done by program. If users can
> manually (time-consuming and error-prone) redact the byte-code when pasting
> a backtrace into a mail etc. then that can be done by program.

Drew, can you show me what it will look like to have the elision performed?
Sometimes the byte-code contains strings that give me a clue as to the
problem, so I'm wondering what will disappear if this is fixed.

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

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

> Drew, can you show me what it will look like to have the elision
> performed? Sometimes the byte-code contains strings that give me
> a clue as to the problem, so I'm wondering what will disappear if
> this is fixed.

Nothing prevents letting a user control the degree of elision.
If someone thinks that part of a #[...] occurrence might be
helpful then a yankable and readable part of it could be included.
In general, my guess is that most #[...] occurrences can just
be removed (replaced by an elision indicator).

I can show what I do, at least:

1. I start by trying to yank a whole backtrace into a bug-report
   buffer.

2. That typically doesn't work completely: some of the yanked
   text is truncated (not yanked) because it is binary stuff
   from byte-code.

3. So I often have to yank separate pieces of a backtrace - parts
   that are yankable (which might still contain some byte-code,
   but byte-code that is yankable).

4. I remove anything that is problematic/meaningless, leaving
   ordinary text that humans can read.  In my experience there
   is nothing in the byte code that is of interest and that
   doesn't also appear anyway in the non-byte-code part of the
   backtrace.

5. I can include some from an Emacs 24.5 backtrace (Emacs 25 is
   unusable for me - crashes within a minute or two, and has
   done so for a couple years now).

6. Caveat: I byte-compile my code with the oldest Emacs version
   that that particular code supports.  That could be Emacs 20,
   22, 23 (or maybe 24 or 25, for some libraries).

Here's a backtrace with some byte-code in it:

Debugger entered--entering a function:
* icicle-ucs-names()
* #[(prompt &optional names) "\b\204


See, only the top two lines got pasted (even into an Outlook
window, and the second line was truncated at the first null
byte (it appears as ^@ in the backtrace, where that is a null
char and not two chars).

The " \204 that you (might) see here looks like this in Emacs:
"^H\204^G^@\306" etc., but each of those ^LETTER occurrences
is a control char, not a doublet with first char ^.

Then I would yank a bit more, not bothering to copy the
same byte-code that appears in the third line:

* apply(#[(prompt &optional names) "\b\204


Then I would copy and paste some more - in this case all of
the rest of the backtrace has no byte-code:

* read-char-by-name("Unicode (name or hex): ")
  (list (read-char-by-name "Unicode (name or hex): ") (prefix-numeric-value current-prefix-arg) t t)
  call-interactively(ucsc-insert nil nil)
  command-execute(ucsc-insert)

Then I would clean up the byte-code that was successfully
yanked, probably replacing it with "...".  I don't have a
special way of doing these things.  I just edit manually,
to give something like this:

Debugger entered--entering a function:
* icicle-ucs-names()
* #[(prompt &optional names) "..." [names cands prompt new-prompt enable-recursive-minibuffers completion-ignore-case icicle-ucs-names delq nil mapcar icicle-make-char-candidate copy-sequence t (1) "	" ((3 (face icicle-candidate-part))) icicle-mctize-all lambda (string pred action) if (eq action (quote metadata)) (quote (metadata (category . unicode-name))) complete-with-action action quote (string pred) completing-read string-match-p "\\`[0-9a-fA-F]+\\'" string-to-number 16 "^#" read assoc-string try-completion icicle-transform-multi-completion (2) get-text-property 0 icicle-whole-candidate characterp error "Invalid character: `%s'" add-to-list icicle-read-char-history icicle-read-char-by-name-multi-completion-flag icicle-show-multi-completion-flag icicle-multi-completing-p icicle-list-use-nth-parts icicle-transform-before-sort-p ...] 10 "Read a character... I'VE ELIDED MOST OF A LONG DOC STRING HERE")
* apply(#[(prompt &optional names) - same as line above.)
* read-char-by-name("Unicode (name or hex): ")
  (list (read-char-by-name "Unicode (name or hex): ") (prefix-numeric-value current-prefix-arg) t t)
  call-interactively(ucsc-insert nil nil)
  command-execute(ucsc-insert) 

In this case I also manually elided a long doc string, not
just byte-code.

Is it worthwhile to keep some of what is inside #[...]?
Here, I did - I removed only the binary code stuff.  But it
might be good in most cases to just elide each occurrence of
#[STUFF].

HTH - Drew

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

It looks like this bug was closed a couple times because
6991-done@debbugs.gnu.org was added to the Cc list.  I've removed it,
and let's try not to add it again.

Drew Adams <drew.adams@oracle.com> writes:
>
> Here's a backtrace with some byte-code in it:
>
> Debugger entered--entering a function:
> * icicle-ucs-names()
> * #[(prompt &optional names) "\b\204
>
>
> See, only the top two lines got pasted (even into an Outlook
> window, and the second line was truncated at the first null
> byte (it appears as ^@ in the backtrace, where that is a null
> char and not two chars).

I would propose something like the below, which will cause the NUL byte
to be rendered as \0 instead of ^@.  We could potentially do this with
other control characters too, if they cause trouble too?

I do think it's worth keeping the bytecode in the backtrace, because
it's not useless: you can run `disassemble' on it and get something
meaningful.  Perhaps hiding the byte code with display properties would
be useful.

modified   src/print.c
@@ -1477,18 +1477,20 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 		     could be taken as part of it,
 		     output `\ ' to prevent that.  */
 		  if (need_nonhex && c_isxdigit (c))
 		    print_c_string ("\\ ", printcharfun);
+                  need_nonhex = false;
 
 		  if (c == '\n' && print_escape_newlines
 		      ? (c = 'n', true)
 		      : c == '\f' && print_escape_newlines
 		      ? (c = 'f', true)
+                      : c == '\0' && print_escape_nonascii
+                      ? (c = '0', need_nonhex = true)
 		      : c == '\"' || c == '\\')
 		    printchar ('\\', printcharfun);
 
 		  printchar (c, printcharfun);
-		  need_nonhex = false;
 		}
 	    }
 	  printchar ('\"', printcharfun);
 

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Drew Adams]

> I would propose something like the below, which will cause the NUL
> byte to be rendered as \0 instead of ^@.  We could potentially do this
> with other control characters too, if they cause trouble too?
> 
> I do think it's worth keeping the bytecode in the backtrace, because
> it's not useless: you can run `disassemble' on it and get something
> meaningful.  Perhaps hiding the byte code with display properties
> would be useful.

Any improvement is an improvement.  Please go for it.  Thx.

That's a simple change that shouldn't introduce new problems.
If others propose more elaborate improvements, they will likely
be independent - this won't bother them.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Date: Fri, 18 Nov 2016 20:55:54 -0500
> Cc: Juanma Barranquero <lekktu@gmail.com>, Lars Ingebrigtsen <larsi@gnus.org>,
> 	John Wiegley <johnw@gnu.org>,
> 	Stefan Monnier <monnier@iro.umontreal.ca>, 6991@debbugs.gnu.org
> 
> Drew Adams <drew.adams@oracle.com> writes:
> >
> > Here's a backtrace with some byte-code in it:
> >
> > Debugger entered--entering a function:
> > * icicle-ucs-names()
> > * #[(prompt &optional names) "\b\204
> >
> >
> > See, only the top two lines got pasted (even into an Outlook
> > window, and the second line was truncated at the first null
> > byte (it appears as ^@ in the backtrace, where that is a null
> > char and not two chars).
> 
> I would propose something like the below, which will cause the NUL byte
> to be rendered as \0 instead of ^@.  We could potentially do this with
> other control characters too, if they cause trouble too?

Isn't the fact that copying text into the clipboard stops at the first
null character a Windows-specific issue?  And if it isn't Windows
specific, isn't it at least specific to selections?

I think Emacs doesn't need this change for all occurrences of the null
byte, because Emacs Lisp strings and buffer text will happily DTRT
with them (they were designed to do so).  So I thin we should only
"fix" this problem where it happens, not in print functions in
general.

> I do think it's worth keeping the bytecode in the backtrace, because
> it's not useless: you can run `disassemble' on it and get something
> meaningful.

Exactly.  But if we change print_object like you suggest, there's no
way of being sure the null bytes won't be mangled by some application
of a print function.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> I would propose something like the below, which will cause the NUL byte
>> to be rendered as \0 instead of ^@.  We could potentially do this with
>> other control characters too, if they cause trouble too?
>
> Isn't the fact that copying text into the clipboard stops at the first
> null character a Windows-specific issue?  And if it isn't Windows
> specific, isn't it at least specific to selections?

It seems to be application specific.  When I copy to a Firefox text area
on GNU/Linux I get a truncated result, but using xclip | od -c, I can
see the NUL byte and following characters are there.

>
> I think Emacs doesn't need this change for all occurrences of the null
> byte, because Emacs Lisp strings and buffer text will happily DTRT
> with them (they were designed to do so).  So I thin we should only
> "fix" this problem where it happens, not in print functions in
> general.

We could try fixing this in `gui-select-text', but the problem there is
that we don't necessarily know that replace NUL with "\0" is valid.

>
> Exactly.  But if we change print_object like you suggest, there's no
> way of being sure the null bytes won't be mangled by some application
> of a print function.

I'm not sure what you mean.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 6991@debbugs.gnu.org,  lekktu@gmail.com,  johnw@gnu.org,  monnier@iro.umontreal.ca,  larsi@gnus.org,  drew.adams@oracle.com
> Date: Sat, 19 Nov 2016 09:39:09 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> I would propose something like the below, which will cause the NUL byte
> >> to be rendered as \0 instead of ^@.  We could potentially do this with
> >> other control characters too, if they cause trouble too?
> >
> > Isn't the fact that copying text into the clipboard stops at the first
> > null character a Windows-specific issue?  And if it isn't Windows
> > specific, isn't it at least specific to selections?
> 
> It seems to be application specific.  When I copy to a Firefox text area
> on GNU/Linux I get a truncated result, but using xclip | od -c, I can
> see the NUL byte and following characters are there.

If this happens on both Windows and X, then both xselect.c and
w32select.c should "encode" null bytes.  Would that solve the problem?

> > Exactly.  But if we change print_object like you suggest, there's no
> > way of being sure the null bytes won't be mangled by some application
> > of a print function.
> 
> I'm not sure what you mean.

A literal string can be printed, and the result is generally the
string itself.  But with your suggestion, the null bytes will be
lossily converted to something else.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 6991@debbugs.gnu.org,  lekktu@gmail.com,  johnw@gnu.org,  monnier@iro.umontreal.ca,  larsi@gnus.org,  drew.adams@oracle.com
>> Date: Sat, 19 Nov 2016 09:39:09 -0500
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> >> 
>> >> I would propose something like the below, which will cause the NUL byte
>> >> to be rendered as \0 instead of ^@.  We could potentially do this with
>> >> other control characters too, if they cause trouble too?
>> >
>> > Isn't the fact that copying text into the clipboard stops at the first
>> > null character a Windows-specific issue?  And if it isn't Windows
>> > specific, isn't it at least specific to selections?
>> 
>> It seems to be application specific.  When I copy to a Firefox text area
>> on GNU/Linux I get a truncated result, but using xclip | od -c, I can
>> see the NUL byte and following characters are there.
>
> If this happens on both Windows and X, then both xselect.c and
> w32select.c should "encode" null bytes.  Would that solve the problem?

When printing a string literal, a null byte can be encoded as "\0", but
in general, when copying an arbitrary piece of text this encoding might
not necessarily be correct.

>
>> > Exactly.  But if we change print_object like you suggest, there's no
>> > way of being sure the null bytes won't be mangled by some application
>> > of a print function.
>> 
>> I'm not sure what you mean.
>
> A literal string can be printed, and the result is generally the
> string itself.  But with your suggestion, the null bytes will be
> lossily converted to something else.

I don't think it's lossy.  Furthermore, newlines and form feeds are
already being converted to "\n" and "\f", respectively.  Bytes higher
than 0x80 are also converted to octal escapes.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 6991@debbugs.gnu.org,  lekktu@gmail.com,  johnw@gnu.org,  monnier@iro.umontreal.ca,  larsi@gnus.org,  drew.adams@oracle.com
> Date: Sat, 19 Nov 2016 10:20:51 -0500
> 
> >> > Isn't the fact that copying text into the clipboard stops at the first
> >> > null character a Windows-specific issue?  And if it isn't Windows
> >> > specific, isn't it at least specific to selections?
> >> 
> >> It seems to be application specific.  When I copy to a Firefox text area
> >> on GNU/Linux I get a truncated result, but using xclip | od -c, I can
> >> see the NUL byte and following characters are there.
> >
> > If this happens on both Windows and X, then both xselect.c and
> > w32select.c should "encode" null bytes.  Would that solve the problem?
> 
> When printing a string literal, a null byte can be encoded as "\0", but
> in general, when copying an arbitrary piece of text this encoding might
> not necessarily be correct.

Not sure what you have in mind.  Can you show an example of when it's
not correct?

At least on MS-Windows, we only support text selections, so doing so
in w32select.c should be TRT, because clipboard text cannot include
null bytes on Windows, AFAIK.  I also think it's TRT elsewhere, when
the selection value is some kind of text.

> > A literal string can be printed, and the result is generally the
> > string itself.  But with your suggestion, the null bytes will be
> > lossily converted to something else.
> 
> I don't think it's lossy.

It's lossy because you can never know whether it came from a null byte
or from a literal ASCII text "\0".

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 6991@debbugs.gnu.org,  lekktu@gmail.com,  johnw@gnu.org,  monnier@iro.umontreal.ca,  larsi@gnus.org,  drew.adams@oracle.com
>> Date: Sat, 19 Nov 2016 10:20:51 -0500
>> 
>> >> > Isn't the fact that copying text into the clipboard stops at the first
>> >> > null character a Windows-specific issue?  And if it isn't Windows
>> >> > specific, isn't it at least specific to selections?
>> >> 
>> >> It seems to be application specific.  When I copy to a Firefox text area
>> >> on GNU/Linux I get a truncated result, but using xclip | od -c, I can
>> >> see the NUL byte and following characters are there.
>> >
>> > If this happens on both Windows and X, then both xselect.c and
>> > w32select.c should "encode" null bytes.  Would that solve the problem?
>> 
>> When printing a string literal, a null byte can be encoded as "\0", but
>> in general, when copying an arbitrary piece of text this encoding might
>> not necessarily be correct.
>
> Not sure what you have in mind.  Can you show an example of when it's
> not correct?

I can't really think of a practical example, but it seems that the same
objection you raised below applies: how would you know whether what was
copied had the literal ASCII text "\0" or a null byte?

>
> At least on MS-Windows, we only support text selections, so doing so
> in w32select.c should be TRT, because clipboard text cannot include
> null bytes on Windows, AFAIK.  I also think it's TRT elsewhere, when
> the selection value is some kind of text.

It doesn't really feel like the Right Thing to me, there's no particular
reason to choose "\0" for this, e.g., why not use "^@" (an ASCII caret
followed by at sign)?  In the case of printing a string literal, it's
established that a backslash means escaping within the double quotes.

>
>> > A literal string can be printed, and the result is generally the
>> > string itself.  But with your suggestion, the null bytes will be
>> > lossily converted to something else.
>> 
>> I don't think it's lossy.
>
> It's lossy because you can never know whether it came from a null byte
> or from a literal ASCII text "\0".

It's already the case that ASCII text "\0" is printed as "\\0", without
my patch:

(print (string 0 ?\\ ?0) (current-buffer))

"^@\\0" ;; I replaced the null byte with "^@" to avoid trouble with
        ;; email clients

With my patch, ^@ is replaced with \0:

(print (string 0 ?\\ ?0) (current-buffer))

"\0\\0"

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 6991@debbugs.gnu.org,  lekktu@gmail.com,  johnw@gnu.org,  monnier@iro.umontreal.ca,  larsi@gnus.org,  drew.adams@oracle.com
> Date: Sat, 19 Nov 2016 17:33:11 -0500
> 
> >> > If this happens on both Windows and X, then both xselect.c and
> >> > w32select.c should "encode" null bytes.  Would that solve the problem?
> >> 
> >> When printing a string literal, a null byte can be encoded as "\0", but
> >> in general, when copying an arbitrary piece of text this encoding might
> >> not necessarily be correct.
> >
> > Not sure what you have in mind.  Can you show an example of when it's
> > not correct?
> 
> I can't really think of a practical example, but it seems that the same
> objection you raised below applies: how would you know whether what was
> copied had the literal ASCII text "\0" or a null byte?

We can't.  But since null bytes cannot be put into the Windows
clipboard, we have only 2 possible ways of action: either replace the
null bytes with something else, or lose everything past the first null
bytes (which is the current behavior, against which this bug report
was submitted).  So if we want to solve this problem, what else can we
do except use some lossy conversion?

> > At least on MS-Windows, we only support text selections, so doing so
> > in w32select.c should be TRT, because clipboard text cannot include
> > null bytes on Windows, AFAIK.  I also think it's TRT elsewhere, when
> > the selection value is some kind of text.
> 
> It doesn't really feel like the Right Thing to me, there's no particular
> reason to choose "\0" for this, e.g., why not use "^@" (an ASCII caret
> followed by at sign)?

If you thought I was arguing against ^@ and for \0, then this is a
misunderstanding: I don't really care one way of the other.  I was
saying that we must do _something_ to replace those null bytes, if we
want the text beyond the first one be seen in the application into
which you paste.

> > It's lossy because you can never know whether it came from a null byte
> > or from a literal ASCII text "\0".
> 
> It's already the case that ASCII text "\0" is printed as "\\0", without
> my patch:
> 
> (print (string 0 ?\\ ?0) (current-buffer))
> 
> "^@\\0" ;; I replaced the null byte with "^@" to avoid trouble with
>         ;; email clients
> 
> With my patch, ^@ is replaced with \0:
> 
> (print (string 0 ?\\ ?0) (current-buffer))
> 
> "\0\\0"

It just doesn't feel right to me to fix a problem that is specific to
selections in a general-purpose low-level facility for printing
strings.  Emacs can handle null bytes in strings very well, so I see
no need to change the print functions.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Noam Postavsky]

On Sun, Nov 20, 2016 at 10:46 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> It just doesn't feel right to me to fix a problem that is specific to
> selections in a general-purpose low-level facility for printing
> strings.  Emacs can handle null bytes in strings very well, so I see
> no need to change the print functions.

Does the fact that this replacement would only happen when
`print-escape-nonascii' is non-nil help at all? And the fact that this
same function already escapes newline, formfeed, and every character
larger than 0x80 (all of which Emacs can handle in strings too)?

Can we have both solutions? The selection fix is lossy, so avoiding
the need for it where possible seems like a good thing to me.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Tue, 22 Nov 2016 13:07:13 -0500
> Cc: 6991@debbugs.gnu.org, Juanma Barranquero <lekktu@gmail.com>, John Wiegley <johnw@gnu.org>, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, Lars Magne Ingebrigtsen <larsi@gnus.org>, 
> 	Drew Adams <drew.adams@oracle.com>
> 
> On Sun, Nov 20, 2016 at 10:46 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > It just doesn't feel right to me to fix a problem that is specific to
> > selections in a general-purpose low-level facility for printing
> > strings.  Emacs can handle null bytes in strings very well, so I see
> > no need to change the print functions.
> 
> Does the fact that this replacement would only happen when
> `print-escape-nonascii' is non-nil help at all? And the fact that this
> same function already escapes newline, formfeed, and every character
> larger than 0x80 (all of which Emacs can handle in strings too)?
> 
> Can we have both solutions? The selection fix is lossy, so avoiding
> the need for it where possible seems like a good thing to me.

I'm confused: which problem the above is supposed to fix?  Are we
still talking about putting null bytes in selections, or are we
talking about something else?

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Noam Postavsky]

On Tue, Nov 22, 2016 at 1:52 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Tue, 22 Nov 2016 13:07:13 -0500
>> Cc: 6991@debbugs.gnu.org, Juanma Barranquero <lekktu@gmail.com>, John Wiegley <johnw@gnu.org>,
>>       Stefan Monnier <monnier@iro.umontreal.ca>, Lars Magne Ingebrigtsen <larsi@gnus.org>,
>>       Drew Adams <drew.adams@oracle.com>
>>
>> On Sun, Nov 20, 2016 at 10:46 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >
>> > It just doesn't feel right to me to fix a problem that is specific to
>> > selections in a general-purpose low-level facility for printing
>> > strings.  Emacs can handle null bytes in strings very well, so I see
>> > no need to change the print functions.
>>
>> Does the fact that this replacement would only happen when
>> `print-escape-nonascii' is non-nil help at all? And the fact that this
>> same function already escapes newline, formfeed, and every character
>> larger than 0x80 (all of which Emacs can handle in strings too)?
>>
>> Can we have both solutions? The selection fix is lossy, so avoiding
>> the need for it where possible seems like a good thing to me.
>
> I'm confused: which problem the above is supposed to fix?  Are we
> still talking about putting null bytes in selections, or are we
> talking about something else?

The original bug report is about copying backtraces containing byte
code to other applications (e.g., web browser, mail client, etc). The
byte code in backtraces is currently printed with several characters
backslash escaped (newline, formfeed, backslash, double quote, and
characters higher than 0x80). I propose to extend this escaping to
null bytes as well. That will (somewhat indirectly) solve the problem
of copying backtraces to other applications, without lossyness (i.e.,
(equal (read (print str)) str) remains true). It won't solve the
problem of copying arbitrary text containing null bytes to other
applications, it only avoids the most common case of the user needing
to copy text containing null bytes.

So in addition to that, your proposal to escape null bytes in xselect
and w32select would still be needed to cover the general case. The
drawback to replacing nulls in the {x,w32}select code is that the
conversion is lossy, and there is a slightly increased chance of the
user not noticing there was lossy conversion (relative to the current
lossy "conversion" of truncating the string).

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Tue, 22 Nov 2016 16:07:06 -0500
> Cc: 6991@debbugs.gnu.org, Juanma Barranquero <lekktu@gmail.com>, John Wiegley <johnw@gnu.org>, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, Lars Magne Ingebrigtsen <larsi@gnus.org>, 
> 	Drew Adams <drew.adams@oracle.com>
> 
> > I'm confused: which problem the above is supposed to fix?  Are we
> > still talking about putting null bytes in selections, or are we
> > talking about something else?
> 
> The original bug report is about copying backtraces containing byte
> code to other applications (e.g., web browser, mail client, etc). The
> byte code in backtraces is currently printed with several characters
> backslash escaped (newline, formfeed, backslash, double quote, and
> characters higher than 0x80). I propose to extend this escaping to
> null bytes as well. That will (somewhat indirectly) solve the problem
> of copying backtraces to other applications, without lossyness (i.e.,
> (equal (read (print str)) str) remains true). It won't solve the
> problem of copying arbitrary text containing null bytes to other
> applications, it only avoids the most common case of the user needing
> to copy text containing null bytes.

I'm not necessarily opposed, but I never had any problems with binary
nulls, except when copying to clipboard.

> So in addition to that, your proposal to escape null bytes in xselect
> and w32select would still be needed to cover the general case. The
> drawback to replacing nulls in the {x,w32}select code is that the
> conversion is lossy, and there is a slightly increased chance of the
> user not noticing there was lossy conversion (relative to the current
> lossy "conversion" of truncating the string).

Yes, it's lossy, but what other alternative do we have, except losing
much more?

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Tue, 22 Nov 2016 16:07:06 -0500
>> Cc: 6991@debbugs.gnu.org, Juanma Barranquero <lekktu@gmail.com>, John Wiegley <johnw@gnu.org>, 
>> 	Stefan Monnier <monnier@iro.umontreal.ca>, Lars Magne Ingebrigtsen <larsi@gnus.org>, 
>> 	Drew Adams <drew.adams@oracle.com>
>> 
>> > I'm confused: which problem the above is supposed to fix?  Are we
>> > still talking about putting null bytes in selections, or are we
>> > talking about something else?
>> 
>> The original bug report is about copying backtraces containing byte
>> code to other applications (e.g., web browser, mail client, etc). The
>> byte code in backtraces is currently printed with several characters
>> backslash escaped (newline, formfeed, backslash, double quote, and
>> characters higher than 0x80). I propose to extend this escaping to
>> null bytes as well. That will (somewhat indirectly) solve the problem
>> of copying backtraces to other applications, without lossyness (i.e.,
>> (equal (read (print str)) str) remains true). It won't solve the
>> problem of copying arbitrary text containing null bytes to other
>> applications, it only avoids the most common case of the user needing
>> to copy text containing null bytes.
>
> I'm not necessarily opposed, but I never had any problems with binary
> nulls, except when copying to clipboard.

I've never needed to copy binary nulls except when a backtrace had one.

>
>> So in addition to that, your proposal to escape null bytes in xselect
>> and w32select would still be needed to cover the general case. The
>> drawback to replacing nulls in the {x,w32}select code is that the
>> conversion is lossy, and there is a slightly increased chance of the
>> user not noticing there was lossy conversion (relative to the current
>> lossy "conversion" of truncating the string).
>
> Yes, it's lossy, but what other alternative do we have, except losing
> much more?

Yes, there's no perfect solution.  That's why I prefer to solve just the
immediate problem by extending the escaping in `print' to cover null
bytes.  And this will keep working if, for example, we make the general
case of copying null bytes to clipboard use a customizable replacement.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Stefan Monnier]

> Yes, there's no perfect solution.  That's why I prefer to solve just the
> immediate problem by extending the escaping in `print' to cover null
> bytes.

FWIW, I agree.  The only issue I can see here is that depending on how
it's done, it could affect the size of the .elc files (by using two bytes
per bytecode 0 rather than 1).  Nothing too terrible, but it's probably
worth checking whether it does make such a difference and if so how serious
it is.


        Stefan

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:
> FWIW, I agree.  The only issue I can see here is that depending on how
> it's done, it could affect the size of the .elc files (by using two bytes
> per bytecode 0 rather than 1).  Nothing too terrible, but it's probably
> worth checking whether it does make such a difference and if so how serious
> it is.

On average it increases Emacs' .elc by ~200 bytes each, 276kB in total,
or 100.62%.


[-- Attachment #2: table of size changes for print NUL as \0 --]
[-- Type: application/octet-stream, Size: 28222 bytes --]

[-- Attachment #3: Type: text/plain, Size: 752 bytes --]


But actually, while looking at this, I understood more about what the
print_escape_nonascii flag is used for (i.e., multibyte vs unibyte
stuff), and I no longer think it makes sense for it to affect printing
the NUL byte anyway.

I propose adding a new flag print_escape_control_characters instead (see
patch #3 in the series).  I also implemented hiding the byte code
functions with text properties in #4.  It's not quite satisfactory
though, because it doesn't cover byte code functions values that are
arguments, only byte code being called.  I think printing needs to be
made more flexible in order to cleanly catch all byte code values.
Patch #5 replaces NUL bytes with "\0" in X selections (I guess it covers
w32 as well? Haven't checked yet).


[-- Attachment #4: screenshot --]
[-- Type: image/png, Size: 69444 bytes --]

[-- Attachment #5: patch --]
[-- Type: text/plain, Size: 5288 bytes --]

From 622b9b1f328cfbf527e0dd350fd45566f4716849 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 09:19:00 -0500
Subject: [PATCH v1 1/5] Operate on frame list instead of printed backtrace

* lisp/emacs-lisp/debug.el (debugger--insert-backtrace): New function,
prints the given backtrace frames.
(debugger-setup-buffer): Use it instead of editing the backtrace
buffer text.
---
 lisp/emacs-lisp/debug.el | 84 +++++++++++++++++++++++++++---------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index cb77148c28..f24f62fa71 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -264,6 +264,40 @@ debug
       (setq debug-on-next-call debugger-step-after-exit)
       debugger-value)))
 \f
+
+(defun debugger-insert-backtrace (frames do-xrefs)
+  "Format and insert the backtrace FRAMES at point.
+Make functions into cross-reference buttons if DO-XREFS is non-nil."
+  (let ((standard-output (current-buffer))
+        (eval-buffers eval-buffer-list))
+    (require 'help-mode)     ; Define `help-function-def' button type.
+    (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
+      (insert (if (plist-get flags :debug-on-exit)
+                  "* " "  "))
+      (let ((fun-file (and do-xrefs (symbol-file fun 'defun)))
+            (fun-pt (point)))
+        (cond
+         ((and evald (not debugger-stack-frame-as-list))
+          (prin1 fun)
+          (if args (prin1 args) (princ "()")))
+         (t
+          (prin1 (cons fun args))
+          (cl-incf fun-pt)))
+        (when fun-file
+          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
+                            :type 'help-function-def
+                            'help-args (list fun fun-file))))
+      ;; After any frame that uses eval-buffer, insert a line that
+      ;; states the buffer position it's reading at.
+      (when (and eval-buffers (memq fun '(eval-buffer eval-region)))
+        (insert (format "  ; Reading at buffer position %d"
+                        ;; This will get the wrong result if there are
+                        ;; two nested eval-region calls for the same
+                        ;; buffer.  That's not a very useful case.
+                        (with-current-buffer (pop eval-buffers)
+                          (point)))))
+      (insert "\n"))))
+
 (defun debugger-setup-buffer (args)
   "Initialize the `*Backtrace*' buffer for entry to the debugger.
 That buffer should be current already."
@@ -271,22 +305,6 @@ debugger-setup-buffer
   (erase-buffer)
   (set-buffer-multibyte t)		;Why was it nil ?  -stef
   (setq buffer-undo-list t)
-  (let ((standard-output (current-buffer))
-	(print-escape-newlines t)
-	(print-level 8)
-        (print-length 50))
-    ;; FIXME the debugger could pass a custom callback to mapbacktrace
-    ;; instead of manipulating printed results.
-    (mapbacktrace #'backtrace--print-frame 'debug))
-  (goto-char (point-min))
-  (delete-region (point)
-		 (progn
-                   (forward-line (if (eq (car args) 'debug)
-                                     ;; Remove debug--implement-debug-on-entry
-                                     ;; and the advice's `apply' frame.
-				     3
-				   1))
-		   (point)))
   (insert "Debugger entered")
   ;; lambda is for debug-on-call when a function call is next.
   ;; debug is for debug-on-entry function called.
@@ -301,10 +319,7 @@ debugger-setup-buffer
        (setq pos (point))
        (setq debugger-value (nth 1 args))
        (prin1 debugger-value (current-buffer))
-       (insert ?\n)
-       (delete-char 1)
-       (insert ? )
-       (beginning-of-line))
+       (insert ?\n))
       ;; Watchpoint triggered.
       ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
        (insert
@@ -341,23 +356,20 @@ debugger-setup-buffer
                   (cdr args) args)
               (current-buffer))
        (insert ?\n)))
+    (let ((frames (nthcdr
+                   ;; Remove debug--implement-debug-on-entry and the
+                   ;; advice's `apply' frame.
+                   (if (eq (car args) 'debug) 3 1)
+                   (backtrace-frames 'debug)))
+          (print-escape-newlines t)
+          (print-level 8)
+          (print-length 50))
+      (when (eq (car args) 'exit)
+        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))
+      (debugger-insert-backtrace frames t))
     ;; Place point on "stack frame 0" (bug#15101).
-    (goto-char pos))
-  ;; After any frame that uses eval-buffer,
-  ;; insert a line that states the buffer position it's reading at.
-  (save-excursion
-    (let ((tem eval-buffer-list))
-      (while (and tem
-		  (re-search-forward "^  eval-\\(buffer\\|region\\)(" nil t))
-	(end-of-line)
-	(insert (format "  ; Reading at buffer position %d"
-			;; This will get the wrong result
-			;; if there are two nested eval-region calls
-			;; for the same buffer.  That's not a very useful case.
-			(with-current-buffer (car tem)
-			  (point))))
-	(pop tem))))
-  (debugger-make-xrefs))
+    (goto-char pos)))
+
 
 (defun debugger-make-xrefs (&optional buffer)
   "Attach cross-references to function names in the `*Backtrace*' buffer."
-- 
2.11.1


[-- Attachment #6: patch --]
[-- Type: text/plain, Size: 6872 bytes --]

From 356c3e17edb19f5daef9f83f2c4d5694290f1221 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 17:19:41 -0500
Subject: [PATCH v1 2/5] Improve ert backtrace recording

Change ert to use the new `backtrace-frames' function instead of
collecting frames one by one with `backtrace-frame'.  Additionally,
collect frames starting from `signal' instead the somewhat arbitrary
"6 from the bottom".  Skipping 6 frames would skip the expression that
actually caused the signal that triggered the debugger.  Possibly 6
was chosen because in the case of a failed test, the triggering frame
is an `ert-fail' call, which is not so interesting.  But in case of
test throwing an error, this drop the `error' call which is too much.

* lisp/emacs-lisp/ert.el (ert--print-backtrace): Remove.
(ert--print-backtrace): Add DO-XREFS parameter, delegate to
`debugger-insert-backtrace'.
(ert--run-test-debugger): Record the backtrace frames starting from
the instigating `signal' call.
(ert-run-tests-batch): Pass nil for `ert--print-backtrace's new
DO-XREFS parameter.
(ert-results-pop-to-backtrace-for-test-at-point): Pass t as DO-XREFS
to `ert--print-backtrace' and remove call to `debugger-make-xrefs'.
* test/lisp/emacs-lisp/ert-tests.el (ert-test-record-backtrace): Check
the backtrace list instead of comparing its string representation.
Expect `signal' to be the first frame.
---
 lisp/emacs-lisp/ert.el            | 62 ++++++++++++---------------------------
 test/lisp/emacs-lisp/ert-tests.el |  8 ++---
 2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 785f4aca1c..98e4cd7c24 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -669,48 +669,12 @@ ert-debug-on-error
 (cl-defstruct (ert-test-aborted-with-non-local-exit
                (:include ert-test-result)))
 
-
-(defun ert--record-backtrace ()
-  "Record the current backtrace (as a list) and return it."
-  ;; Since the backtrace is stored in the result object, result
-  ;; objects must only be printed with appropriate limits
-  ;; (`print-level' and `print-length') in place.  For interactive
-  ;; use, the cost of ensuring this possibly outweighs the advantage
-  ;; of storing the backtrace for
-  ;; `ert-results-pop-to-backtrace-for-test-at-point' given that we
-  ;; already have `ert-results-rerun-test-debugging-errors-at-point'.
-  ;; For batch use, however, printing the backtrace may be useful.
-  (cl-loop
-   ;; 6 is the number of frames our own debugger adds (when
-   ;; compiled; more when interpreted).  FIXME: Need to describe a
-   ;; procedure for determining this constant.
-   for i from 6
-   for frame = (backtrace-frame i)
-   while frame
-   collect frame))
-
-(defun ert--print-backtrace (backtrace)
+(defun ert--print-backtrace (backtrace do-xrefs)
   "Format the backtrace BACKTRACE to the current buffer."
-  ;; This is essentially a reimplementation of Fbacktrace
-  ;; (src/eval.c), but for a saved backtrace, not the current one.
   (let ((print-escape-newlines t)
         (print-level 8)
         (print-length 50))
-    (dolist (frame backtrace)
-      (pcase-exhaustive frame
-        (`(nil ,special-operator . ,arg-forms)
-         ;; Special operator.
-         (insert
-          (format "  %S\n" (cons special-operator arg-forms))))
-        (`(t ,fn . ,args)
-         ;; Function call.
-         (insert (format "  %S(" fn))
-         (cl-loop for firstp = t then nil
-                  for arg in args do
-                  (unless firstp
-                    (insert " "))
-                  (insert (format "%S" arg)))
-         (insert ")\n"))))))
+    (debugger-insert-backtrace backtrace do-xrefs)))
 
 ;; A container for the state of the execution of a single test and
 ;; environment data needed during its execution.
@@ -749,7 +713,19 @@ ert--run-test-debugger
                       ((quit) 'quit)
 		      ((ert-test-skipped) 'skipped)
                       (otherwise 'failed)))
-              (backtrace (ert--record-backtrace))
+              ;; We store the backtrace in the result object for
+              ;; `ert-results-pop-to-backtrace-for-test-at-point'.
+              ;; This means we have to limit `print-level' and
+              ;; `print-length' when printing result objects.  That
+              ;; might not be worth while when we can also use
+              ;; `ert-results-rerun-test-debugging-errors-at-point',
+              ;; (i.e., when running interactively) but having the
+              ;; backtrace ready for printing is important for batch
+              ;; use.
+              ;;
+              ;; Grab the frames starting from `signal', frames below
+              ;; that are all from the debugger.
+              (backtrace (backtrace-frames 'signal))
               (infos (reverse ert--infos)))
          (setf (ert--test-execution-info-result info)
                (cl-ecase type
@@ -1404,8 +1380,9 @@ ert-run-tests-batch
               (ert-test-result-with-condition
                (message "Test %S backtrace:" (ert-test-name test))
                (with-temp-buffer
-                 (ert--print-backtrace (ert-test-result-with-condition-backtrace
-                                        result))
+                 (ert--print-backtrace
+                  (ert-test-result-with-condition-backtrace result)
+                  nil)
                  (goto-char (point-min))
                  (while (not (eobp))
                    (let ((start (point))
@@ -2394,8 +2371,7 @@ ert-results-pop-to-backtrace-for-test-at-point
            ;; Use unibyte because `debugger-setup-buffer' also does so.
            (set-buffer-multibyte nil)
            (setq truncate-lines t)
-           (ert--print-backtrace backtrace)
-           (debugger-make-xrefs)
+           (ert--print-backtrace backtrace t)
            (goto-char (point-min))
            (insert (substitute-command-keys "Backtrace for test `"))
            (ert-insert-test-name-button (ert-test-name test))
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index fc5790c365..317838b250 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -367,12 +367,8 @@ ert-test--which-file
          (test (make-ert-test :body test-body))
          (result (ert-run-test test)))
     (should (ert-test-failed-p result))
-    (with-temp-buffer
-      (ert--print-backtrace (ert-test-failed-backtrace result))
-      (goto-char (point-min))
-      (end-of-line)
-      (let ((first-line (buffer-substring-no-properties (point-min) (point))))
-        (should (equal first-line (format "  %S()" test-body)))))))
+    (should (eq (nth 1 (car (ert-test-failed-backtrace result)))
+                'signal))))
 
 (ert-deftest ert-test-messages ()
   :tags '(:causes-redisplay)
-- 
2.11.1


[-- Attachment #7: patch --]
[-- Type: text/plain, Size: 4592 bytes --]

From 85bcf94d7bcf55e718f80fb22a3e0c8e351b0787 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 18:13:54 -0500
Subject: [PATCH v1 3/5] Escape control characters in backtraces (Bug#6991)

* src/print.c (syms_of_print): Add new variable,
print-escape-control-characters.
(print_object): Print control characters with octal escape codes when
print-escape-control-characters is true.
* lisp/subr.el (backtrace):
* lisp/emacs-lisp/debug.el (debugger-setup-buffer): Bind
`print-escape-control-characters' to t.
---
 lisp/emacs-lisp/debug.el |  2 +-
 lisp/subr.el             |  3 ++-
 src/print.c              | 40 +++++++++++++++++++++++++++++-----------
 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index f24f62fa71..42e44761de 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -361,7 +361,7 @@ debugger-setup-buffer
                    ;; advice's `apply' frame.
                    (if (eq (car args) 'debug) 3 1)
                    (backtrace-frames 'debug)))
-          (print-escape-newlines t)
+          (print-escape-control-characters t)
           (print-level 8)
           (print-length 50))
       (when (eq (car args) 'exit)
diff --git a/lisp/subr.el b/lisp/subr.el
index a204577ddf..6e4779ef1f 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4472,7 +4472,8 @@ backtrace--print-frame
 (defun backtrace ()
   "Print a trace of Lisp function calls currently active.
 Output stream used is value of `standard-output'."
-  (let ((print-level (or print-level 8)))
+  (let ((print-level (or print-level 8))
+        (print-escape-control-characters t))
     (mapbacktrace #'backtrace--print-frame 'backtrace)))
 
 (defun backtrace-frames (&optional base)
diff --git a/src/print.c b/src/print.c
index db3d00f51f..1d3958a39e 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1476,17 +1476,29 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 		  /* If we just had a hex escape, and this character
 		     could be taken as part of it,
 		     output `\ ' to prevent that.  */
-		  if (need_nonhex && c_isxdigit (c))
-		    print_c_string ("\\ ", printcharfun);
-
-		  if (c == '\n' && print_escape_newlines
-		      ? (c = 'n', true)
-		      : c == '\f' && print_escape_newlines
-		      ? (c = 'f', true)
-		      : c == '\"' || c == '\\')
-		    printchar ('\\', printcharfun);
-
-		  printchar (c, printcharfun);
+                  if (c_isxdigit (c))
+                    {
+                      if (need_nonhex)
+                        print_c_string ("\\ ", printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (c == '\n' && print_escape_newlines
+                           ? (c = 'n', true)
+                           : c == '\f' && print_escape_newlines
+                           ? (c = 'f', true)
+                           : c == '\"' || c == '\\')
+                    {
+                      printchar ('\\', printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (print_escape_control_characters && c_iscntrl (c))
+                    {
+                      char outbuf[5];
+                      int len = sprintf (outbuf, "\\%03o", c + 0u);
+                      strout (outbuf, len, len, printcharfun);
+                    }
+                  else
+                    printchar (c, printcharfun);
 		  need_nonhex = false;
 		}
 	    }
@@ -2264,6 +2276,11 @@ syms_of_print (void)
 Also print formfeeds as `\\f'.  */);
   print_escape_newlines = 0;
 
+  DEFVAR_BOOL ("print-escape-control-characters", print_escape_control_characters,
+	       doc: /* Non-nil means print control characters in strings as `\\OOO'.
+\(OOO is the octal representation of the character code.)*/);
+  print_escape_control_characters = 0;
+
   DEFVAR_BOOL ("print-escape-nonascii", print_escape_nonascii,
 	       doc: /* Non-nil means print unibyte non-ASCII chars in strings as \\OOO.
 \(OOO is the octal representation of the character code.)
@@ -2352,6 +2369,7 @@ representation) and `#N#' in place of each subsequent occurrence,
   DEFSYM (Qprint_escape_newlines, "print-escape-newlines");
   DEFSYM (Qprint_escape_multibyte, "print-escape-multibyte");
   DEFSYM (Qprint_escape_nonascii, "print-escape-nonascii");
+  DEFSYM (Qprint_escape_control_characters, "print-escape-control-characters");
 
   print_prune_charset_plist = Qnil;
   staticpro (&print_prune_charset_plist);
-- 
2.11.1


[-- Attachment #8: patch --]
[-- Type: text/plain, Size: 1967 bytes --]

From bbec49e74acae529ddac91a23a9c81af53454e7e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 19:00:19 -0500
Subject: [PATCH v1 4/5] Hide byte code in backtraces (Bug#6991)

* lisp/emacs-lisp/debug.el (help-byte-code): New button type, calls
`disassemble' in its action.
(debugger-insert-backtrace): Cover byte code with help-byte-code
button.
---
 lisp/emacs-lisp/debug.el | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 42e44761de..94b683dcb9 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -265,6 +265,12 @@ debug
       debugger-value)))
 \f
 
+(define-button-type 'help-byte-code
+  'follow-link t
+  'action (lambda (button)
+            (disassemble (button-get button 'byte-code-function)))
+  'help-echo (purecopy "mouse-2, RET: disassemble this function"))
+
 (defun debugger-insert-backtrace (frames do-xrefs)
   "Format and insert the backtrace FRAMES at point.
 Make functions into cross-reference buttons if DO-XREFS is non-nil."
@@ -286,7 +292,15 @@ debugger-insert-backtrace
         (when fun-file
           (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
                             :type 'help-function-def
-                            'help-args (list fun fun-file))))
+                            'help-args (list fun fun-file)))
+        (when (byte-code-function-p fun)
+          (goto-char fun-pt)
+          (forward-sexp)
+          (make-text-button fun-pt (point)
+                            :type 'help-byte-code
+                            'byte-code-function fun
+                            'display "#<compiled function>")
+          (end-of-line)))
       ;; After any frame that uses eval-buffer, insert a line that
       ;; states the buffer position it's reading at.
       (when (and eval-buffers (memq fun '(eval-buffer eval-region)))
-- 
2.11.1


[-- Attachment #9: patch --]
[-- Type: text/plain, Size: 802 bytes --]

From 4f49294621e64a20462c369499d128523bf4a5de Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 19:47:55 -0500
Subject: [PATCH v1 5/5] Escape NUL bytes in X selections (Bug#6991)

* lisp/select.el (xselect--encode-string): Replace NUL bytes with
"\0".
---
 lisp/select.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/select.el b/lisp/select.el
index a4a79255a9..705d6469d3 100644
--- a/lisp/select.el
+++ b/lisp/select.el
@@ -475,6 +475,9 @@ xselect--encode-string
 	   (t
 	    (error "Unknown selection type: %S" type)))))
 
+      ;; Most programs are unable to handle NUL bytes in strings.
+      (setq str (replace-regexp-in-string "\0" "\\0" str t t))
+
       (setq next-selection-coding-system nil)
       (cons type str))))
 
-- 
2.11.1

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

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

npostavs@users.sourceforge.net writes:

> I propose adding a new flag print_escape_control_characters instead (see
> patch #3 in the series).  I also implemented hiding the byte code
> functions with text properties in #4.  It's not quite satisfactory
> though, because it doesn't cover byte code functions values that are
> arguments, only byte code being called.  I think printing needs to be
> made more flexible in order to cleanly catch all byte code values.
> Patch #5 replaces NUL bytes with "\0" in X selections (I guess it covers
> w32 as well? Haven't checked yet).

Updated the patchset to use cl-prin1, now it applies to function values
in arguments as well.  This one actually doesn't include the byte code
text at all (path of least resistance: cl-print-object wasn't already
omitting bytecode and I haven't bothered adding it).


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

From 92415966acf8ba30869b11d7ee81e88d96253a40 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 09:19:00 -0500
Subject: [PATCH v2 1/6] Operate on frame list instead of printed backtrace

* lisp/emacs-lisp/debug.el (debugger--insert-backtrace): New function,
prints the given backtrace frames.
(debugger-setup-buffer): Use it instead of editing the backtrace
buffer text.
---
 lisp/emacs-lisp/debug.el | 84 +++++++++++++++++++++++++++---------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 83456fc31a..0c8306d428 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -264,6 +264,40 @@ (defun debug (&rest args)
       (setq debug-on-next-call debugger-step-after-exit)
       debugger-value)))
 \f
+
+(defun debugger-insert-backtrace (frames do-xrefs)
+  "Format and insert the backtrace FRAMES at point.
+Make functions into cross-reference buttons if DO-XREFS is non-nil."
+  (let ((standard-output (current-buffer))
+        (eval-buffers eval-buffer-list))
+    (require 'help-mode)     ; Define `help-function-def' button type.
+    (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
+      (insert (if (plist-get flags :debug-on-exit)
+                  "* " "  "))
+      (let ((fun-file (and do-xrefs (symbol-file fun 'defun)))
+            (fun-pt (point)))
+        (cond
+         ((and evald (not debugger-stack-frame-as-list))
+          (prin1 fun)
+          (if args (prin1 args) (princ "()")))
+         (t
+          (prin1 (cons fun args))
+          (cl-incf fun-pt)))
+        (when fun-file
+          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
+                            :type 'help-function-def
+                            'help-args (list fun fun-file))))
+      ;; After any frame that uses eval-buffer, insert a line that
+      ;; states the buffer position it's reading at.
+      (when (and eval-buffers (memq fun '(eval-buffer eval-region)))
+        (insert (format "  ; Reading at buffer position %d"
+                        ;; This will get the wrong result if there are
+                        ;; two nested eval-region calls for the same
+                        ;; buffer.  That's not a very useful case.
+                        (with-current-buffer (pop eval-buffers)
+                          (point)))))
+      (insert "\n"))))
+
 (defun debugger-setup-buffer (args)
   "Initialize the `*Backtrace*' buffer for entry to the debugger.
 That buffer should be current already."
@@ -271,22 +305,6 @@ (defun debugger-setup-buffer (args)
   (erase-buffer)
   (set-buffer-multibyte t)		;Why was it nil ?  -stef
   (setq buffer-undo-list t)
-  (let ((standard-output (current-buffer))
-	(print-escape-newlines t)
-	(print-level 8)
-        (print-length 50))
-    ;; FIXME the debugger could pass a custom callback to mapbacktrace
-    ;; instead of manipulating printed results.
-    (mapbacktrace #'backtrace--print-frame 'debug))
-  (goto-char (point-min))
-  (delete-region (point)
-		 (progn
-                   (forward-line (if (eq (car args) 'debug)
-                                     ;; Remove debug--implement-debug-on-entry
-                                     ;; and the advice's `apply' frame.
-				     3
-				   1))
-		   (point)))
   (insert "Debugger entered")
   ;; lambda is for debug-on-call when a function call is next.
   ;; debug is for debug-on-entry function called.
@@ -301,10 +319,7 @@ (defun debugger-setup-buffer (args)
        (setq pos (point))
        (setq debugger-value (nth 1 args))
        (prin1 debugger-value (current-buffer))
-       (insert ?\n)
-       (delete-char 1)
-       (insert ? )
-       (beginning-of-line))
+       (insert ?\n))
       ;; Watchpoint triggered.
       ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
        (insert
@@ -341,23 +356,20 @@ (defun debugger-setup-buffer (args)
                   (cdr args) args)
               (current-buffer))
        (insert ?\n)))
+    (let ((frames (nthcdr
+                   ;; Remove debug--implement-debug-on-entry and the
+                   ;; advice's `apply' frame.
+                   (if (eq (car args) 'debug) 3 1)
+                   (backtrace-frames 'debug)))
+          (print-escape-newlines t)
+          (print-level 8)
+          (print-length 50))
+      (when (eq (car args) 'exit)
+        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))
+      (debugger-insert-backtrace frames t))
     ;; Place point on "stack frame 0" (bug#15101).
-    (goto-char pos))
-  ;; After any frame that uses eval-buffer,
-  ;; insert a line that states the buffer position it's reading at.
-  (save-excursion
-    (let ((tem eval-buffer-list))
-      (while (and tem
-		  (re-search-forward "^  eval-\\(buffer\\|region\\)(" nil t))
-	(end-of-line)
-	(insert (format "  ; Reading at buffer position %d"
-			;; This will get the wrong result
-			;; if there are two nested eval-region calls
-			;; for the same buffer.  That's not a very useful case.
-			(with-current-buffer (car tem)
-			  (point))))
-	(pop tem))))
-  (debugger-make-xrefs))
+    (goto-char pos)))
+
 
 (defun debugger-make-xrefs (&optional buffer)
   "Attach cross-references to function names in the `*Backtrace*' buffer."
-- 
2.11.1


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

From 2308506a4cc49e934a9b358737d54cb2932583ab Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 17:19:41 -0500
Subject: [PATCH v2 2/6] Improve ert backtrace recording

Change ert to use the new `backtrace-frames' function instead of
collecting frames one by one with `backtrace-frame'.  Additionally,
collect frames starting from `signal' instead the somewhat arbitrary
"6 from the bottom".  Skipping 6 frames would skip the expression that
actually caused the signal that triggered the debugger.  Possibly 6
was chosen because in the case of a failed test, the triggering frame
is an `ert-fail' call, which is not so interesting.  But in case of
test throwing an error, this drop the `error' call which is too much.

* lisp/emacs-lisp/ert.el (ert--print-backtrace): Remove.
(ert--print-backtrace): Add DO-XREFS parameter, delegate to
`debugger-insert-backtrace'.
(ert--run-test-debugger): Record the backtrace frames starting from
the instigating `signal' call.
(ert-run-tests-batch): Pass nil for `ert--print-backtrace's new
DO-XREFS parameter.
(ert-results-pop-to-backtrace-for-test-at-point): Pass t as DO-XREFS
to `ert--print-backtrace' and remove call to `debugger-make-xrefs'.
* test/lisp/emacs-lisp/ert-tests.el (ert-test-record-backtrace): Check
the backtrace list instead of comparing its string representation.
Expect `signal' to be the first frame.
---
 lisp/emacs-lisp/ert.el            | 62 ++++++++++++---------------------------
 test/lisp/emacs-lisp/ert-tests.el |  8 ++---
 2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 2c49a634e3..402798603a 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -670,48 +670,12 @@ (cl-defstruct (ert-test-skipped (:include ert-test-result-with-condition)))
 (cl-defstruct (ert-test-aborted-with-non-local-exit
                (:include ert-test-result)))
 
-
-(defun ert--record-backtrace ()
-  "Record the current backtrace (as a list) and return it."
-  ;; Since the backtrace is stored in the result object, result
-  ;; objects must only be printed with appropriate limits
-  ;; (`print-level' and `print-length') in place.  For interactive
-  ;; use, the cost of ensuring this possibly outweighs the advantage
-  ;; of storing the backtrace for
-  ;; `ert-results-pop-to-backtrace-for-test-at-point' given that we
-  ;; already have `ert-results-rerun-test-debugging-errors-at-point'.
-  ;; For batch use, however, printing the backtrace may be useful.
-  (cl-loop
-   ;; 6 is the number of frames our own debugger adds (when
-   ;; compiled; more when interpreted).  FIXME: Need to describe a
-   ;; procedure for determining this constant.
-   for i from 6
-   for frame = (backtrace-frame i)
-   while frame
-   collect frame))
-
-(defun ert--print-backtrace (backtrace)
+(defun ert--print-backtrace (backtrace do-xrefs)
   "Format the backtrace BACKTRACE to the current buffer."
-  ;; This is essentially a reimplementation of Fbacktrace
-  ;; (src/eval.c), but for a saved backtrace, not the current one.
   (let ((print-escape-newlines t)
         (print-level 8)
         (print-length 50))
-    (dolist (frame backtrace)
-      (pcase-exhaustive frame
-        (`(nil ,special-operator . ,arg-forms)
-         ;; Special operator.
-         (insert
-          (format "  %S\n" (cons special-operator arg-forms))))
-        (`(t ,fn . ,args)
-         ;; Function call.
-         (insert (format "  %S(" fn))
-         (cl-loop for firstp = t then nil
-                  for arg in args do
-                  (unless firstp
-                    (insert " "))
-                  (insert (format "%S" arg)))
-         (insert ")\n"))))))
+    (debugger-insert-backtrace backtrace do-xrefs)))
 
 ;; A container for the state of the execution of a single test and
 ;; environment data needed during its execution.
@@ -750,7 +714,19 @@ (defun ert--run-test-debugger (info args)
                       ((quit) 'quit)
 		      ((ert-test-skipped) 'skipped)
                       (otherwise 'failed)))
-              (backtrace (ert--record-backtrace))
+              ;; We store the backtrace in the result object for
+              ;; `ert-results-pop-to-backtrace-for-test-at-point'.
+              ;; This means we have to limit `print-level' and
+              ;; `print-length' when printing result objects.  That
+              ;; might not be worth while when we can also use
+              ;; `ert-results-rerun-test-debugging-errors-at-point',
+              ;; (i.e., when running interactively) but having the
+              ;; backtrace ready for printing is important for batch
+              ;; use.
+              ;;
+              ;; Grab the frames starting from `signal', frames below
+              ;; that are all from the debugger.
+              (backtrace (backtrace-frames 'signal))
               (infos (reverse ert--infos)))
          (setf (ert--test-execution-info-result info)
                (cl-ecase type
@@ -1409,8 +1385,9 @@ (defun ert-run-tests-batch (&optional selector)
               (ert-test-result-with-condition
                (message "Test %S backtrace:" (ert-test-name test))
                (with-temp-buffer
-                 (ert--print-backtrace (ert-test-result-with-condition-backtrace
-                                        result))
+                 (ert--print-backtrace
+                  (ert-test-result-with-condition-backtrace result)
+                  nil)
                  (goto-char (point-min))
                  (while (not (eobp))
                    (let ((start (point))
@@ -2420,8 +2397,7 @@ (defun ert-results-pop-to-backtrace-for-test-at-point ()
            ;; Use unibyte because `debugger-setup-buffer' also does so.
            (set-buffer-multibyte nil)
            (setq truncate-lines t)
-           (ert--print-backtrace backtrace)
-           (debugger-make-xrefs)
+           (ert--print-backtrace backtrace t)
            (goto-char (point-min))
            (insert (substitute-command-keys "Backtrace for test `"))
            (ert-insert-test-name-button (ert-test-name test))
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index fc5790c365..317838b250 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -367,12 +367,8 @@ (ert-deftest ert-test-record-backtrace ()
          (test (make-ert-test :body test-body))
          (result (ert-run-test test)))
     (should (ert-test-failed-p result))
-    (with-temp-buffer
-      (ert--print-backtrace (ert-test-failed-backtrace result))
-      (goto-char (point-min))
-      (end-of-line)
-      (let ((first-line (buffer-substring-no-properties (point-min) (point))))
-        (should (equal first-line (format "  %S()" test-body)))))))
+    (should (eq (nth 1 (car (ert-test-failed-backtrace result)))
+                'signal))))
 
 (ert-deftest ert-test-messages ()
   :tags '(:causes-redisplay)
-- 
2.11.1


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

From f740346aeffaa2b5c70727ef0fe83c8f1e2e0d33 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 18:13:54 -0500
Subject: [PATCH v2 3/6] Escape control characters in backtraces (Bug#6991)

* src/print.c (syms_of_print): Add new variable,
print-escape-control-characters.
(print_object): Print control characters with octal escape codes when
print-escape-control-characters is true.
* lisp/subr.el (backtrace):
* lisp/emacs-lisp/debug.el (debugger-setup-buffer): Bind
`print-escape-control-characters' to t.
---
 lisp/emacs-lisp/debug.el |  1 +
 lisp/subr.el             |  3 ++-
 src/print.c              | 45 +++++++++++++++++++++++++++++++++------------
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 0c8306d428..effe7f0cb3 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -362,6 +362,7 @@ (defun debugger-setup-buffer (args)
                    (if (eq (car args) 'debug) 3 1)
                    (backtrace-frames 'debug)))
           (print-escape-newlines t)
+          (print-escape-control-characters t)
           (print-level 8)
           (print-length 50))
       (when (eq (car args) 'exit)
diff --git a/lisp/subr.el b/lisp/subr.el
index 8d5d2a779c..c8036665db 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4513,7 +4513,8 @@ (defun backtrace--print-frame (evald func args flags)
 (defun backtrace ()
   "Print a trace of Lisp function calls currently active.
 Output stream used is value of `standard-output'."
-  (let ((print-level (or print-level 8)))
+  (let ((print-level (or print-level 8))
+        (print-escape-control-characters t))
     (mapbacktrace #'backtrace--print-frame 'backtrace)))
 
 (defun backtrace-frames (&optional base)
diff --git a/src/print.c b/src/print.c
index 49408bbeb4..dd95507624 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1839,21 +1839,36 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 		}
 	      else
 		{
+                  bool still_need_nonhex = false;
 		  /* If we just had a hex escape, and this character
 		     could be taken as part of it,
 		     output `\ ' to prevent that.  */
-		  if (need_nonhex && c_isxdigit (c))
-		    print_c_string ("\\ ", printcharfun);
-
-		  if (c == '\n' && print_escape_newlines
-		      ? (c = 'n', true)
-		      : c == '\f' && print_escape_newlines
-		      ? (c = 'f', true)
-		      : c == '\"' || c == '\\')
-		    printchar ('\\', printcharfun);
-
-		  printchar (c, printcharfun);
-		  need_nonhex = false;
+                  if (c_isxdigit (c))
+                    {
+                      if (need_nonhex)
+                        print_c_string ("\\ ", printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (c == '\n' && print_escape_newlines
+                           ? (c = 'n', true)
+                           : c == '\f' && print_escape_newlines
+                           ? (c = 'f', true)
+                           : c == '\0' && print_escape_control_characters
+                           ? (c = '0', still_need_nonhex = true)
+                           : c == '\"' || c == '\\')
+                    {
+                      printchar ('\\', printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (print_escape_control_characters && c_iscntrl (c))
+                    {
+                      char outbuf[1 + 3 + 1];
+                      int len = sprintf (outbuf, "\\%03o", c + 0u);
+                      strout (outbuf, len, len, printcharfun);
+                    }
+                  else
+                    printchar (c, printcharfun);
+		  need_nonhex = still_need_nonhex;
 		}
 	    }
 	  printchar ('\"', printcharfun);
@@ -2298,6 +2313,11 @@ syms_of_print (void)
 Also print formfeeds as `\\f'.  */);
   print_escape_newlines = 0;
 
+  DEFVAR_BOOL ("print-escape-control-characters", print_escape_control_characters,
+	       doc: /* Non-nil means print control characters in strings as `\\OOO'.
+\(OOO is the octal representation of the character code.)*/);
+  print_escape_control_characters = 0;
+
   DEFVAR_BOOL ("print-escape-nonascii", print_escape_nonascii,
 	       doc: /* Non-nil means print unibyte non-ASCII chars in strings as \\OOO.
 \(OOO is the octal representation of the character code.)
@@ -2387,6 +2407,7 @@ representation) and `#N#' in place of each subsequent occurrence,
   DEFSYM (Qprint_escape_newlines, "print-escape-newlines");
   DEFSYM (Qprint_escape_multibyte, "print-escape-multibyte");
   DEFSYM (Qprint_escape_nonascii, "print-escape-nonascii");
+  DEFSYM (Qprint_escape_control_characters, "print-escape-control-characters");
 
   print_prune_charset_plist = Qnil;
   staticpro (&print_prune_charset_plist);
-- 
2.11.1


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

From e5499ea5fdb3b4aad76b4d38f4d657b034778711 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 19:00:19 -0500
Subject: [PATCH v2 4/6] Hide byte code in backtraces (Bug#6991)

* lisp/emacs-lisp/cl-print.el: Autoload `disassemble-1'.
(cl-print-compiled-button): New variable.
(help-byte-code): New button type, calls `disassemble' in its action.
(cl-print-object): Use it if `cl-print-compiled-button' is
non-nil.
(debugger-insert-backtrace): Use `cl-print' with `cl-print-compiled-button'
let-bound to t.

* lisp/emacs-lisp/cl-print.el
---
 lisp/emacs-lisp/cl-print.el | 33 +++++++++++++++++++++++++++++----
 lisp/emacs-lisp/debug.el    | 11 ++++++++---
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/cl-print.el b/lisp/emacs-lisp/cl-print.el
index 65c86d2b65..44c6a4312d 100644
--- a/lisp/emacs-lisp/cl-print.el
+++ b/lisp/emacs-lisp/cl-print.el
@@ -33,6 +33,8 @@
 
 ;;; Code:
 
+(require 'button)
+
 (defvar cl-print-readably nil
   "If non-nil, try and make sure the result can be `read'.")
 
@@ -74,13 +76,27 @@ (cl-defmethod cl-print-object ((object vector) stream)
     (cl-print-object (aref object i) stream))
   (princ "]" stream))
 
+(define-button-type 'help-byte-code
+  'follow-link t
+  'action (lambda (button)
+            (disassemble (button-get button 'byte-code-function)))
+  'help-echo (purecopy "mouse-2, RET: disassemble this function"))
+
 (defvar cl-print-compiled nil
   "Control how to print byte-compiled functions.  Can be:
 - `static' to print the vector of constants.
 - `disassemble' to print the disassembly of the code.
 - nil to skip printing any details about the code.")
 
+(defvar cl-print-compiled-button nil
+  "Control how to print byte-compiled functions into buffers.
+When the stream is a buffer, make the bytecode part of the output
+into a button whose action shows the function's disassembly.")
+
+(autoload 'disassemble-1 "disass")
+
 (cl-defmethod cl-print-object ((object compiled-function) stream)
+  (unless stream (setq stream standard-output))
   ;; We use "#f(...)" rather than "#<...>" so that pp.el gives better results.
   (princ "#f(compiled-function " stream)
   (let ((args (help-function-arglist object 'preserve-names)))
@@ -108,10 +124,19 @@ (cl-defmethod cl-print-object ((object compiled-function) stream)
          (disassemble-1 object 0)
          (buffer-string))
        stream)
-    (princ " #<bytecode>" stream)
-    (when (eq cl-print-compiled 'static)
-      (princ " " stream)
-      (cl-print-object (aref object 2) stream)))
+    (princ " " stream)
+    (let ((button-start (and cl-print-compiled-button
+                             (bufferp stream)
+                             (with-current-buffer stream (point)))))
+      (princ "#<bytecode>" stream)
+      (when (eq cl-print-compiled 'static)
+        (princ " " stream)
+        (cl-print-object (aref object 2) stream))
+      (when button-start
+        (with-current-buffer stream
+          (make-text-button button-start (point)
+                            :type 'help-byte-code
+                            'byte-code-function object)))))
   (princ ")" stream))
 
 ;; This belongs in nadvice.el, of course, but some load-ordering issues make it
diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index effe7f0cb3..83c206e163 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -265,10 +265,15 @@ (defun debug (&rest args)
       debugger-value)))
 \f
 
+(defvar cl-print-compiled)
+(defvar cl-print-compiled-button)
+
 (defun debugger-insert-backtrace (frames do-xrefs)
   "Format and insert the backtrace FRAMES at point.
 Make functions into cross-reference buttons if DO-XREFS is non-nil."
   (let ((standard-output (current-buffer))
+        (cl-print-compiled nil)
+        (cl-print-compiled-button t)
         (eval-buffers eval-buffer-list))
     (require 'help-mode)     ; Define `help-function-def' button type.
     (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
@@ -278,10 +283,10 @@ (defun debugger-insert-backtrace (frames do-xrefs)
             (fun-pt (point)))
         (cond
          ((and evald (not debugger-stack-frame-as-list))
-          (prin1 fun)
-          (if args (prin1 args) (princ "()")))
+          (cl-prin1 fun)
+          (if args (cl-prin1 args) (princ "()")))
          (t
-          (prin1 (cons fun args))
+          (cl-prin1 (cons fun args))
           (cl-incf fun-pt)))
         (when fun-file
           (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
-- 
2.11.1


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

From 57e94182464e6d6fec3e0f7dccdda8a45abf38df Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 19:47:55 -0500
Subject: [PATCH v2 5/6] Escape NUL bytes in X selections (Bug#6991)

* lisp/select.el (xselect--encode-string): Replace NUL bytes with
"\0".
---
 lisp/select.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/select.el b/lisp/select.el
index 4849d7d515..579c5c7e2e 100644
--- a/lisp/select.el
+++ b/lisp/select.el
@@ -475,6 +475,9 @@ (defun xselect--encode-string (type str &optional can-modify)
 	   (t
 	    (error "Unknown selection type: %S" type)))))
 
+      ;; Most programs are unable to handle NUL bytes in strings.
+      (setq str (replace-regexp-in-string "\0" "\\0" str t t))
+
       (setq next-selection-coding-system nil)
       (cons type str))))
 
-- 
2.11.1


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

From 2ad16d82d8d5e521b646c2ca53a047e30b05c1c8 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 27 May 2017 22:40:46 -0400
Subject: [PATCH v2 6/6] Don't redundantly cl-print arglist in function
 docstring again

* lisp/emacs-lisp/cl-print.el (cl-print-object): Don't print arglist
part of docstring.
---
 lisp/emacs-lisp/cl-print.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/cl-print.el b/lisp/emacs-lisp/cl-print.el
index 44c6a4312d..3958ee80a3 100644
--- a/lisp/emacs-lisp/cl-print.el
+++ b/lisp/emacs-lisp/cl-print.el
@@ -103,10 +103,10 @@ (cl-defmethod cl-print-object ((object compiled-function) stream)
     (if args
         (prin1 args stream)
       (princ "()" stream)))
-  (let ((doc (documentation object 'raw)))
-    (when doc
-      (princ " " stream)
-      (prin1 doc stream)))
+  (pcase (help-split-fundoc (documentation object 'raw) object)
+    (`(,_ . ,(and doc (guard (stringp doc))))
+     (princ " " stream)
+     (prin1 doc stream)))
   (let ((inter (interactive-form object)))
     (when inter
       (princ " " stream)
-- 
2.11.1

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

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

npostavs@users.sourceforge.net writes:

> npostavs@users.sourceforge.net writes:
>
>> I propose adding a new flag print_escape_control_characters instead (see
>> patch #3 in the series).  I also implemented hiding the byte code
>> functions with text properties in #4.  It's not quite satisfactory
>> though, because it doesn't cover byte code functions values that are
>> arguments, only byte code being called.  I think printing needs to be
>> made more flexible in order to cleanly catch all byte code values.
>> Patch #5 replaces NUL bytes with "\0" in X selections (I guess it covers
>> w32 as well? Haven't checked yet).
>
> Updated the patchset to use cl-prin1, now it applies to function values
> in arguments as well.  This one actually doesn't include the byte code
> text at all (path of least resistance: cl-print-object wasn't already
> omitting bytecode and I haven't bothered adding it).

I made use of cl-prin1 depend on a custom option, and replace NUL bytes
also in w32.  I think this is ready to merge now, I will probably do so
sometime next week.


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

From c3aaaccb31bf926ce08cb0dc17b36ffc01ce7e7d Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 09:19:00 -0500
Subject: [PATCH v3 1/6] Operate on frame list instead of printed backtrace

* lisp/emacs-lisp/debug.el (debugger--insert-backtrace): New function,
prints the given backtrace frames.
(debugger-setup-buffer): Use it instead of editing the backtrace
buffer text.
---
 lisp/emacs-lisp/debug.el | 84 +++++++++++++++++++++++++++---------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 83456fc31a..0c8306d428 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -264,6 +264,40 @@ (defun debug (&rest args)
       (setq debug-on-next-call debugger-step-after-exit)
       debugger-value)))
 \f
+
+(defun debugger-insert-backtrace (frames do-xrefs)
+  "Format and insert the backtrace FRAMES at point.
+Make functions into cross-reference buttons if DO-XREFS is non-nil."
+  (let ((standard-output (current-buffer))
+        (eval-buffers eval-buffer-list))
+    (require 'help-mode)     ; Define `help-function-def' button type.
+    (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
+      (insert (if (plist-get flags :debug-on-exit)
+                  "* " "  "))
+      (let ((fun-file (and do-xrefs (symbol-file fun 'defun)))
+            (fun-pt (point)))
+        (cond
+         ((and evald (not debugger-stack-frame-as-list))
+          (prin1 fun)
+          (if args (prin1 args) (princ "()")))
+         (t
+          (prin1 (cons fun args))
+          (cl-incf fun-pt)))
+        (when fun-file
+          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
+                            :type 'help-function-def
+                            'help-args (list fun fun-file))))
+      ;; After any frame that uses eval-buffer, insert a line that
+      ;; states the buffer position it's reading at.
+      (when (and eval-buffers (memq fun '(eval-buffer eval-region)))
+        (insert (format "  ; Reading at buffer position %d"
+                        ;; This will get the wrong result if there are
+                        ;; two nested eval-region calls for the same
+                        ;; buffer.  That's not a very useful case.
+                        (with-current-buffer (pop eval-buffers)
+                          (point)))))
+      (insert "\n"))))
+
 (defun debugger-setup-buffer (args)
   "Initialize the `*Backtrace*' buffer for entry to the debugger.
 That buffer should be current already."
@@ -271,22 +305,6 @@ (defun debugger-setup-buffer (args)
   (erase-buffer)
   (set-buffer-multibyte t)		;Why was it nil ?  -stef
   (setq buffer-undo-list t)
-  (let ((standard-output (current-buffer))
-	(print-escape-newlines t)
-	(print-level 8)
-        (print-length 50))
-    ;; FIXME the debugger could pass a custom callback to mapbacktrace
-    ;; instead of manipulating printed results.
-    (mapbacktrace #'backtrace--print-frame 'debug))
-  (goto-char (point-min))
-  (delete-region (point)
-		 (progn
-                   (forward-line (if (eq (car args) 'debug)
-                                     ;; Remove debug--implement-debug-on-entry
-                                     ;; and the advice's `apply' frame.
-				     3
-				   1))
-		   (point)))
   (insert "Debugger entered")
   ;; lambda is for debug-on-call when a function call is next.
   ;; debug is for debug-on-entry function called.
@@ -301,10 +319,7 @@ (defun debugger-setup-buffer (args)
        (setq pos (point))
        (setq debugger-value (nth 1 args))
        (prin1 debugger-value (current-buffer))
-       (insert ?\n)
-       (delete-char 1)
-       (insert ? )
-       (beginning-of-line))
+       (insert ?\n))
       ;; Watchpoint triggered.
       ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
        (insert
@@ -341,23 +356,20 @@ (defun debugger-setup-buffer (args)
                   (cdr args) args)
               (current-buffer))
        (insert ?\n)))
+    (let ((frames (nthcdr
+                   ;; Remove debug--implement-debug-on-entry and the
+                   ;; advice's `apply' frame.
+                   (if (eq (car args) 'debug) 3 1)
+                   (backtrace-frames 'debug)))
+          (print-escape-newlines t)
+          (print-level 8)
+          (print-length 50))
+      (when (eq (car args) 'exit)
+        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))
+      (debugger-insert-backtrace frames t))
     ;; Place point on "stack frame 0" (bug#15101).
-    (goto-char pos))
-  ;; After any frame that uses eval-buffer,
-  ;; insert a line that states the buffer position it's reading at.
-  (save-excursion
-    (let ((tem eval-buffer-list))
-      (while (and tem
-		  (re-search-forward "^  eval-\\(buffer\\|region\\)(" nil t))
-	(end-of-line)
-	(insert (format "  ; Reading at buffer position %d"
-			;; This will get the wrong result
-			;; if there are two nested eval-region calls
-			;; for the same buffer.  That's not a very useful case.
-			(with-current-buffer (car tem)
-			  (point))))
-	(pop tem))))
-  (debugger-make-xrefs))
+    (goto-char pos)))
+
 
 (defun debugger-make-xrefs (&optional buffer)
   "Attach cross-references to function names in the `*Backtrace*' buffer."
-- 
2.11.1


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

From aae0b062d707ec490b909b0dec4017483b630dc3 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 17:19:41 -0500
Subject: [PATCH v3 2/6] Improve ert backtrace recording

Change ert to use the new `backtrace-frames' function instead of
collecting frames one by one with `backtrace-frame'.  Additionally,
collect frames starting from `signal' instead the somewhat arbitrary
"6 from the bottom".  Skipping 6 frames would skip the expression that
actually caused the signal that triggered the debugger.  Possibly 6
was chosen because in the case of a failed test, the triggering frame
is an `ert-fail' call, which is not so interesting.  But in case of
test throwing an error, this drop the `error' call which is too much.

* lisp/emacs-lisp/ert.el (ert--print-backtrace): Remove.
(ert--print-backtrace): Add DO-XREFS parameter, delegate to
`debugger-insert-backtrace'.
(ert--run-test-debugger): Record the backtrace frames starting from
the instigating `signal' call.
(ert-run-tests-batch): Pass nil for `ert--print-backtrace's new
DO-XREFS parameter.
(ert-results-pop-to-backtrace-for-test-at-point): Pass t as DO-XREFS
to `ert--print-backtrace' and remove call to `debugger-make-xrefs'.
* test/lisp/emacs-lisp/ert-tests.el (ert-test-record-backtrace): Check
the backtrace list instead of comparing its string representation.
Expect `signal' to be the first frame.
---
 lisp/emacs-lisp/ert.el            | 62 ++++++++++++---------------------------
 test/lisp/emacs-lisp/ert-tests.el |  8 ++---
 2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 2c49a634e3..402798603a 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -670,48 +670,12 @@ (cl-defstruct (ert-test-skipped (:include ert-test-result-with-condition)))
 (cl-defstruct (ert-test-aborted-with-non-local-exit
                (:include ert-test-result)))
 
-
-(defun ert--record-backtrace ()
-  "Record the current backtrace (as a list) and return it."
-  ;; Since the backtrace is stored in the result object, result
-  ;; objects must only be printed with appropriate limits
-  ;; (`print-level' and `print-length') in place.  For interactive
-  ;; use, the cost of ensuring this possibly outweighs the advantage
-  ;; of storing the backtrace for
-  ;; `ert-results-pop-to-backtrace-for-test-at-point' given that we
-  ;; already have `ert-results-rerun-test-debugging-errors-at-point'.
-  ;; For batch use, however, printing the backtrace may be useful.
-  (cl-loop
-   ;; 6 is the number of frames our own debugger adds (when
-   ;; compiled; more when interpreted).  FIXME: Need to describe a
-   ;; procedure for determining this constant.
-   for i from 6
-   for frame = (backtrace-frame i)
-   while frame
-   collect frame))
-
-(defun ert--print-backtrace (backtrace)
+(defun ert--print-backtrace (backtrace do-xrefs)
   "Format the backtrace BACKTRACE to the current buffer."
-  ;; This is essentially a reimplementation of Fbacktrace
-  ;; (src/eval.c), but for a saved backtrace, not the current one.
   (let ((print-escape-newlines t)
         (print-level 8)
         (print-length 50))
-    (dolist (frame backtrace)
-      (pcase-exhaustive frame
-        (`(nil ,special-operator . ,arg-forms)
-         ;; Special operator.
-         (insert
-          (format "  %S\n" (cons special-operator arg-forms))))
-        (`(t ,fn . ,args)
-         ;; Function call.
-         (insert (format "  %S(" fn))
-         (cl-loop for firstp = t then nil
-                  for arg in args do
-                  (unless firstp
-                    (insert " "))
-                  (insert (format "%S" arg)))
-         (insert ")\n"))))))
+    (debugger-insert-backtrace backtrace do-xrefs)))
 
 ;; A container for the state of the execution of a single test and
 ;; environment data needed during its execution.
@@ -750,7 +714,19 @@ (defun ert--run-test-debugger (info args)
                       ((quit) 'quit)
 		      ((ert-test-skipped) 'skipped)
                       (otherwise 'failed)))
-              (backtrace (ert--record-backtrace))
+              ;; We store the backtrace in the result object for
+              ;; `ert-results-pop-to-backtrace-for-test-at-point'.
+              ;; This means we have to limit `print-level' and
+              ;; `print-length' when printing result objects.  That
+              ;; might not be worth while when we can also use
+              ;; `ert-results-rerun-test-debugging-errors-at-point',
+              ;; (i.e., when running interactively) but having the
+              ;; backtrace ready for printing is important for batch
+              ;; use.
+              ;;
+              ;; Grab the frames starting from `signal', frames below
+              ;; that are all from the debugger.
+              (backtrace (backtrace-frames 'signal))
               (infos (reverse ert--infos)))
          (setf (ert--test-execution-info-result info)
                (cl-ecase type
@@ -1409,8 +1385,9 @@ (defun ert-run-tests-batch (&optional selector)
               (ert-test-result-with-condition
                (message "Test %S backtrace:" (ert-test-name test))
                (with-temp-buffer
-                 (ert--print-backtrace (ert-test-result-with-condition-backtrace
-                                        result))
+                 (ert--print-backtrace
+                  (ert-test-result-with-condition-backtrace result)
+                  nil)
                  (goto-char (point-min))
                  (while (not (eobp))
                    (let ((start (point))
@@ -2420,8 +2397,7 @@ (defun ert-results-pop-to-backtrace-for-test-at-point ()
            ;; Use unibyte because `debugger-setup-buffer' also does so.
            (set-buffer-multibyte nil)
            (setq truncate-lines t)
-           (ert--print-backtrace backtrace)
-           (debugger-make-xrefs)
+           (ert--print-backtrace backtrace t)
            (goto-char (point-min))
            (insert (substitute-command-keys "Backtrace for test `"))
            (ert-insert-test-name-button (ert-test-name test))
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index fc5790c365..317838b250 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -367,12 +367,8 @@ (ert-deftest ert-test-record-backtrace ()
          (test (make-ert-test :body test-body))
          (result (ert-run-test test)))
     (should (ert-test-failed-p result))
-    (with-temp-buffer
-      (ert--print-backtrace (ert-test-failed-backtrace result))
-      (goto-char (point-min))
-      (end-of-line)
-      (let ((first-line (buffer-substring-no-properties (point-min) (point))))
-        (should (equal first-line (format "  %S()" test-body)))))))
+    (should (eq (nth 1 (car (ert-test-failed-backtrace result)))
+                'signal))))
 
 (ert-deftest ert-test-messages ()
   :tags '(:causes-redisplay)
-- 
2.11.1


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

From ff73e5a06cb943dcb58ee24213e82316b8594061 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 18:13:54 -0500
Subject: [PATCH v3 3/6] Escape control characters in backtraces (Bug#6991)

* src/print.c (syms_of_print): Add new variable,
print-escape-control-characters.
(print_object): Print control characters with octal escape codes when
print-escape-control-characters is true.
* lisp/subr.el (backtrace):
* lisp/emacs-lisp/debug.el (debugger-setup-buffer): Bind
`print-escape-control-characters' to t.
---
 lisp/emacs-lisp/debug.el |  1 +
 lisp/subr.el             |  3 ++-
 src/print.c              | 45 +++++++++++++++++++++++++++++++++------------
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 0c8306d428..effe7f0cb3 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -362,6 +362,7 @@ (defun debugger-setup-buffer (args)
                    (if (eq (car args) 'debug) 3 1)
                    (backtrace-frames 'debug)))
           (print-escape-newlines t)
+          (print-escape-control-characters t)
           (print-level 8)
           (print-length 50))
       (when (eq (car args) 'exit)
diff --git a/lisp/subr.el b/lisp/subr.el
index ef00286b34..76c13a8d23 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4513,7 +4513,8 @@ (defun backtrace--print-frame (evald func args flags)
 (defun backtrace ()
   "Print a trace of Lisp function calls currently active.
 Output stream used is value of `standard-output'."
-  (let ((print-level (or print-level 8)))
+  (let ((print-level (or print-level 8))
+        (print-escape-control-characters t))
     (mapbacktrace #'backtrace--print-frame 'backtrace)))
 
 (defun backtrace-frames (&optional base)
diff --git a/src/print.c b/src/print.c
index 6bf8af9ef9..50c75d7712 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1870,21 +1870,36 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 		}
 	      else
 		{
+                  bool still_need_nonhex = false;
 		  /* If we just had a hex escape, and this character
 		     could be taken as part of it,
 		     output `\ ' to prevent that.  */
-		  if (need_nonhex && c_isxdigit (c))
-		    print_c_string ("\\ ", printcharfun);
-
-		  if (c == '\n' && print_escape_newlines
-		      ? (c = 'n', true)
-		      : c == '\f' && print_escape_newlines
-		      ? (c = 'f', true)
-		      : c == '\"' || c == '\\')
-		    printchar ('\\', printcharfun);
-
-		  printchar (c, printcharfun);
-		  need_nonhex = false;
+                  if (c_isxdigit (c))
+                    {
+                      if (need_nonhex)
+                        print_c_string ("\\ ", printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (c == '\n' && print_escape_newlines
+                           ? (c = 'n', true)
+                           : c == '\f' && print_escape_newlines
+                           ? (c = 'f', true)
+                           : c == '\0' && print_escape_control_characters
+                           ? (c = '0', still_need_nonhex = true)
+                           : c == '\"' || c == '\\')
+                    {
+                      printchar ('\\', printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (print_escape_control_characters && c_iscntrl (c))
+                    {
+                      char outbuf[1 + 3 + 1];
+                      int len = sprintf (outbuf, "\\%03o", c + 0u);
+                      strout (outbuf, len, len, printcharfun);
+                    }
+                  else
+                    printchar (c, printcharfun);
+		  need_nonhex = still_need_nonhex;
 		}
 	    }
 	  printchar ('\"', printcharfun);
@@ -2329,6 +2344,11 @@ syms_of_print (void)
 Also print formfeeds as `\\f'.  */);
   print_escape_newlines = 0;
 
+  DEFVAR_BOOL ("print-escape-control-characters", print_escape_control_characters,
+	       doc: /* Non-nil means print control characters in strings as `\\OOO'.
+\(OOO is the octal representation of the character code.)*/);
+  print_escape_control_characters = 0;
+
   DEFVAR_BOOL ("print-escape-nonascii", print_escape_nonascii,
 	       doc: /* Non-nil means print unibyte non-ASCII chars in strings as \\OOO.
 \(OOO is the octal representation of the character code.)
@@ -2418,6 +2438,7 @@ representation) and `#N#' in place of each subsequent occurrence,
   DEFSYM (Qprint_escape_newlines, "print-escape-newlines");
   DEFSYM (Qprint_escape_multibyte, "print-escape-multibyte");
   DEFSYM (Qprint_escape_nonascii, "print-escape-nonascii");
+  DEFSYM (Qprint_escape_control_characters, "print-escape-control-characters");
 
   print_prune_charset_plist = Qnil;
   staticpro (&print_prune_charset_plist);
-- 
2.11.1


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

From 1a6b3342b1f2f401bb1c9e5bcf14a66ae2d7c472 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 27 May 2017 22:40:46 -0400
Subject: [PATCH v3 4/6] Don't redundantly cl-print arglist in function
 docstring again

* lisp/emacs-lisp/cl-print.el (cl-print-object): Don't print arglist
part of docstring.
* test/lisp/emacs-lisp/cl-print-tests.el (cl-print-tests-1): Update
test accordingly.
---
 lisp/emacs-lisp/cl-print.el            | 8 ++++----
 test/lisp/emacs-lisp/cl-print-tests.el | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/emacs-lisp/cl-print.el b/lisp/emacs-lisp/cl-print.el
index 89a71d1b6c..6703fc99e2 100644
--- a/lisp/emacs-lisp/cl-print.el
+++ b/lisp/emacs-lisp/cl-print.el
@@ -105,10 +105,10 @@ (cl-defmethod cl-print-object ((object compiled-function) stream)
     (if args
         (prin1 args stream)
       (princ "()" stream)))
-  (let ((doc (documentation object 'raw)))
-    (when doc
-      (princ " " stream)
-      (prin1 doc stream)))
+  (pcase (help-split-fundoc (documentation object 'raw) object)
+    (`(,_ . ,(and doc (guard (stringp doc))))
+     (princ " " stream)
+     (prin1 doc stream)))
   (let ((inter (interactive-form object)))
     (when inter
       (princ " " stream)
diff --git a/test/lisp/emacs-lisp/cl-print-tests.el b/test/lisp/emacs-lisp/cl-print-tests.el
index dfbe18d784..6448a1b37f 100644
--- a/test/lisp/emacs-lisp/cl-print-tests.el
+++ b/test/lisp/emacs-lisp/cl-print-tests.el
@@ -34,7 +34,7 @@ (ert-deftest cl-print-tests-1 ()
     (let ((print-circle t))
       (should (equal (cl-prin1-to-string `((x . ,x) (y . ,x)))
                      "((x . #1=#s(cl-print--test :a 1 :b 2)) (y . #1#))")))
-    (should (string-match "\\`#f(compiled-function (x) .*\n\n.*)\\'"
+    (should (string-match "\\`#f(compiled-function (x) \"[^\"]+\" [^\)]*)\\'"
                           (cl-prin1-to-string (symbol-function #'caar))))))
 
 (ert-deftest cl-print-tests-2 ()
-- 
2.11.1


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

From f6f929626abfff951dcf66d8014a7664a8e4f8c9 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 11 Jun 2017 09:51:38 -0400
Subject: [PATCH v3 5/6] Hide byte code in backtraces (Bug#6991)

* lisp/emacs-lisp/debug.el (debugger-print-function): New defcustom,
defaulting to `cl-print'.
(debugger-insert-backtrace): Use it.
* etc/NEWS: Announce it.
---
 etc/NEWS                 |  5 +++++
 lisp/emacs-lisp/debug.el | 17 ++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3bbcaef3f4..5c5575d8d7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -320,6 +320,11 @@ questions, with a handy way to display help texts.
 all call stack frames in a Lisp backtrace buffer as lists.  Both
 debug.el and edebug.el have been updated to heed to this variable.
 
+---
+** Values in call stack frames are now displayed using 'cl-prin1'.
+The old behaviour of using 'prin1' can be restored by customizing the
+new option 'debugger-print-function'.
+
 +++
 ** The new variable 'x-ctrl-keysym' has been added to the existing
 roster of X keysyms.  It can be used in combination with another
diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index effe7f0cb3..e5445416dc 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -49,6 +49,13 @@ (defcustom debugger-batch-max-lines 40
   :group 'debugger
   :version "21.1")
 
+(defcustom debugger-print-function #'cl-prin1
+  "Function used to print values in the debugger backtraces."
+  :type 'function
+  :options '(cl-prin1 prin1)
+  :group 'debugger
+  :version "26.1")
+
 (defcustom debugger-bury-or-kill 'bury
   "What to do with the debugger buffer when exiting `debug'.
 The value affects the behavior of operations on any window
@@ -265,10 +272,14 @@ (defun debug (&rest args)
       debugger-value)))
 \f
 
+(defvar cl-print-compiled)
+(defvar cl-print-compiled-button)
+
 (defun debugger-insert-backtrace (frames do-xrefs)
   "Format and insert the backtrace FRAMES at point.
 Make functions into cross-reference buttons if DO-XREFS is non-nil."
   (let ((standard-output (current-buffer))
+        (cl-print-compiled-button t)
         (eval-buffers eval-buffer-list))
     (require 'help-mode)     ; Define `help-function-def' button type.
     (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
@@ -278,10 +289,10 @@ (defun debugger-insert-backtrace (frames do-xrefs)
             (fun-pt (point)))
         (cond
          ((and evald (not debugger-stack-frame-as-list))
-          (prin1 fun)
-          (if args (prin1 args) (princ "()")))
+          (funcall debugger-print-function fun)
+          (if args (cl-prin1 args) (princ "()")))
          (t
-          (prin1 (cons fun args))
+          (funcall debugger-print-function (cons fun args))
           (cl-incf fun-pt)))
         (when fun-file
           (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
-- 
2.11.1


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

From ad896373f17c52806638fcf12a5bb6a86b390f45 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 19:47:55 -0500
Subject: [PATCH v3 6/6] Escape NUL bytes in X selections (Bug#6991)

* lisp/term/w32-win.el (w32--set-selection):
* lisp/select.el (xselect--encode-string): Replace NUL bytes with
"\0".
* doc/emacs/killing.texi: Document new behavior.
* etc/NEWS (times): Announce it.
---
 doc/emacs/killing.texi | 4 ++++
 etc/NEWS               | 4 ++++
 lisp/select.el         | 3 +++
 lisp/term/w32-win.el   | 2 +-
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/killing.texi b/doc/emacs/killing.texi
index 47de053129..0b5efd04a1 100644
--- a/doc/emacs/killing.texi
+++ b/doc/emacs/killing.texi
@@ -519,6 +519,10 @@ Clipboard
 data to the clipboard manager, change the variable
 @code{x-select-enable-clipboard-manager} to @code{nil}.
 
+  Since strings containing NUL bytes are usually truncated when passed
+through the clipboard, Emacs replaces such characters with ``\0''
+before transfering them to the system's clipboard.
+
 @vindex select-enable-primary
 @findex clipboard-kill-region
 @findex clipboard-kill-ring-save
diff --git a/etc/NEWS b/etc/NEWS
index 5c5575d8d7..61efba702c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -326,6 +326,10 @@ The old behaviour of using 'prin1' can be restored by customizing the
 new option 'debugger-print-function'.
 
 +++
+** NUL bytes in strings copied to the system clipboard are now
+replaced with "\0".
+
++++
 ** The new variable 'x-ctrl-keysym' has been added to the existing
 roster of X keysyms.  It can be used in combination with another
 variable of this kind to swap modifiers in Emacs.
diff --git a/lisp/select.el b/lisp/select.el
index 4849d7d515..579c5c7e2e 100644
--- a/lisp/select.el
+++ b/lisp/select.el
@@ -475,6 +475,9 @@ (defun xselect--encode-string (type str &optional can-modify)
 	   (t
 	    (error "Unknown selection type: %S" type)))))
 
+      ;; Most programs are unable to handle NUL bytes in strings.
+      (setq str (replace-regexp-in-string "\0" "\\0" str t t))
+
       (setq next-selection-coding-system nil)
       (cons type str))))
 
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index fda93884c4..be895a040d 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -396,7 +396,7 @@ (declare-function w32-selection-exists-p "w32select.c")
 ;;; Fix interface to (X-specific) mouse.el
 (defun w32--set-selection (type value)
   (if (eq type 'CLIPBOARD)
-      (w32-set-clipboard-data value)
+      (w32-set-clipboard-data (replace-regexp-in-string "\0" "\\0" value t t))
     (put 'x-selections (or type 'PRIMARY) value)))
 
 (defun w32--get-selection  (&optional type data-type)
-- 
2.11.1

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Stefan Monnier]

> I made use of cl-prin1 depend on a custom option, and replace NUL bytes
> also in w32.  I think this is ready to merge now, I will probably do so
> sometime next week.

A few questions below.

> +        (when fun-file
> +          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
> +                            :type 'help-function-def
> +                            'help-args (list fun fun-file))))

Hmm... this looks like code which was moved from elsewhere, yet I can't
find this elsewhere in your patch(es).

I think that other code is in debugger-make-xrefs, so can't we remove
debugger-make-xrefs?

> +    (let ((frames (nthcdr
> +                   ;; Remove debug--implement-debug-on-entry and the
> +                   ;; advice's `apply' frame.
> +                   (if (eq (car args) 'debug) 3 1)
> +                   (backtrace-frames 'debug)))
> +          (print-escape-newlines t)
> +          (print-level 8)
> +          (print-length 50))

Why let-bind print-* here rather than inside debugger-insert-backtrace?

> +      (when (eq (car args) 'exit)
> +        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))

This looks like code which was moved from elsewhere, yet I can't find
this elsewhere in your patch(es).  What am I missing?

> +  (pcase (help-split-fundoc (documentation object 'raw) object)
> +    (`(,_ . ,(and doc (guard (stringp doc))))
> +     (princ " " stream)
> +     (prin1 doc stream)))

Maybe this deserves a one-line comment explaining that the arglist part
was already printed via help-function-arglist.

> +(defcustom debugger-print-function #'cl-prin1
> +  "Function used to print values in the debugger backtraces."
> +  :type 'function
> +  :options '(cl-prin1 prin1)
> +  :group 'debugger
> +  :version "26.1")

The `:group 'debugger` is redundant (as is the case for all defcustom
in this file).

> +(defvar cl-print-compiled)

Is this used somewhere?

> -          (prin1 fun)
> -          (if args (prin1 args) (princ "()")))
> +          (funcall debugger-print-function fun)
> +          (if args (cl-prin1 args) (princ "()")))

This `cl-prin1` should be replaced with debugger-print-function, right?


        Stefan

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

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


Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>> +        (when fun-file
>> +          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
>> +                            :type 'help-function-def
>> +                            'help-args (list fun fun-file))))
>
> Hmm... this looks like code which was moved from elsewhere, yet I can't
> find this elsewhere in your patch(es).

> I think that other code is in debugger-make-xrefs, so can't we remove
> debugger-make-xrefs?

I'm not sure exactly what you mean by "looks like code which was moved".
It does replace the functionality of debugger-make-xrefs.  But
`ert--make-xrefs-region' is still using `debugger-make-xrefs', and I
don't quite see how to remove that usage.

>> +    (let ((frames (nthcdr
>> +                   ;; Remove debug--implement-debug-on-entry and the
>> +                   ;; advice's `apply' frame.
>> +                   (if (eq (car args) 'debug) 3 1)
>> +                   (backtrace-frames 'debug)))
>> +          (print-escape-newlines t)
>> +          (print-level 8)
>> +          (print-length 50))
>
> Why let-bind print-* here rather than inside debugger-insert-backtrace?

I thought moving those inside might needlessly make the function less
flexible, though nobody is currently making use of the flexibility so
maybe it's not worth it...

>> +      (when (eq (car args) 'exit)
>> +        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))
>
> This looks like code which was moved from elsewhere, yet I can't find
> this elsewhere in your patch(es).  What am I missing?

backtrace--print-frame I guess?  I haven't changed the printing for
`backtrace', perhaps I should...

>> +  (pcase (help-split-fundoc (documentation object 'raw) object)
>> +    (`(,_ . ,(and doc (guard (stringp doc))))
>> +     (princ " " stream)
>> +     (prin1 doc stream)))
>
> Maybe this deserves a one-line comment explaining that the arglist part
> was already printed via help-function-arglist.

Sure.

>> +(defcustom debugger-print-function #'cl-prin1
>> +  "Function used to print values in the debugger backtraces."
>> +  :type 'function
>> +  :options '(cl-prin1 prin1)
>> +  :group 'debugger
>> +  :version "26.1")
>
> The `:group 'debugger` is redundant (as is the case for all defcustom
> in this file).

Yeah, I just followed the others, I'll remove it.

>> +(defvar cl-print-compiled)
>
> Is this used somewhere?

Oh, I think that's leftover from avoiding Bug#27117.

>> -          (prin1 fun)
>> -          (if args (prin1 args) (princ "()")))
>> +          (funcall debugger-print-function fun)
>> +          (if args (cl-prin1 args) (princ "()")))
>
> This `cl-prin1` should be replaced with debugger-print-function, right?

Oops!


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 5349 bytes --]

From a4bd2230a428560338afd4b5f2e74eccba9c4afc Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 09:19:00 -0500
Subject: [PATCH v4 1/6] Operate on frame list instead of printed backtrace

* lisp/emacs-lisp/debug.el (debugger-insert-backtrace): New function,
prints the given backtrace frames.
(debugger-setup-buffer): Use it instead of editing the backtrace
buffer text.
---
 lisp/emacs-lisp/debug.el | 84 +++++++++++++++++++++++++++---------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 83456fc31a..0c8306d428 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -264,6 +264,40 @@ (defun debug (&rest args)
       (setq debug-on-next-call debugger-step-after-exit)
       debugger-value)))
 \f
+
+(defun debugger-insert-backtrace (frames do-xrefs)
+  "Format and insert the backtrace FRAMES at point.
+Make functions into cross-reference buttons if DO-XREFS is non-nil."
+  (let ((standard-output (current-buffer))
+        (eval-buffers eval-buffer-list))
+    (require 'help-mode)     ; Define `help-function-def' button type.
+    (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
+      (insert (if (plist-get flags :debug-on-exit)
+                  "* " "  "))
+      (let ((fun-file (and do-xrefs (symbol-file fun 'defun)))
+            (fun-pt (point)))
+        (cond
+         ((and evald (not debugger-stack-frame-as-list))
+          (prin1 fun)
+          (if args (prin1 args) (princ "()")))
+         (t
+          (prin1 (cons fun args))
+          (cl-incf fun-pt)))
+        (when fun-file
+          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
+                            :type 'help-function-def
+                            'help-args (list fun fun-file))))
+      ;; After any frame that uses eval-buffer, insert a line that
+      ;; states the buffer position it's reading at.
+      (when (and eval-buffers (memq fun '(eval-buffer eval-region)))
+        (insert (format "  ; Reading at buffer position %d"
+                        ;; This will get the wrong result if there are
+                        ;; two nested eval-region calls for the same
+                        ;; buffer.  That's not a very useful case.
+                        (with-current-buffer (pop eval-buffers)
+                          (point)))))
+      (insert "\n"))))
+
 (defun debugger-setup-buffer (args)
   "Initialize the `*Backtrace*' buffer for entry to the debugger.
 That buffer should be current already."
@@ -271,22 +305,6 @@ (defun debugger-setup-buffer (args)
   (erase-buffer)
   (set-buffer-multibyte t)		;Why was it nil ?  -stef
   (setq buffer-undo-list t)
-  (let ((standard-output (current-buffer))
-	(print-escape-newlines t)
-	(print-level 8)
-        (print-length 50))
-    ;; FIXME the debugger could pass a custom callback to mapbacktrace
-    ;; instead of manipulating printed results.
-    (mapbacktrace #'backtrace--print-frame 'debug))
-  (goto-char (point-min))
-  (delete-region (point)
-		 (progn
-                   (forward-line (if (eq (car args) 'debug)
-                                     ;; Remove debug--implement-debug-on-entry
-                                     ;; and the advice's `apply' frame.
-				     3
-				   1))
-		   (point)))
   (insert "Debugger entered")
   ;; lambda is for debug-on-call when a function call is next.
   ;; debug is for debug-on-entry function called.
@@ -301,10 +319,7 @@ (defun debugger-setup-buffer (args)
        (setq pos (point))
        (setq debugger-value (nth 1 args))
        (prin1 debugger-value (current-buffer))
-       (insert ?\n)
-       (delete-char 1)
-       (insert ? )
-       (beginning-of-line))
+       (insert ?\n))
       ;; Watchpoint triggered.
       ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
        (insert
@@ -341,23 +356,20 @@ (defun debugger-setup-buffer (args)
                   (cdr args) args)
               (current-buffer))
        (insert ?\n)))
+    (let ((frames (nthcdr
+                   ;; Remove debug--implement-debug-on-entry and the
+                   ;; advice's `apply' frame.
+                   (if (eq (car args) 'debug) 3 1)
+                   (backtrace-frames 'debug)))
+          (print-escape-newlines t)
+          (print-level 8)
+          (print-length 50))
+      (when (eq (car args) 'exit)
+        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))
+      (debugger-insert-backtrace frames t))
     ;; Place point on "stack frame 0" (bug#15101).
-    (goto-char pos))
-  ;; After any frame that uses eval-buffer,
-  ;; insert a line that states the buffer position it's reading at.
-  (save-excursion
-    (let ((tem eval-buffer-list))
-      (while (and tem
-		  (re-search-forward "^  eval-\\(buffer\\|region\\)(" nil t))
-	(end-of-line)
-	(insert (format "  ; Reading at buffer position %d"
-			;; This will get the wrong result
-			;; if there are two nested eval-region calls
-			;; for the same buffer.  That's not a very useful case.
-			(with-current-buffer (car tem)
-			  (point))))
-	(pop tem))))
-  (debugger-make-xrefs))
+    (goto-char pos)))
+
 
 (defun debugger-make-xrefs (&optional buffer)
   "Attach cross-references to function names in the `*Backtrace*' buffer."
-- 
2.11.1


[-- Attachment #3: patch --]
[-- Type: text/plain, Size: 7008 bytes --]

From 65b27e283c7037a049969ba03098b59018ff3553 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 17:19:41 -0500
Subject: [PATCH v4 2/6] Improve ert backtrace recording

Change ert to use the new `backtrace-frames' function instead of
collecting frames one by one with `backtrace-frame'.  Additionally,
collect frames starting from `signal' instead the somewhat arbitrary
"6 from the bottom".  Skipping 6 frames would skip the expression that
actually caused the signal that triggered the debugger.  Possibly 6
was chosen because in the case of a failed test, the triggering frame
is an `ert-fail' call, which is not so interesting.  But in case of
test throwing an error, this drops the `error' call which is too much.

* lisp/emacs-lisp/ert.el (ert--print-backtrace): Remove.
(ert--print-backtrace): Add DO-XREFS parameter, delegate to
`debugger-insert-backtrace'.
(ert--run-test-debugger): Record the backtrace frames starting from
the instigating `signal' call.
(ert-run-tests-batch): Pass nil for `ert--print-backtrace's new
DO-XREFS parameter.
(ert-results-pop-to-backtrace-for-test-at-point): Pass t as DO-XREFS
to `ert--print-backtrace' and remove call to `debugger-make-xrefs'.
* test/lisp/emacs-lisp/ert-tests.el (ert-test-record-backtrace): Check
the backtrace list instead of comparing its string representation.
Expect `signal' to be the first frame.
---
 lisp/emacs-lisp/ert.el            | 62 ++++++++++++---------------------------
 test/lisp/emacs-lisp/ert-tests.el |  8 ++---
 2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 2c49a634e3..402798603a 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -670,48 +670,12 @@ (cl-defstruct (ert-test-skipped (:include ert-test-result-with-condition)))
 (cl-defstruct (ert-test-aborted-with-non-local-exit
                (:include ert-test-result)))
 
-
-(defun ert--record-backtrace ()
-  "Record the current backtrace (as a list) and return it."
-  ;; Since the backtrace is stored in the result object, result
-  ;; objects must only be printed with appropriate limits
-  ;; (`print-level' and `print-length') in place.  For interactive
-  ;; use, the cost of ensuring this possibly outweighs the advantage
-  ;; of storing the backtrace for
-  ;; `ert-results-pop-to-backtrace-for-test-at-point' given that we
-  ;; already have `ert-results-rerun-test-debugging-errors-at-point'.
-  ;; For batch use, however, printing the backtrace may be useful.
-  (cl-loop
-   ;; 6 is the number of frames our own debugger adds (when
-   ;; compiled; more when interpreted).  FIXME: Need to describe a
-   ;; procedure for determining this constant.
-   for i from 6
-   for frame = (backtrace-frame i)
-   while frame
-   collect frame))
-
-(defun ert--print-backtrace (backtrace)
+(defun ert--print-backtrace (backtrace do-xrefs)
   "Format the backtrace BACKTRACE to the current buffer."
-  ;; This is essentially a reimplementation of Fbacktrace
-  ;; (src/eval.c), but for a saved backtrace, not the current one.
   (let ((print-escape-newlines t)
         (print-level 8)
         (print-length 50))
-    (dolist (frame backtrace)
-      (pcase-exhaustive frame
-        (`(nil ,special-operator . ,arg-forms)
-         ;; Special operator.
-         (insert
-          (format "  %S\n" (cons special-operator arg-forms))))
-        (`(t ,fn . ,args)
-         ;; Function call.
-         (insert (format "  %S(" fn))
-         (cl-loop for firstp = t then nil
-                  for arg in args do
-                  (unless firstp
-                    (insert " "))
-                  (insert (format "%S" arg)))
-         (insert ")\n"))))))
+    (debugger-insert-backtrace backtrace do-xrefs)))
 
 ;; A container for the state of the execution of a single test and
 ;; environment data needed during its execution.
@@ -750,7 +714,19 @@ (defun ert--run-test-debugger (info args)
                       ((quit) 'quit)
 		      ((ert-test-skipped) 'skipped)
                       (otherwise 'failed)))
-              (backtrace (ert--record-backtrace))
+              ;; We store the backtrace in the result object for
+              ;; `ert-results-pop-to-backtrace-for-test-at-point'.
+              ;; This means we have to limit `print-level' and
+              ;; `print-length' when printing result objects.  That
+              ;; might not be worth while when we can also use
+              ;; `ert-results-rerun-test-debugging-errors-at-point',
+              ;; (i.e., when running interactively) but having the
+              ;; backtrace ready for printing is important for batch
+              ;; use.
+              ;;
+              ;; Grab the frames starting from `signal', frames below
+              ;; that are all from the debugger.
+              (backtrace (backtrace-frames 'signal))
               (infos (reverse ert--infos)))
          (setf (ert--test-execution-info-result info)
                (cl-ecase type
@@ -1409,8 +1385,9 @@ (defun ert-run-tests-batch (&optional selector)
               (ert-test-result-with-condition
                (message "Test %S backtrace:" (ert-test-name test))
                (with-temp-buffer
-                 (ert--print-backtrace (ert-test-result-with-condition-backtrace
-                                        result))
+                 (ert--print-backtrace
+                  (ert-test-result-with-condition-backtrace result)
+                  nil)
                  (goto-char (point-min))
                  (while (not (eobp))
                    (let ((start (point))
@@ -2420,8 +2397,7 @@ (defun ert-results-pop-to-backtrace-for-test-at-point ()
            ;; Use unibyte because `debugger-setup-buffer' also does so.
            (set-buffer-multibyte nil)
            (setq truncate-lines t)
-           (ert--print-backtrace backtrace)
-           (debugger-make-xrefs)
+           (ert--print-backtrace backtrace t)
            (goto-char (point-min))
            (insert (substitute-command-keys "Backtrace for test `"))
            (ert-insert-test-name-button (ert-test-name test))
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index fc5790c365..317838b250 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -367,12 +367,8 @@ (ert-deftest ert-test-record-backtrace ()
          (test (make-ert-test :body test-body))
          (result (ert-run-test test)))
     (should (ert-test-failed-p result))
-    (with-temp-buffer
-      (ert--print-backtrace (ert-test-failed-backtrace result))
-      (goto-char (point-min))
-      (end-of-line)
-      (let ((first-line (buffer-substring-no-properties (point-min) (point))))
-        (should (equal first-line (format "  %S()" test-body)))))))
+    (should (eq (nth 1 (car (ert-test-failed-backtrace result)))
+                'signal))))
 
 (ert-deftest ert-test-messages ()
   :tags '(:causes-redisplay)
-- 
2.11.1


[-- Attachment #4: patch --]
[-- Type: text/plain, Size: 4891 bytes --]

From dc941652d3b7f37fbe950b68bfbea00e0a626513 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 18:13:54 -0500
Subject: [PATCH v4 3/6] Escape control characters in backtraces (Bug#6991)

* src/print.c (syms_of_print): Add new variable,
print-escape-control-characters.
(print_object): Print control characters with octal escape codes when
print-escape-control-characters is true.
* lisp/subr.el (backtrace):
* lisp/emacs-lisp/debug.el (debugger-setup-buffer): Bind
`print-escape-control-characters' to t.
---
 lisp/emacs-lisp/debug.el |  1 +
 lisp/subr.el             |  3 ++-
 src/print.c              | 45 +++++++++++++++++++++++++++++++++------------
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 0c8306d428..effe7f0cb3 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -362,6 +362,7 @@ (defun debugger-setup-buffer (args)
                    (if (eq (car args) 'debug) 3 1)
                    (backtrace-frames 'debug)))
           (print-escape-newlines t)
+          (print-escape-control-characters t)
           (print-level 8)
           (print-length 50))
       (when (eq (car args) 'exit)
diff --git a/lisp/subr.el b/lisp/subr.el
index d0c8517c54..a9edff6166 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4514,7 +4514,8 @@ (defun backtrace--print-frame (evald func args flags)
 (defun backtrace ()
   "Print a trace of Lisp function calls currently active.
 Output stream used is value of `standard-output'."
-  (let ((print-level (or print-level 8)))
+  (let ((print-level (or print-level 8))
+        (print-escape-control-characters t))
     (mapbacktrace #'backtrace--print-frame 'backtrace)))
 
 (defun backtrace-frames (&optional base)
diff --git a/src/print.c b/src/print.c
index 6bf8af9ef9..50c75d7712 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1870,21 +1870,36 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 		}
 	      else
 		{
+                  bool still_need_nonhex = false;
 		  /* If we just had a hex escape, and this character
 		     could be taken as part of it,
 		     output `\ ' to prevent that.  */
-		  if (need_nonhex && c_isxdigit (c))
-		    print_c_string ("\\ ", printcharfun);
-
-		  if (c == '\n' && print_escape_newlines
-		      ? (c = 'n', true)
-		      : c == '\f' && print_escape_newlines
-		      ? (c = 'f', true)
-		      : c == '\"' || c == '\\')
-		    printchar ('\\', printcharfun);
-
-		  printchar (c, printcharfun);
-		  need_nonhex = false;
+                  if (c_isxdigit (c))
+                    {
+                      if (need_nonhex)
+                        print_c_string ("\\ ", printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (c == '\n' && print_escape_newlines
+                           ? (c = 'n', true)
+                           : c == '\f' && print_escape_newlines
+                           ? (c = 'f', true)
+                           : c == '\0' && print_escape_control_characters
+                           ? (c = '0', still_need_nonhex = true)
+                           : c == '\"' || c == '\\')
+                    {
+                      printchar ('\\', printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (print_escape_control_characters && c_iscntrl (c))
+                    {
+                      char outbuf[1 + 3 + 1];
+                      int len = sprintf (outbuf, "\\%03o", c + 0u);
+                      strout (outbuf, len, len, printcharfun);
+                    }
+                  else
+                    printchar (c, printcharfun);
+		  need_nonhex = still_need_nonhex;
 		}
 	    }
 	  printchar ('\"', printcharfun);
@@ -2329,6 +2344,11 @@ syms_of_print (void)
 Also print formfeeds as `\\f'.  */);
   print_escape_newlines = 0;
 
+  DEFVAR_BOOL ("print-escape-control-characters", print_escape_control_characters,
+	       doc: /* Non-nil means print control characters in strings as `\\OOO'.
+\(OOO is the octal representation of the character code.)*/);
+  print_escape_control_characters = 0;
+
   DEFVAR_BOOL ("print-escape-nonascii", print_escape_nonascii,
 	       doc: /* Non-nil means print unibyte non-ASCII chars in strings as \\OOO.
 \(OOO is the octal representation of the character code.)
@@ -2418,6 +2438,7 @@ representation) and `#N#' in place of each subsequent occurrence,
   DEFSYM (Qprint_escape_newlines, "print-escape-newlines");
   DEFSYM (Qprint_escape_multibyte, "print-escape-multibyte");
   DEFSYM (Qprint_escape_nonascii, "print-escape-nonascii");
+  DEFSYM (Qprint_escape_control_characters, "print-escape-control-characters");
 
   print_prune_charset_plist = Qnil;
   staticpro (&print_prune_charset_plist);
-- 
2.11.1


[-- Attachment #5: patch --]
[-- Type: text/plain, Size: 2053 bytes --]

From db0b3569a4baac81c40c86da88e93f4ae6561b79 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 27 May 2017 22:40:46 -0400
Subject: [PATCH v4 4/6] Don't redundantly cl-print arglist in function
 docstring again

* lisp/emacs-lisp/cl-print.el (cl-print-object): Don't print arglist
part of docstring.
* test/lisp/emacs-lisp/cl-print-tests.el (cl-print-tests-1): Update
test accordingly.
---
 lisp/emacs-lisp/cl-print.el            | 9 +++++----
 test/lisp/emacs-lisp/cl-print-tests.el | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lisp/emacs-lisp/cl-print.el b/lisp/emacs-lisp/cl-print.el
index 89a71d1b6c..824d0b7b4f 100644
--- a/lisp/emacs-lisp/cl-print.el
+++ b/lisp/emacs-lisp/cl-print.el
@@ -105,10 +105,11 @@ (cl-defmethod cl-print-object ((object compiled-function) stream)
     (if args
         (prin1 args stream)
       (princ "()" stream)))
-  (let ((doc (documentation object 'raw)))
-    (when doc
-      (princ " " stream)
-      (prin1 doc stream)))
+  (pcase (help-split-fundoc (documentation object 'raw) object)
+    ;; Drop args which `help-function-arglist' already printed.
+    (`(,_usage . ,(and doc (guard (stringp doc))))
+     (princ " " stream)
+     (prin1 doc stream)))
   (let ((inter (interactive-form object)))
     (when inter
       (princ " " stream)
diff --git a/test/lisp/emacs-lisp/cl-print-tests.el b/test/lisp/emacs-lisp/cl-print-tests.el
index dfbe18d784..6448a1b37f 100644
--- a/test/lisp/emacs-lisp/cl-print-tests.el
+++ b/test/lisp/emacs-lisp/cl-print-tests.el
@@ -34,7 +34,7 @@ (ert-deftest cl-print-tests-1 ()
     (let ((print-circle t))
       (should (equal (cl-prin1-to-string `((x . ,x) (y . ,x)))
                      "((x . #1=#s(cl-print--test :a 1 :b 2)) (y . #1#))")))
-    (should (string-match "\\`#f(compiled-function (x) .*\n\n.*)\\'"
+    (should (string-match "\\`#f(compiled-function (x) \"[^\"]+\" [^\)]*)\\'"
                           (cl-prin1-to-string (symbol-function #'caar))))))
 
 (ert-deftest cl-print-tests-2 ()
-- 
2.11.1


[-- Attachment #6: patch --]
[-- Type: text/plain, Size: 2912 bytes --]

From f4a9c861f2b14f9958925d328c7ce19f6eca0e2a Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 11 Jun 2017 09:51:38 -0400
Subject: [PATCH v4 5/6] Hide byte code in backtraces (Bug#6991)

* lisp/emacs-lisp/debug.el (debugger-print-function): New defcustom,
defaulting to `cl-print'.
(debugger-insert-backtrace): Use it.
* etc/NEWS: Announce it.
---
 etc/NEWS                 |  5 +++++
 lisp/emacs-lisp/debug.el | 15 ++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c7a5674e51..21510fe539 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -320,6 +320,11 @@ questions, with a handy way to display help texts.
 all call stack frames in a Lisp backtrace buffer as lists.  Both
 debug.el and edebug.el have been updated to heed to this variable.
 
+---
+** Values in call stack frames are now displayed using 'cl-prin1'.
+The old behaviour of using 'prin1' can be restored by customizing the
+new option 'debugger-print-function'.
+
 +++
 ** The new variable 'x-ctrl-keysym' has been added to the existing
 roster of X keysyms.  It can be used in combination with another
diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index effe7f0cb3..78e29c4f17 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -49,6 +49,12 @@ (defcustom debugger-batch-max-lines 40
   :group 'debugger
   :version "21.1")
 
+(defcustom debugger-print-function #'cl-prin1
+  "Function used to print values in the debugger backtraces."
+  :type 'function
+  :options '(cl-prin1 prin1)
+  :version "26.1")
+
 (defcustom debugger-bury-or-kill 'bury
   "What to do with the debugger buffer when exiting `debug'.
 The value affects the behavior of operations on any window
@@ -265,10 +271,13 @@ (defun debug (&rest args)
       debugger-value)))
 \f
 
+(defvar cl-print-compiled-button)
+
 (defun debugger-insert-backtrace (frames do-xrefs)
   "Format and insert the backtrace FRAMES at point.
 Make functions into cross-reference buttons if DO-XREFS is non-nil."
   (let ((standard-output (current-buffer))
+        (cl-print-compiled-button t)
         (eval-buffers eval-buffer-list))
     (require 'help-mode)     ; Define `help-function-def' button type.
     (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
@@ -278,10 +287,10 @@ (defun debugger-insert-backtrace (frames do-xrefs)
             (fun-pt (point)))
         (cond
          ((and evald (not debugger-stack-frame-as-list))
-          (prin1 fun)
-          (if args (prin1 args) (princ "()")))
+          (funcall debugger-print-function fun)
+          (if args (funcall debugger-print-function args) (princ "()")))
          (t
-          (prin1 (cons fun args))
+          (funcall debugger-print-function (cons fun args))
           (cl-incf fun-pt)))
         (when fun-file
           (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
-- 
2.11.1


[-- Attachment #7: patch --]
[-- Type: text/plain, Size: 2751 bytes --]

From bd576ed162cd3379cb945d8796d158d9563aa48e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 19:47:55 -0500
Subject: [PATCH v4 6/6] Escape NUL bytes in X selections (Bug#6991)

* lisp/term/w32-win.el (w32--set-selection):
* lisp/select.el (xselect--encode-string): Replace NUL bytes with
"\0".
* doc/emacs/killing.texi: Document new behavior.
* etc/NEWS (times): Announce it.
---
 doc/emacs/killing.texi | 4 ++++
 etc/NEWS               | 4 ++++
 lisp/select.el         | 3 +++
 lisp/term/w32-win.el   | 2 +-
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/killing.texi b/doc/emacs/killing.texi
index 47de053129..0b5efd04a1 100644
--- a/doc/emacs/killing.texi
+++ b/doc/emacs/killing.texi
@@ -519,6 +519,10 @@ Clipboard
 data to the clipboard manager, change the variable
 @code{x-select-enable-clipboard-manager} to @code{nil}.
 
+  Since strings containing NUL bytes are usually truncated when passed
+through the clipboard, Emacs replaces such characters with ``\0''
+before transfering them to the system's clipboard.
+
 @vindex select-enable-primary
 @findex clipboard-kill-region
 @findex clipboard-kill-ring-save
diff --git a/etc/NEWS b/etc/NEWS
index 21510fe539..281bacffd0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -326,6 +326,10 @@ The old behaviour of using 'prin1' can be restored by customizing the
 new option 'debugger-print-function'.
 
 +++
+** NUL bytes in strings copied to the system clipboard are now
+replaced with "\0".
+
++++
 ** The new variable 'x-ctrl-keysym' has been added to the existing
 roster of X keysyms.  It can be used in combination with another
 variable of this kind to swap modifiers in Emacs.
diff --git a/lisp/select.el b/lisp/select.el
index 4849d7d515..579c5c7e2e 100644
--- a/lisp/select.el
+++ b/lisp/select.el
@@ -475,6 +475,9 @@ (defun xselect--encode-string (type str &optional can-modify)
 	   (t
 	    (error "Unknown selection type: %S" type)))))
 
+      ;; Most programs are unable to handle NUL bytes in strings.
+      (setq str (replace-regexp-in-string "\0" "\\0" str t t))
+
       (setq next-selection-coding-system nil)
       (cons type str))))
 
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index fda93884c4..be895a040d 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -396,7 +396,7 @@ (declare-function w32-selection-exists-p "w32select.c")
 ;;; Fix interface to (X-specific) mouse.el
 (defun w32--set-selection (type value)
   (if (eq type 'CLIPBOARD)
-      (w32-set-clipboard-data value)
+      (w32-set-clipboard-data (replace-regexp-in-string "\0" "\\0" value t t))
     (put 'x-selections (or type 'PRIMARY) value)))
 
 (defun w32--get-selection  (&optional type data-type)
-- 
2.11.1

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Stefan Monnier]

>>> +        (when fun-file
>>> +          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
>>> +                            :type 'help-function-def
>>> +                            'help-args (list fun fun-file))))
>> Hmm... this looks like code which was moved from elsewhere, yet I can't
>> find this elsewhere in your patch(es).
>> I think that other code is in debugger-make-xrefs, so can't we remove
>> debugger-make-xrefs?
> I'm not sure exactly what you mean by "looks like code which was moved".

It's not functionality described as new in the commit log, so it's
presumably behavior which was earlier implemented elsewhere.

> It does replace the functionality of debugger-make-xrefs.  But
> `ert--make-xrefs-region' is still using `debugger-make-xrefs', and I
> don't quite see how to remove that usage.

I see.  Maybe we should move it to ert.el, then?

>>> +    (let ((frames (nthcdr
>>> +                   ;; Remove debug--implement-debug-on-entry and the
>>> +                   ;; advice's `apply' frame.
>>> +                   (if (eq (car args) 'debug) 3 1)
>>> +                   (backtrace-frames 'debug)))
>>> +          (print-escape-newlines t)
>>> +          (print-level 8)
>>> +          (print-length 50))
>> 
>> Why let-bind print-* here rather than inside debugger-insert-backtrace?

> I thought moving those inside might needlessly make the function less
> flexible, though nobody is currently making use of the flexibility so
> maybe it's not worth it...

Hmm... I can agree with it for level and length, but I think that the
escape-newline behavior is indispensable.

>>> +      (when (eq (car args) 'exit)
>>> +        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))
>> 
>> This looks like code which was moved from elsewhere, yet I can't find
>> this elsewhere in your patch(es).  What am I missing?

> backtrace--print-frame I guess?  I haven't changed the printing for
> `backtrace', perhaps I should...

Hmm... I don't see anything that corresponds to this setf in
backtrace--print-frame.  What do the above 2 lines do, and where is the
corresponding code in the current debug.el?  Or is that a new feature in
your code?  (if so, where is it documented in the commit message?)


        Stefan

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> It does replace the functionality of debugger-make-xrefs.  But
>> `ert--make-xrefs-region' is still using `debugger-make-xrefs', and I
>> don't quite see how to remove that usage.
>
> I see.  Maybe we should move it to ert.el, then?

Sure.

>>>> +          (print-escape-newlines t)
>>>> +          (print-level 8)
>>>> +          (print-length 50))
>>> 
>>> Why let-bind print-* here rather than inside debugger-insert-backtrace?
>
>> I thought moving those inside might needlessly make the function less
>> flexible, though nobody is currently making use of the flexibility so
>> maybe it's not worth it...
>
> Hmm... I can agree with it for level and length, but I think that the
> escape-newline behavior is indispensable.

Okay, that makes sense.

>>>> +      (when (eq (car args) 'exit)
>>>> +        (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil))
>>> 
>>> This looks like code which was moved from elsewhere, yet I can't find
>>> this elsewhere in your patch(es).  What am I missing?
>
>> backtrace--print-frame I guess?  I haven't changed the printing for
>> `backtrace', perhaps I should...
>
> Hmm... I don't see anything that corresponds to this setf in
> backtrace--print-frame.  What do the above 2 lines do, and where is the
> corresponding code in the current debug.el?  Or is that a new feature in
> your code?  (if so, where is it documented in the commit message?)

Ah, sorry, my memory of the old code got a little fuzzy, it doesn't
correspond to backtrace--print-frame (that function only contains the
code which reads the :debug-on-exit flag).  It's actually replacing the
code removed in this hunk:

@@ -301,10 +319,7 @@ (defun debugger-setup-buffer (args)
        (setq pos (point))
        (setq debugger-value (nth 1 args))
        (prin1 debugger-value (current-buffer))
-       (insert ?\n)
-       (delete-char 1)
-       (insert ? )
-       (beginning-of-line))
+       (insert ?\n))
       ;; Watchpoint triggered.
       ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
        (insert

So it's another instance of operating on the backtrace frame object
directly, instead of manipulating the text after printing (i.e.,
unsetting the :debug-on-exit flag instead of erasing its representation
"*" in the buffer).

Also, as I'm looking at this, I wonder if I should replace the (prin1
debugger-value ...) calls with (funcall debugger-print-function ...)
too.  Hmm, and I probably shouldn't have moved those print-*
let-bindings at all because they could be relevant to the code printing
"frame 0".

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Stefan Monnier]

> Ah, sorry, my memory of the old code got a little fuzzy, it doesn't
> correspond to backtrace--print-frame (that function only contains the
> code which reads the :debug-on-exit flag).  It's actually replacing the
> code removed in this hunk:

> @@ -301,10 +319,7 @@ (defun debugger-setup-buffer (args)
>         (setq pos (point))
>         (setq debugger-value (nth 1 args))
>         (prin1 debugger-value (current-buffer))
> -       (insert ?\n)
> -       (delete-char 1)
> -       (insert ? )
> -       (beginning-of-line))
> +       (insert ?\n))
>        ;; Watchpoint triggered.
>        ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
>         (insert

> So it's another instance of operating on the backtrace frame object
> directly, instead of manipulating the text after printing (i.e.,
> unsetting the :debug-on-exit flag instead of erasing its representation
> "*" in the buffer).

Ah, yes, I see it now, thanks.  It's a good change, then: the new code
is more clear.  I wonder why we do that, tho:
the previous code didn't have a comment, so I'm left guessing that maybe
it's that we don't want to advertise as "will stop when exiting foo"
a function which we're exiting?

> Also, as I'm looking at this, I wonder if I should replace the (prin1
> debugger-value ...) calls with (funcall debugger-print-function ...)

Sounds right.

> too.  Hmm, and I probably shouldn't have moved those print-*
> let-bindings at all because they could be relevant to the code printing
> "frame 0".

Good point.


        Stefan

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

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

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>  I wonder why we do that, tho:
> the previous code didn't have a comment, so I'm left guessing that maybe
> it's that we don't want to advertise as "will stop when exiting foo"
> a function which we're exiting?

I tried git-blame, but that code seems to have been like that since
"initial revision" (1991).  I think your guess sounds reasonable.

Anyway, here are the updated patches.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 6186 bytes --]

From 9ff1fc669d1239ac6e84f6fd045f18ec5483f552 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 09:19:00 -0500
Subject: [PATCH 1/6] Operate on frame list instead of printed backtrace

* lisp/emacs-lisp/debug.el (debugger-insert-backtrace): New function,
prints the given backtrace frames.
(debugger-setup-buffer): Use it instead of editing the backtrace
buffer text.
---
 lisp/emacs-lisp/debug.el | 97 +++++++++++++++++++++++++++---------------------
 1 file changed, 55 insertions(+), 42 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 83456fc31a..1bb1960d07 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -264,6 +264,40 @@ (defun debug (&rest args)
       (setq debug-on-next-call debugger-step-after-exit)
       debugger-value)))
 \f
+
+(defun debugger-insert-backtrace (frames do-xrefs)
+  "Format and insert the backtrace FRAMES at point.
+Make functions into cross-reference buttons if DO-XREFS is non-nil."
+  (let ((standard-output (current-buffer))
+        (eval-buffers eval-buffer-list))
+    (require 'help-mode)     ; Define `help-function-def' button type.
+    (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
+      (insert (if (plist-get flags :debug-on-exit)
+                  "* " "  "))
+      (let ((fun-file (and do-xrefs (symbol-file fun 'defun)))
+            (fun-pt (point)))
+        (cond
+         ((and evald (not debugger-stack-frame-as-list))
+          (prin1 fun)
+          (if args (prin1 args) (princ "()")))
+         (t
+          (prin1 (cons fun args))
+          (cl-incf fun-pt)))
+        (when fun-file
+          (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
+                            :type 'help-function-def
+                            'help-args (list fun fun-file))))
+      ;; After any frame that uses eval-buffer, insert a line that
+      ;; states the buffer position it's reading at.
+      (when (and eval-buffers (memq fun '(eval-buffer eval-region)))
+        (insert (format "  ; Reading at buffer position %d"
+                        ;; This will get the wrong result if there are
+                        ;; two nested eval-region calls for the same
+                        ;; buffer.  That's not a very useful case.
+                        (with-current-buffer (pop eval-buffers)
+                          (point)))))
+      (insert "\n"))))
+
 (defun debugger-setup-buffer (args)
   "Initialize the `*Backtrace*' buffer for entry to the debugger.
 That buffer should be current already."
@@ -271,26 +305,19 @@ (defun debugger-setup-buffer (args)
   (erase-buffer)
   (set-buffer-multibyte t)		;Why was it nil ?  -stef
   (setq buffer-undo-list t)
-  (let ((standard-output (current-buffer))
-	(print-escape-newlines t)
-	(print-level 8)
-        (print-length 50))
-    ;; FIXME the debugger could pass a custom callback to mapbacktrace
-    ;; instead of manipulating printed results.
-    (mapbacktrace #'backtrace--print-frame 'debug))
-  (goto-char (point-min))
-  (delete-region (point)
-		 (progn
-                   (forward-line (if (eq (car args) 'debug)
-                                     ;; Remove debug--implement-debug-on-entry
-                                     ;; and the advice's `apply' frame.
-				     3
-				   1))
-		   (point)))
   (insert "Debugger entered")
   ;; lambda is for debug-on-call when a function call is next.
   ;; debug is for debug-on-entry function called.
-  (let ((pos (point)))
+  (let ((frames (nthcdr
+                 ;; Remove debug--implement-debug-on-entry and the
+                 ;; advice's `apply' frame.
+                 (if (eq (car args) 'debug) 3 1)
+                 (backtrace-frames 'debug)))
+        (print-escape-newlines t)
+        (print-escape-control-characters t)
+        (print-level 8)
+        (print-length 50)
+        (pos (point)))
     (pcase (car args)
       ((or `lambda `debug)
        (insert "--entering a function:\n")
@@ -300,11 +327,9 @@ (defun debugger-setup-buffer (args)
        (insert "--returning value: ")
        (setq pos (point))
        (setq debugger-value (nth 1 args))
-       (prin1 debugger-value (current-buffer))
-       (insert ?\n)
-       (delete-char 1)
-       (insert ? )
-       (beginning-of-line))
+       (funcall debugger-print-function debugger-value (current-buffer))
+       (setf (cl-getf (nth 3 (car frames)) :debug-on-exit) nil)
+       (insert ?\n))
       ;; Watchpoint triggered.
       ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
        (insert
@@ -327,7 +352,7 @@ (defun debugger-setup-buffer (args)
       (`error
        (insert "--Lisp error: ")
        (setq pos (point))
-       (prin1 (nth 1 args) (current-buffer))
+       (funcall debugger-print-function (nth 1 args) (current-buffer))
        (insert ?\n))
       ;; debug-on-call, when the next thing is an eval.
       (`t
@@ -337,27 +362,15 @@ (defun debugger-setup-buffer (args)
       (_
        (insert ": ")
        (setq pos (point))
-       (prin1 (if (eq (car args) 'nil)
-                  (cdr args) args)
-              (current-buffer))
+       (funcall debugger-print-function
+                (if (eq (car args) 'nil)
+                    (cdr args) args)
+                (current-buffer))
        (insert ?\n)))
+    (debugger-insert-backtrace frames t)
     ;; Place point on "stack frame 0" (bug#15101).
-    (goto-char pos))
-  ;; After any frame that uses eval-buffer,
-  ;; insert a line that states the buffer position it's reading at.
-  (save-excursion
-    (let ((tem eval-buffer-list))
-      (while (and tem
-		  (re-search-forward "^  eval-\\(buffer\\|region\\)(" nil t))
-	(end-of-line)
-	(insert (format "  ; Reading at buffer position %d"
-			;; This will get the wrong result
-			;; if there are two nested eval-region calls
-			;; for the same buffer.  That's not a very useful case.
-			(with-current-buffer (car tem)
-			  (point))))
-	(pop tem))))
-  (debugger-make-xrefs))
+    (goto-char pos)))
+
 
 (defun debugger-make-xrefs (&optional buffer)
   "Attach cross-references to function names in the `*Backtrace*' buffer."
-- 
2.11.1


[-- Attachment #3: patch --]
[-- Type: text/plain, Size: 11605 bytes --]

From be573593e5051f3b18c046e9f09b37a6f629ec5d Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 17:19:41 -0500
Subject: [PATCH 2/6] Improve ert backtrace recording

Change ert to use the new `backtrace-frames' function instead of
collecting frames one by one with `backtrace-frame'.  Additionally,
collect frames starting from `signal' instead the somewhat arbitrary
"6 from the bottom".  Skipping 6 frames would skip the expression that
actually caused the signal that triggered the debugger.  Possibly 6
was chosen because in the case of a failed test, the triggering frame
is an `ert-fail' call, which is not so interesting.  But in case of
test throwing an error, this drops the `error' call which is too much.

* lisp/emacs-lisp/debug.el (debugger-make-xrefs): Remove.
lisp/emacs-lisp/ert.el (ert--make-xrefs-region): Bring in relevant
code from `debugger-make-xrefs'.
(ert--print-backtrace): Add DO-XREFS parameter, delegate to
`debugger-insert-backtrace'.
(ert--run-test-debugger): Record the backtrace frames starting from
the instigating `signal' call.
(ert-run-tests-batch): Pass nil for `ert--print-backtrace's new
DO-XREFS parameter.
(ert-results-pop-to-backtrace-for-test-at-point): Pass t as DO-XREFS
to `ert--print-backtrace' and remove call to `debugger-make-xrefs'.
* test/lisp/emacs-lisp/ert-tests.el (ert-test-record-backtrace): Check
the backtrace list instead of comparing its string representation.
Expect `signal' to be the first frame.
---
 lisp/emacs-lisp/debug.el          | 71 --------------------------------
 lisp/emacs-lisp/ert.el            | 85 +++++++++++++++++----------------------
 test/lisp/emacs-lisp/ert-tests.el |  8 +---
 3 files changed, 38 insertions(+), 126 deletions(-)

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 1bb1960d07..a75242aa5a 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -371,77 +371,6 @@ (defun debugger-setup-buffer (args)
     ;; Place point on "stack frame 0" (bug#15101).
     (goto-char pos)))
 
-
-(defun debugger-make-xrefs (&optional buffer)
-  "Attach cross-references to function names in the `*Backtrace*' buffer."
-  (interactive "b")
-  (with-current-buffer (or buffer (current-buffer))
-    (save-excursion
-      (setq buffer (current-buffer))
-      (let ((inhibit-read-only t)
-	    (old-end (point-min)) (new-end (point-min)))
-	;; If we saved an old backtrace, find the common part
-	;; between the new and the old.
-	;; Compare line by line, starting from the end,
-	;; because that's the part that is likely to be unchanged.
-	(if debugger-previous-backtrace
-	    (let (old-start new-start (all-match t))
-	      (goto-char (point-max))
-	      (with-temp-buffer
-		(insert debugger-previous-backtrace)
-		(while (and all-match (not (bobp)))
-		  (setq old-end (point))
-		  (forward-line -1)
-		  (setq old-start (point))
-		  (with-current-buffer buffer
-		    (setq new-end (point))
-		    (forward-line -1)
-		    (setq new-start (point)))
-		  (if (not (zerop
-			    (let ((case-fold-search nil))
-			      (compare-buffer-substrings
-			       (current-buffer) old-start old-end
-			       buffer new-start new-end))))
-		      (setq all-match nil))))
-	      ;; Now new-end is the position of the start of the
-	      ;; unchanged part in the current buffer, and old-end is
-	      ;; the position of that same text in the saved old
-	      ;; backtrace.  But we must subtract (point-min) since strings are
-	      ;; indexed in origin 0.
-
-	      ;; Replace the unchanged part of the backtrace
-	      ;; with the text from debugger-previous-backtrace,
-	      ;; since that already has the proper xrefs.
-	      ;; With this optimization, we only need to scan
-	      ;; the changed part of the backtrace.
-	      (delete-region new-end (point-max))
-	      (goto-char (point-max))
-	      (insert (substring debugger-previous-backtrace
-				 (- old-end (point-min))))
-	      ;; Make the unchanged part of the backtrace inaccessible
-	      ;; so it won't be scanned.
-	      (narrow-to-region (point-min) new-end)))
-
-	;; Scan the new part of the backtrace, inserting xrefs.
-	(goto-char (point-min))
-	(while (progn
-		 (goto-char (+ (point) 2))
-		 (skip-syntax-forward "^w_")
-		 (not (eobp)))
-	  (let* ((beg (point))
-		 (end (progn (skip-syntax-forward "w_") (point)))
-		 (sym (intern-soft (buffer-substring-no-properties
-				    beg end)))
-		 (file (and sym (symbol-file sym 'defun))))
-	    (when file
-	      (goto-char beg)
-	      ;; help-xref-button needs to operate on something matched
-	      ;; by a regexp, so set that up for it.
-	      (re-search-forward "\\(\\sw\\|\\s_\\)+")
-	      (help-xref-button 0 'help-function-def sym file)))
-	  (forward-line 1))
-	(widen))
-      (setq debugger-previous-backtrace (buffer-string)))))
 \f
 (defun debugger-step-through ()
   "Proceed, stepping through subexpressions of this expression.
diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 2c49a634e3..7edc40188e 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -670,48 +670,12 @@ (cl-defstruct (ert-test-skipped (:include ert-test-result-with-condition)))
 (cl-defstruct (ert-test-aborted-with-non-local-exit
                (:include ert-test-result)))
 
-
-(defun ert--record-backtrace ()
-  "Record the current backtrace (as a list) and return it."
-  ;; Since the backtrace is stored in the result object, result
-  ;; objects must only be printed with appropriate limits
-  ;; (`print-level' and `print-length') in place.  For interactive
-  ;; use, the cost of ensuring this possibly outweighs the advantage
-  ;; of storing the backtrace for
-  ;; `ert-results-pop-to-backtrace-for-test-at-point' given that we
-  ;; already have `ert-results-rerun-test-debugging-errors-at-point'.
-  ;; For batch use, however, printing the backtrace may be useful.
-  (cl-loop
-   ;; 6 is the number of frames our own debugger adds (when
-   ;; compiled; more when interpreted).  FIXME: Need to describe a
-   ;; procedure for determining this constant.
-   for i from 6
-   for frame = (backtrace-frame i)
-   while frame
-   collect frame))
-
-(defun ert--print-backtrace (backtrace)
+(defun ert--print-backtrace (backtrace do-xrefs)
   "Format the backtrace BACKTRACE to the current buffer."
-  ;; This is essentially a reimplementation of Fbacktrace
-  ;; (src/eval.c), but for a saved backtrace, not the current one.
   (let ((print-escape-newlines t)
         (print-level 8)
         (print-length 50))
-    (dolist (frame backtrace)
-      (pcase-exhaustive frame
-        (`(nil ,special-operator . ,arg-forms)
-         ;; Special operator.
-         (insert
-          (format "  %S\n" (cons special-operator arg-forms))))
-        (`(t ,fn . ,args)
-         ;; Function call.
-         (insert (format "  %S(" fn))
-         (cl-loop for firstp = t then nil
-                  for arg in args do
-                  (unless firstp
-                    (insert " "))
-                  (insert (format "%S" arg)))
-         (insert ")\n"))))))
+    (debugger-insert-backtrace backtrace do-xrefs)))
 
 ;; A container for the state of the execution of a single test and
 ;; environment data needed during its execution.
@@ -750,7 +714,19 @@ (defun ert--run-test-debugger (info args)
                       ((quit) 'quit)
 		      ((ert-test-skipped) 'skipped)
                       (otherwise 'failed)))
-              (backtrace (ert--record-backtrace))
+              ;; We store the backtrace in the result object for
+              ;; `ert-results-pop-to-backtrace-for-test-at-point'.
+              ;; This means we have to limit `print-level' and
+              ;; `print-length' when printing result objects.  That
+              ;; might not be worth while when we can also use
+              ;; `ert-results-rerun-test-debugging-errors-at-point',
+              ;; (i.e., when running interactively) but having the
+              ;; backtrace ready for printing is important for batch
+              ;; use.
+              ;;
+              ;; Grab the frames starting from `signal', frames below
+              ;; that are all from the debugger.
+              (backtrace (backtrace-frames 'signal))
               (infos (reverse ert--infos)))
          (setf (ert--test-execution-info-result info)
                (cl-ecase type
@@ -1409,8 +1385,9 @@ (defun ert-run-tests-batch (&optional selector)
               (ert-test-result-with-condition
                (message "Test %S backtrace:" (ert-test-name test))
                (with-temp-buffer
-                 (ert--print-backtrace (ert-test-result-with-condition-backtrace
-                                        result))
+                 (ert--print-backtrace
+                  (ert-test-result-with-condition-backtrace result)
+                  nil)
                  (goto-char (point-min))
                  (while (not (eobp))
                    (let ((start (point))
@@ -1828,12 +1805,23 @@ (defun ert--make-xrefs-region (begin end)
 
 BEGIN and END specify a region in the current buffer."
   (save-excursion
-    (save-restriction
-      (narrow-to-region begin end)
-      ;; Inhibit optimization in `debugger-make-xrefs' that would
-      ;; sometimes insert unrelated backtrace info into our buffer.
-      (let ((debugger-previous-backtrace nil))
-        (debugger-make-xrefs)))))
+    (goto-char begin)
+    (while (progn
+             (goto-char (+ (point) 2))
+             (skip-syntax-forward "^w_")
+             (< (point) end))
+      (let* ((beg (point))
+             (end (progn (skip-syntax-forward "w_") (point)))
+             (sym (intern-soft (buffer-substring-no-properties
+                                beg end)))
+             (file (and sym (symbol-file sym 'defun))))
+        (when file
+          (goto-char beg)
+          ;; help-xref-button needs to operate on something matched
+          ;; by a regexp, so set that up for it.
+          (re-search-forward "\\(\\sw\\|\\s_\\)+")
+          (help-xref-button 0 'help-function-def sym file)))
+      (forward-line 1))))
 
 (defun ert--string-first-line (s)
   "Return the first line of S, or S if it contains no newlines.
@@ -2420,8 +2408,7 @@ (defun ert-results-pop-to-backtrace-for-test-at-point ()
            ;; Use unibyte because `debugger-setup-buffer' also does so.
            (set-buffer-multibyte nil)
            (setq truncate-lines t)
-           (ert--print-backtrace backtrace)
-           (debugger-make-xrefs)
+           (ert--print-backtrace backtrace t)
            (goto-char (point-min))
            (insert (substitute-command-keys "Backtrace for test `"))
            (ert-insert-test-name-button (ert-test-name test))
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index fc5790c365..317838b250 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -367,12 +367,8 @@ (ert-deftest ert-test-record-backtrace ()
          (test (make-ert-test :body test-body))
          (result (ert-run-test test)))
     (should (ert-test-failed-p result))
-    (with-temp-buffer
-      (ert--print-backtrace (ert-test-failed-backtrace result))
-      (goto-char (point-min))
-      (end-of-line)
-      (let ((first-line (buffer-substring-no-properties (point-min) (point))))
-        (should (equal first-line (format "  %S()" test-body)))))))
+    (should (eq (nth 1 (car (ert-test-failed-backtrace result)))
+                'signal))))
 
 (ert-deftest ert-test-messages ()
   :tags '(:causes-redisplay)
-- 
2.11.1


[-- Attachment #4: patch --]
[-- Type: text/plain, Size: 4336 bytes --]

From 7f67318c05006c3447e2b1074c78e10d78e5d8ec Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 18:13:54 -0500
Subject: [PATCH 3/6] Escape control characters in backtraces (Bug#6991)

* src/print.c (syms_of_print): Add new variable,
print-escape-control-characters.
(print_object): Print control characters with octal escape codes when
print-escape-control-characters is true.
* lisp/subr.el (backtrace):
* lisp/emacs-lisp/debug.el (debugger-setup-buffer): Bind
`print-escape-control-characters' to t.
---
 lisp/subr.el |  3 ++-
 src/print.c  | 45 +++++++++++++++++++++++++++++++++------------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index d0c8517c54..a9edff6166 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4514,7 +4514,8 @@ (defun backtrace--print-frame (evald func args flags)
 (defun backtrace ()
   "Print a trace of Lisp function calls currently active.
 Output stream used is value of `standard-output'."
-  (let ((print-level (or print-level 8)))
+  (let ((print-level (or print-level 8))
+        (print-escape-control-characters t))
     (mapbacktrace #'backtrace--print-frame 'backtrace)))
 
 (defun backtrace-frames (&optional base)
diff --git a/src/print.c b/src/print.c
index 6bf8af9ef9..50c75d7712 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1870,21 +1870,36 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 		}
 	      else
 		{
+                  bool still_need_nonhex = false;
 		  /* If we just had a hex escape, and this character
 		     could be taken as part of it,
 		     output `\ ' to prevent that.  */
-		  if (need_nonhex && c_isxdigit (c))
-		    print_c_string ("\\ ", printcharfun);
-
-		  if (c == '\n' && print_escape_newlines
-		      ? (c = 'n', true)
-		      : c == '\f' && print_escape_newlines
-		      ? (c = 'f', true)
-		      : c == '\"' || c == '\\')
-		    printchar ('\\', printcharfun);
-
-		  printchar (c, printcharfun);
-		  need_nonhex = false;
+                  if (c_isxdigit (c))
+                    {
+                      if (need_nonhex)
+                        print_c_string ("\\ ", printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (c == '\n' && print_escape_newlines
+                           ? (c = 'n', true)
+                           : c == '\f' && print_escape_newlines
+                           ? (c = 'f', true)
+                           : c == '\0' && print_escape_control_characters
+                           ? (c = '0', still_need_nonhex = true)
+                           : c == '\"' || c == '\\')
+                    {
+                      printchar ('\\', printcharfun);
+                      printchar (c, printcharfun);
+                    }
+                  else if (print_escape_control_characters && c_iscntrl (c))
+                    {
+                      char outbuf[1 + 3 + 1];
+                      int len = sprintf (outbuf, "\\%03o", c + 0u);
+                      strout (outbuf, len, len, printcharfun);
+                    }
+                  else
+                    printchar (c, printcharfun);
+		  need_nonhex = still_need_nonhex;
 		}
 	    }
 	  printchar ('\"', printcharfun);
@@ -2329,6 +2344,11 @@ syms_of_print (void)
 Also print formfeeds as `\\f'.  */);
   print_escape_newlines = 0;
 
+  DEFVAR_BOOL ("print-escape-control-characters", print_escape_control_characters,
+	       doc: /* Non-nil means print control characters in strings as `\\OOO'.
+\(OOO is the octal representation of the character code.)*/);
+  print_escape_control_characters = 0;
+
   DEFVAR_BOOL ("print-escape-nonascii", print_escape_nonascii,
 	       doc: /* Non-nil means print unibyte non-ASCII chars in strings as \\OOO.
 \(OOO is the octal representation of the character code.)
@@ -2418,6 +2438,7 @@ representation) and `#N#' in place of each subsequent occurrence,
   DEFSYM (Qprint_escape_newlines, "print-escape-newlines");
   DEFSYM (Qprint_escape_multibyte, "print-escape-multibyte");
   DEFSYM (Qprint_escape_nonascii, "print-escape-nonascii");
+  DEFSYM (Qprint_escape_control_characters, "print-escape-control-characters");
 
   print_prune_charset_plist = Qnil;
   staticpro (&print_prune_charset_plist);
-- 
2.11.1


[-- Attachment #5: patch --]
[-- Type: text/plain, Size: 2050 bytes --]

From 7d2e4c3ff2788fff7e5ee7481e4983eb185c8402 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 27 May 2017 22:40:46 -0400
Subject: [PATCH 4/6] Don't redundantly cl-print arglist in function docstring
 again

* lisp/emacs-lisp/cl-print.el (cl-print-object): Don't print arglist
part of docstring.
* test/lisp/emacs-lisp/cl-print-tests.el (cl-print-tests-1): Update
test accordingly.
---
 lisp/emacs-lisp/cl-print.el            | 9 +++++----
 test/lisp/emacs-lisp/cl-print-tests.el | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lisp/emacs-lisp/cl-print.el b/lisp/emacs-lisp/cl-print.el
index 89a71d1b6c..824d0b7b4f 100644
--- a/lisp/emacs-lisp/cl-print.el
+++ b/lisp/emacs-lisp/cl-print.el
@@ -105,10 +105,11 @@ (cl-defmethod cl-print-object ((object compiled-function) stream)
     (if args
         (prin1 args stream)
       (princ "()" stream)))
-  (let ((doc (documentation object 'raw)))
-    (when doc
-      (princ " " stream)
-      (prin1 doc stream)))
+  (pcase (help-split-fundoc (documentation object 'raw) object)
+    ;; Drop args which `help-function-arglist' already printed.
+    (`(,_usage . ,(and doc (guard (stringp doc))))
+     (princ " " stream)
+     (prin1 doc stream)))
   (let ((inter (interactive-form object)))
     (when inter
       (princ " " stream)
diff --git a/test/lisp/emacs-lisp/cl-print-tests.el b/test/lisp/emacs-lisp/cl-print-tests.el
index dfbe18d784..6448a1b37f 100644
--- a/test/lisp/emacs-lisp/cl-print-tests.el
+++ b/test/lisp/emacs-lisp/cl-print-tests.el
@@ -34,7 +34,7 @@ (ert-deftest cl-print-tests-1 ()
     (let ((print-circle t))
       (should (equal (cl-prin1-to-string `((x . ,x) (y . ,x)))
                      "((x . #1=#s(cl-print--test :a 1 :b 2)) (y . #1#))")))
-    (should (string-match "\\`#f(compiled-function (x) .*\n\n.*)\\'"
+    (should (string-match "\\`#f(compiled-function (x) \"[^\"]+\" [^\)]*)\\'"
                           (cl-prin1-to-string (symbol-function #'caar))))))
 
 (ert-deftest cl-print-tests-2 ()
-- 
2.11.1


[-- Attachment #6: patch --]
[-- Type: text/plain, Size: 2909 bytes --]

From 593c4758cdf1c177ab103bb506321b964c28cf21 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 11 Jun 2017 09:51:38 -0400
Subject: [PATCH 5/6] Hide byte code in backtraces (Bug#6991)

* lisp/emacs-lisp/debug.el (debugger-print-function): New defcustom,
defaulting to `cl-print'.
(debugger-insert-backtrace): Use it.
* etc/NEWS: Announce it.
---
 etc/NEWS                 |  5 +++++
 lisp/emacs-lisp/debug.el | 15 ++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c7a5674e51..21510fe539 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -320,6 +320,11 @@ questions, with a handy way to display help texts.
 all call stack frames in a Lisp backtrace buffer as lists.  Both
 debug.el and edebug.el have been updated to heed to this variable.
 
+---
+** Values in call stack frames are now displayed using 'cl-prin1'.
+The old behaviour of using 'prin1' can be restored by customizing the
+new option 'debugger-print-function'.
+
 +++
 ** The new variable 'x-ctrl-keysym' has been added to the existing
 roster of X keysyms.  It can be used in combination with another
diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index a75242aa5a..3f1b4cddb3 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -49,6 +49,12 @@ (defcustom debugger-batch-max-lines 40
   :group 'debugger
   :version "21.1")
 
+(defcustom debugger-print-function #'cl-prin1
+  "Function used to print values in the debugger backtraces."
+  :type 'function
+  :options '(cl-prin1 prin1)
+  :version "26.1")
+
 (defcustom debugger-bury-or-kill 'bury
   "What to do with the debugger buffer when exiting `debug'.
 The value affects the behavior of operations on any window
@@ -265,10 +271,13 @@ (defun debug (&rest args)
       debugger-value)))
 \f
 
+(defvar cl-print-compiled-button)
+
 (defun debugger-insert-backtrace (frames do-xrefs)
   "Format and insert the backtrace FRAMES at point.
 Make functions into cross-reference buttons if DO-XREFS is non-nil."
   (let ((standard-output (current-buffer))
+        (cl-print-compiled-button t)
         (eval-buffers eval-buffer-list))
     (require 'help-mode)     ; Define `help-function-def' button type.
     (pcase-dolist (`(,evald ,fun ,args ,flags) frames)
@@ -278,10 +287,10 @@ (defun debugger-insert-backtrace (frames do-xrefs)
             (fun-pt (point)))
         (cond
          ((and evald (not debugger-stack-frame-as-list))
-          (prin1 fun)
-          (if args (prin1 args) (princ "()")))
+          (funcall debugger-print-function fun)
+          (if args (funcall debugger-print-function args) (princ "()")))
          (t
-          (prin1 (cons fun args))
+          (funcall debugger-print-function (cons fun args))
           (cl-incf fun-pt)))
         (when fun-file
           (make-text-button fun-pt (+ fun-pt (length (symbol-name fun)))
-- 
2.11.1


[-- Attachment #7: patch --]
[-- Type: text/plain, Size: 2748 bytes --]

From f0b87839fe2cb8279acde98877f24c5e96f7a307 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 19:47:55 -0500
Subject: [PATCH 6/6] Escape NUL bytes in X selections (Bug#6991)

* lisp/term/w32-win.el (w32--set-selection):
* lisp/select.el (xselect--encode-string): Replace NUL bytes with
"\0".
* doc/emacs/killing.texi: Document new behavior.
* etc/NEWS (times): Announce it.
---
 doc/emacs/killing.texi | 4 ++++
 etc/NEWS               | 4 ++++
 lisp/select.el         | 3 +++
 lisp/term/w32-win.el   | 2 +-
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/killing.texi b/doc/emacs/killing.texi
index 47de053129..0b5efd04a1 100644
--- a/doc/emacs/killing.texi
+++ b/doc/emacs/killing.texi
@@ -519,6 +519,10 @@ Clipboard
 data to the clipboard manager, change the variable
 @code{x-select-enable-clipboard-manager} to @code{nil}.
 
+  Since strings containing NUL bytes are usually truncated when passed
+through the clipboard, Emacs replaces such characters with ``\0''
+before transfering them to the system's clipboard.
+
 @vindex select-enable-primary
 @findex clipboard-kill-region
 @findex clipboard-kill-ring-save
diff --git a/etc/NEWS b/etc/NEWS
index 21510fe539..281bacffd0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -326,6 +326,10 @@ The old behaviour of using 'prin1' can be restored by customizing the
 new option 'debugger-print-function'.
 
 +++
+** NUL bytes in strings copied to the system clipboard are now
+replaced with "\0".
+
++++
 ** The new variable 'x-ctrl-keysym' has been added to the existing
 roster of X keysyms.  It can be used in combination with another
 variable of this kind to swap modifiers in Emacs.
diff --git a/lisp/select.el b/lisp/select.el
index 4849d7d515..579c5c7e2e 100644
--- a/lisp/select.el
+++ b/lisp/select.el
@@ -475,6 +475,9 @@ (defun xselect--encode-string (type str &optional can-modify)
 	   (t
 	    (error "Unknown selection type: %S" type)))))
 
+      ;; Most programs are unable to handle NUL bytes in strings.
+      (setq str (replace-regexp-in-string "\0" "\\0" str t t))
+
       (setq next-selection-coding-system nil)
       (cons type str))))
 
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index fda93884c4..be895a040d 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -396,7 +396,7 @@ (declare-function w32-selection-exists-p "w32select.c")
 ;;; Fix interface to (X-specific) mouse.el
 (defun w32--set-selection (type value)
   (if (eq type 'CLIPBOARD)
-      (w32-set-clipboard-data value)
+      (w32-set-clipboard-data (replace-regexp-in-string "\0" "\\0" value t t))
     (put 'x-selections (or type 'PRIMARY) value)))
 
 (defun w32--get-selection  (&optional type data-type)
-- 
2.11.1

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Stefan Monnier]

> Anyway, here are the updated patches.

Looks good to me, thanks,


        Stefan

bug#6991: Please keep bytecode out of *Backtrace* buffers

[npostavs]

tags 6991 fixed
close 6991 26.1
quit

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Anyway, here are the updated patches.
>
> Looks good to me, thanks,

Thanks for the review, it was very helpful!

Pushed to master (with minor fix to make sure debugger-print-function is
used only after the commit which defines it, plus some commit message
typos).

[1: 522e3c1585]: 2017-06-29 19:29:10 -0400
  Operate on frame list instead of printed backtrace
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=522e3c15853279bf2a0ed1759c5b0ba3c9e0b7be

[2: ead545824e]: 2017-06-29 19:37:25 -0400
  Improve ert backtrace recording
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ead545824e511ab18d18b5223eab80e1f4fe3d64

[3: eb9d3eca80]: 2017-06-29 19:40:22 -0400
  Escape control characters in backtraces (Bug#6991)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=eb9d3eca801c1ea847956a96fafd29eef9bbe5d1

[4: b567c48869]: 2017-06-29 19:40:23 -0400
  Don't redundantly cl-print arglist in function docstring again
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b567c48869b1484c6b1d263afc5cb67f22e99125

[5: 0ae28c71c7]: 2017-06-29 19:40:23 -0400
  Hide byte code in backtraces (Bug#6991)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=0ae28c71c739dfecbe94a5ff6786e81021d2d1cf

[6: c87c87fcc3]: 2017-06-29 19:40:23 -0400
  Escape NUL bytes in X selections (Bug#6991)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c87c87fcc361494815bbd1d92f450b0b80a3ecbb

[7: 169532b0eb]: 2017-06-29 19:42:32 -0400
  ; Merge: Backtrace printing improvements (Bug#6991)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=169532b0ebc3acb0b1c943d0b3d8b569cd57ca4b

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Richard Stallman]

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

Why not take the bytecode _entirely_ out of the backtrace buffer?
Is it ever any use?

I proposed a week ago to print bytecode functions differently,
without showing the bytecode.

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

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Noam Postavsky]

On Sat, Nov 26, 2016 at 6:45 PM, Richard Stallman <rms@gnu.org> wrote:
>
> Why not take the bytecode _entirely_ out of the backtrace buffer?
> Is it ever any use?

It can be disassembled, and may hold useful information that would
otherwise be missing.

>
> I proposed a week ago to print bytecode functions differently,
> without showing the bytecode.

I think it might make sense to hide the bytecode with display
properties, but keep the text itself so that it's not lost when copied
to an email.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Clément Pit--Claudel]

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

On 2016-11-26 19:33, Noam Postavsky wrote:
> I think it might make sense to hide the bytecode with display
> properties, but keep the text itself so that it's not lost when copied
> to an email.

This sounds like a great idea!  Maybe with an indication that bytecode was omitted?  Something like "[bytecode]"?

I wonder if it would make sense to move `backtrace' to Lisp, too; it would make it easier to customize it.

Clément.



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

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sat, 26 Nov 2016 19:33:30 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, Juanma Barranquero <lekktu@gmail.com>, John Wiegley <johnw@gnu.org>, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, 6991@debbugs.gnu.org, 
> 	Lars Magne Ingebrigtsen <larsi@gnus.org>
> 
> I think it might make sense to hide the bytecode with display
> properties, but keep the text itself so that it's not lost when copied
> to an email.

Text properties won't prevent the hidden text from being copied.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Noam Postavsky]

On Sat, Nov 26, 2016 at 10:36 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Sat, 26 Nov 2016 19:33:30 -0500
>> Cc: Eli Zaretskii <eliz@gnu.org>, Juanma Barranquero <lekktu@gmail.com>, John Wiegley <johnw@gnu.org>,
>>       Stefan Monnier <monnier@iro.umontreal.ca>, 6991@debbugs.gnu.org,
>>       Lars Magne Ingebrigtsen <larsi@gnus.org>
>>
>> I think it might make sense to hide the bytecode with display
>> properties, but keep the text itself so that it's not lost when copied
>> to an email.
>
> Text properties won't prevent the hidden text from being copied.

Yes, that was my intention.

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Richard Stallman]

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

  > > Why not take the bytecode _entirely_ out of the backtrace buffer?
  > > Is it ever any use?

  > It can be disassembled, and may hold useful information that would
  > otherwise be missing.

That is no reason to put the bytecode in the backtrace by default.
We can make another way to get the bytecode when you really want that.

  > I think it might make sense to hide the bytecode with display
  > properties, but keep the text itself so that it's not lost when copied
  > to an email.

That might be a good approach.

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

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Richard Stallman]

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

Showing the byte-compiled code is not very useful.

Perhaps it would be good to make the byte-compiled function have the
name of the function it was compiled from as one of its elements.
We could make princ output  <byte-compiled FUNNAME ARGLIST ADDRESS>
while not changing what prin1 outputs.

Then the debugger could use princ for byte-compiled functions,
and provide some commmand to access the actual function object to examine it.

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

bug#6991: Please keep bytecode out of *Backtrace* buffers

[Lars Ingebrigtsen]

John Wiegley <jwiegley@gmail.com> writes:

>>>>>> Drew Adams <drew.adams@oracle.com> writes:
>
>> What's more, _users_ currently do the work by hand, so it must be possible
>> to at least partly (probably fully) get it done by program. If users can
>> manually (time-consuming and error-prone) redact the byte-code when pasting
>> a backtrace into a mail etc. then that can be done by program.
>
> Drew, can you show me what it will look like to have the elision performed?
> Sometimes the byte-code contains strings that give me a clue as to the
> problem, so I'm wondering what will disappear if this is fixed.

I thought the post I made yesterday showed the difference?  And it's
that the byte codes themselves get replaced by "..<bytecode>..", and not
the symbols (etc.) that are useful for figuring out backtraces.

But the patch was backwards -- it inhibited it outside of backtraces
instead of in backtraces.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6991:

[Rocky Bernstein]

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

have started looking at decompiling ELISP bytecode using the techniques
from uncompyle6 <https://pypi.python.org/pypi/uncompyle6> .

So far the results are promising. Of course one isn't going to get the
exact source text back.

For the bytecode for source text

   (setq a nil)
   (setq b nil)

when decompiled gives the equivalent:

      (setq a (setq b nil))

And macros will be in their expanded form. But I believe nevertheless
programmers will have a very good idea of what was going on when an error
was raised.

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

bug#6991:

[Eli Zaretskii]

> From: Rocky Bernstein <rocky@gnu.org>
> Date: Mon, 11 Sep 2017 06:57:58 -0400
> 
> have started looking at decompiling ELISP bytecode using the techniques from uncompyle6 . 
> 
> So far the results are promising. Of course one isn't going to get the exact source text back. 
> 
> For the bytecode for source text
> 
> (setq a nil)
> (setq b nil)
> 
> when decompiled gives the equivalent: 
> 
> (setq a (setq b nil))
> 
> And macros will be in their expanded form. But I believe nevertheless programmers will have a very good idea
> of what was going on when an error was raised.

Sounds interesting and potentially useful.  Please keep up the good
work.

Thanks.

bug#6991:

[Rocky Bernstein]

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

Thanks for the kind words. Right now I see no obstacles to getting this
done. Just lots of work, and not enough hands. I'm going to try to get the
local Emacs group interested in helping out. And if not in this, then
contributing to documenting bytecode in of itself is something that would
be of value.

What I have  right now is at https://github.com/rocky/elisp-decompile .
Alas, for expediency it is right now written in Python.

On Mon, Sep 11, 2017 at 10:28 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Rocky Bernstein <rocky@gnu.org>
> > Date: Mon, 11 Sep 2017 06:57:58 -0400
> >
> > have started looking at decompiling ELISP bytecode using the techniques
> from uncompyle6 .
> >
> > So far the results are promising. Of course one isn't going to get the
> exact source text back.
> >
> > For the bytecode for source text
> >
> > (setq a nil)
> > (setq b nil)
> >
> > when decompiled gives the equivalent:
> >
> > (setq a (setq b nil))
> >
> > And macros will be in their expanded form. But I believe nevertheless
> programmers will have a very good idea
> > of what was going on when an error was raised.
>
> Sounds interesting and potentially useful.  Please keep up the good
> work.
>
> Thanks.
>

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