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

From: [email-masked]
Date: Mon, 26 Jun 2017 08:50:47 -0400

Stefan Monnier <[email-masked]> 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".