On Sun, Dec 01 2013, Stefan Monnier wrote:

>>       x: 9
>>       y: 114
>>       z: (9 . 114)
>
> Nitpick: I'd rather have "x=" instead of "x:" (for me, ":" means "has type").

Well, it's a debugger for Lisp not for ML.

>> +(defun debugger--backtrace-base ()
>> +  "Return the function name that marks the top of the backtrace.
>> +See `backtrace-frame'."
>> +  (cond ((eq 'debug--implement-debug-on-entry
>> +	     (cadr (backtrace-frame 1 'debug)))
>> +	 'debug--implement-debug-on-entry)
>> +	(t 'debug)))
>
> Please use it in debugger-eval-expression as well.

OK.

>> +(defun debugger--locals-visible-p ()
>> +  "Are the local variables of the current stack frame visible?"
>> +  (save-excursion
>> +    (move-to-column 2)
>> +    (get-text-property (point) 'debugger-locals-visible-p)))
>
> The text property shouldn't have a name that ends in `-p'; these names
> are for predicates (i.e. functions returning a boolean), not for
> variables or object fields, or symbol/text properties.

OK.

>> +  (dolist (s+v locals)
>> +    (let ((symbol (car s+v))
>> +	  (value (cdr s+v))
>
> You can use (pcase-dolist (`(,symbol . ,value) locals) ...).

So many ` , . cause cancer of the semicolon.

>> + (cond ((string= (symbol-name symbol)
>> "internal-interpreter-environment")
>> +	     (cond ((or (null value)
>> +			(and (null (cdr value))
>> +			     (symbol (car value))))
>                               ^^^^^^
>                               should be `symbolp', right?
>
> `value' can also be of the form (a b t).
> I.e. (null (cdr value)) is false, but there are no values.
>
> But I tend to think that the handling of
> internal-interpreter-environment should be kept in the C code: this
> symbol should by and large not be exported to Elisp.

Done. Moved it to C.

Helmut