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

[npostavs]

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!

v4-0001-Operate-on-frame-list-instead-of-printed-backtrac.patch
Description: patch

v4-0002-Improve-ert-backtrace-recording.patch
Description: patch

v4-0003-Escape-control-characters-in-backtraces-Bug-6991.patch
Description: patch

v4-0004-Don-t-redundantly-cl-print-arglist-in-function-do.patch
Description: patch

v4-0005-Hide-byte-code-in-backtraces-Bug-6991.patch
Description: patch

v4-0006-Escape-NUL-bytes-in-X-selections-Bug-6991.patch
Description: patch