From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: "Daniel Martín" <mardani29@yahoo.es>
Cc: 50245@debbugs.gnu.org, Lars Ingebrigtsen <larsi@gnus.org>
Subject: bug#50245: 28.0.50; Instrumenting a function does not show "Edebug:" in the echo area
Date: Sat, 02 Oct 2021 11:06:48 -0400 [thread overview]
Message-ID: <jwvilyfmqcq.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <m1o887poee.fsf@yahoo.es> ("Daniel Martín"'s message of "Sat, 02 Oct 2021 15:06:01 +0200")
> bae2cfe63cbd11eaf348dfa7fbb2b9f7362fc747. I'm not sure, but I think
> that the author's intent was to reduce code duplication
Definitely.
> (with possibly other benefits).
This was a nasty case of duplication where the two versions worked quite
differently, yet trying to mimic the other's result. The worst part is
that depending on whether `edebug.el` was loaded either of the two codes
could be used, so any difference in behavior in the "normal C-M-x case"
was a bug, so the "mimicking" had to be as perfect as possible.
> My proposed patch first checks if edebugging is active and then avoids
> that eval-region prints by setting it's PRINTFLAG argument to nil:
>
> diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
> index acf7123225..d1a4200df2 100644
> --- a/lisp/progmodes/elisp-mode.el
> +++ b/lisp/progmodes/elisp-mode.el
> @@ -1612,6 +1612,7 @@ elisp--eval-defun
> (let ((debug-on-error eval-expression-debug-on-error)
> (print-length eval-expression-print-length)
> (print-level eval-expression-print-level)
> + (edebugging edebug-all-defs)
> elisp--eval-defun-result)
> (save-excursion
> ;; Arrange for eval-region to "read" the (possibly) altered form.
I think you'll need a (defvar edebug-all-defs) before that.
> @@ -1629,8 +1630,9 @@ elisp--eval-defun
> ;; Alter the form if necessary.
> (let ((form (eval-sexp-add-defvars
> (elisp--eval-defun-1
> - (macroexpand form)))))
> - (eval-region beg end standard-output
> + (macroexpand form))))
> + (should-print (not edebugging)))
> + (eval-region beg end should-print
> (lambda (_ignore)
> ;; Skipping to the end of the specified region
> ;; will make eval-region return.
This replaces `standard-output` with t, which is probably OK in most
cases, but just to be sure, I'd use
(should-print (if (not edebugging) standard-output)))
> This solves the problem and does not introduce any further regression.
> Any feedback on if this is TRT?
This kind of dependency on Edebug is undesirable, but it's OK.
I can see 2 other approaches:
- Refrain from emitting the message if some message was emitted already
(i.e. checking `current-message`). This is likely brittle (and would
definitely break when `standard-output` points elsewhere ;-)
- Don't print here but print from within the `eval-region`, just like we do
in the Edebug case. Arguably this would be the cleanest in terms of
interaction between the Edebug and the non-Edebug case. But it likely
requires significantly more changes and might introduce more
problems elsewhere.
> If the patch looks good, I can accompany it with a comment that
> explains the reasoning, tests, etc.
Please do and thank you.
Stefan
next prev parent reply other threads:[~2021-10-02 15:06 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <m1wno5hxij.fsf.ref@yahoo.es>
2021-08-29 1:04 ` bug#50245: 28.0.50; Instrumenting a function does not show "Edebug:" in the echo area Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-08-29 19:48 ` Lars Ingebrigtsen
2021-10-02 13:06 ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-10-02 15:06 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2022-08-22 15:39 ` Lars Ingebrigtsen
2022-08-24 22:32 ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-08-25 2:06 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-08-25 12:38 ` Lars Ingebrigtsen
2022-08-25 13:13 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-08-25 12:47 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvilyfmqcq.fsf-monnier+emacs@gnu.org \
--to=bug-gnu-emacs@gnu.org \
--cc=50245@debbugs.gnu.org \
--cc=larsi@gnus.org \
--cc=mardani29@yahoo.es \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.