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

From: Stefan Monnier <[email-masked]>
Date: Sun, 25 Jun 2017 15:11:02 -0400

> 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