bug#67196: M-: uses a wrong value of debug-on-error when it is nil.

[Alan Mackenzie <acm@muc.de>]

Hello, Eli and Stefan.

On Fri, Nov 24, 2023 at 20:48:40 +0200, Eli Zaretskii wrote:
> > Date: Fri, 24 Nov 2023 17:10:47 +0000
> > Cc: Eli Zaretskii <eliz@gnu.org>, 67196@debbugs.gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > Firstly, though, there is a bug in the doc string of
> > eval-expression-debug-on-error: rather than stating what the meaning of
> > the variable is, what it's for, it states the low level details of how
> > it achieves the desired effect.  This is needlessly restrictive.  I
> > propose changing that doc string from:

> >     If non-nil set `debug-on-error' to t in `eval-expression'.
> >     If nil, don't change the value of `debug-on-error'.

> > to something like:

> >     Non-nil means enter debugger on an error in a call from `eval-expression'.
> >     Does not apply to errors handled by `condition-case' or those
> >     matched by `debug-ignored-errors'.
> >     A nil value for this variable will not prevent an entry to
> >     the debugger caused by other variables such as `debug-on-error'.

> First, the last two sentences above should be transposed, as the
> second one is not related to the 1st one, but the 3rd one is.

Done.

> And second, please try to reword so that the text is less complicated
> and easier to understand.

I'm perhaps a bit too close to it.  Apart from the first line (for which
too much information needs squashing in), I can't really see much scope
for improvement.

> Thanks.

Anyway, here's the patch of the current state.  With it, M-:
debug-on-error RET shows nil, when that is the case.  M-: (foo) enters
the debugger when an error gets signalled, assuming
eval-expression-debug-on-error is t (or a suitable list), but doesn't
enter the debugger when e-e-d-o-error is nil.

There's a slight disadvantage to the approach, namely the introduction of
a new internal variable debug-from--eval-expression which is tested from
signal_or_quit.  On the other hand, eval-expression itself has been
noticeably simplified.



diff --git a/lisp/simple.el b/lisp/simple.el
index 02c68912dba..e8a9a795c0b 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1981,11 +1981,17 @@ eval-expression-print-length
   :version "21.1")
 
 (defcustom eval-expression-debug-on-error t
-  "If non-nil set `debug-on-error' to t in `eval-expression'.
-If nil, don't change the value of `debug-on-error'."
+  "Non-nil means enter debugger on error on a call from `eval-expression'.
+Does not apply to errors handled by `condition-case' or those
+matched by `debug-ignored-errors'.
+Like `debug-on-error', this variable's value can also be a list,
+with the same meaning as for `debug-on-error'.
+
+A nil value for this variable will not prevent an entry to
+the debugger caused by other variables such as `debug-on-error'."
   :group 'lisp
   :type 'boolean
-  :version "21.1")
+  :version "30.1")
 
 (defcustom eval-expression-print-maximum-character 127
   "The largest integer that will be displayed as a character.
@@ -2120,34 +2126,19 @@ eval-expression
    (cons (read--expression "Eval: ")
          (eval-expression-get-print-arguments current-prefix-arg)))
 
-  (let (result)
-    (if (null eval-expression-debug-on-error)
-        (setq result
-              (values--store-value
-               (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-      (let ((old-value (make-symbol "t")) new-value)
-        ;; Bind debug-on-error to something unique so that we can
-        ;; detect when evalled code changes it.
-        (let ((debug-on-error old-value))
-          (setq result
-	        (values--store-value
-                 (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-	  (setq new-value debug-on-error))
-        ;; If evalled code has changed the value of debug-on-error,
-        ;; propagate that change to the global binding.
-        (unless (eq old-value new-value)
-	  (setq debug-on-error new-value))))
-
-    (let ((print-length (unless no-truncate eval-expression-print-length))
-          (print-level  (unless no-truncate eval-expression-print-level))
-          (eval-expression-print-maximum-character char-print-limit)
-          (deactivate-mark))
-      (let ((out (if insert-value (current-buffer) t)))
-        (prog1
-            (prin1 result out)
-          (let ((str (and char-print-limit
-                          (eval-expression-print-format result))))
-            (when str (princ str out))))))))
+  (let* ((debug-from--eval-expression eval-expression-debug-on-error)
+         (result (values--store-value
+                  (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
+         (print-length (unless no-truncate eval-expression-print-length))
+         (print-level  (unless no-truncate eval-expression-print-level))
+         (eval-expression-print-maximum-character char-print-limit)
+         (deactivate-mark)
+         (out (if insert-value (current-buffer) t)))
+    (prog1
+        (prin1 result out)
+      (let ((str (and char-print-limit
+                      (eval-expression-print-format result))))
+        (when str (princ str out))))))
 
 (defun edit-and-eval-command (prompt command)
   "Prompting with PROMPT, let user edit COMMAND and eval result.
diff --git a/src/eval.c b/src/eval.c
index 12e811ce264..6cadda01efb 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2033,7 +2033,8 @@ maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig, Lisp_Object data)
       /* Does user want to enter debugger for this kind of error?  */
       && (signal_quit_p (sig)
 	  ? debug_on_quit
-	  : wants_debugger (Vdebug_on_error, conditions))
+	  : (wants_debugger (Vdebug_from__eval_expression, conditions)
+	     || wants_debugger (Vdebug_on_error, conditions)))
       && ! skip_debugger (conditions, combined_data)
       /* See commentary on definition of
          `internal-when-entered-debugger'.  */
@@ -4299,6 +4300,13 @@ syms_of_eval (void)
 See also the variable `debug-on-quit' and `inhibit-debugger'.  */);
   Vdebug_on_error = Qnil;
 
+  DEFVAR_LISP ("debug-from--eval-expression", Vdebug_from__eval_expression,
+	       doc: /* Non-nil means enter debugger if an error is signaled.
+This only applies in forms called by `eval-expression'.  This variable
+has the same semantics as `debug-on-error'.  It is an internal variable
+only.  */);
+  Vdebug_from__eval_expression = Qnil;
+
   DEFVAR_LISP ("debug-ignored-errors", Vdebug_ignored_errors,
     doc: /* List of errors for which the debugger should not be called.
 Each element may be a condition-name or a regexp that matches error messages.


-- 
Alan Mackenzie (Nuremberg, Germany).