|Subject:||bug#6991: Please keep bytecode out of *Backtrace* buffers|
|Date:||Sun, 25 Jun 2017 23:34:56 -0400|
|User-agent:||Gnus/5.13 (Gnus v5.13) Emacs/25.2.50 (gnu/linux)|
Stefan Monnier <address@hidden> 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!
|[Prev in Thread]||Current Thread||[Next in Thread]|