GNU bug report logs - #41781
27.0.91; [PATCH] Eldoc describes the wrong function when reading an expression from the minibuffer

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41781 in the body.
You can then email your comments to 41781 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Daniel Koning <dk <at> danielkoning.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.91; [PATCH] Eldoc describes the wrong function when reading an
 expression from the minibuffer
Date: Tue, 09 Jun 2020 21:56:14 -0500

[Message part 1 (text/plain, inline)]

Severity: minor
Tags: patch

The Eldoc message shows the documentation for the wrong function (or no
documentation at all) in this specific situation:

(a) You're typing an elisp expression into `read-from-minibuffer', and
(b) the function name contains punctuation, such as ! or ?, whose
character class is "punctuation" and not "symbol" in the standard syntax
table.

Function names as described in (b) are not only legal but quite common
in third-party code. There aren't many in the standard distribution, but
you'll notice a few here and there. Try this:

(require 'pcvs)
(call-interactively #'eval-expression)
-----
Eval: (cvs-mode!
-----

The mode line shows the documentation for `cvs-mode' (without the !),
which is a different function and has a different lambda list.

This is happening because the syntax table in the minibuffer never gets
changed to the elisp table, which acknowledges all the valid symbol
characters as part of the "symbol" class.

There are a couple different spots in the code to which you could
attribute this lapse. For one, the elisp-mode.el function
`elisp--current-symbol' isn't wrapped in a `with-syntax-table', unlike
other similar definitions in the same file. I think anyone invoking this
function could reasonably expect it to observe elisp syntax, so that's
what my tiny patch addresses. This fixes the Eldoc problem.

But here's another weird thing further down the call stack.
`read--expression' has a FIXME comment saying to turn on
`emacs-lisp-mode' in the minibuffer -- which would also set the
appropriate syntax table -- but it doesn't actually do it. I guess that
must not work for whatever reason (since it has to have taken longer to
write the comment than it would have taken to add the code). Should it
be changed now so that it does set the major mode? Is there a problem
with specialized major modes in the minibuffer? I hereby kick the can
over to whoever knows more.

Daniel

[0001-lisp-progmodes-elisp-mode.el-elisp-current-symbol-Se.patch (text/x-patch, attachment)]

Message #8 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Daniel Koning <dk <at> danielkoning.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91;
 [PATCH] Eldoc describes the wrong function when reading an expression
 from the minibuffer
Date: Sat, 20 Jun 2020 10:49:14 +0300

Stefan, any comments?

> From: Daniel Koning <dk <at> danielkoning.com>
> Date: Tue, 09 Jun 2020 21:56:14 -0500
> 
> The Eldoc message shows the documentation for the wrong function (or no
> documentation at all) in this specific situation:
> 
> (a) You're typing an elisp expression into `read-from-minibuffer', and
> (b) the function name contains punctuation, such as ! or ?, whose
> character class is "punctuation" and not "symbol" in the standard syntax
> table.
> 
> Function names as described in (b) are not only legal but quite common
> in third-party code. There aren't many in the standard distribution, but
> you'll notice a few here and there. Try this:
> 
> (require 'pcvs)
> (call-interactively #'eval-expression)
> -----
> Eval: (cvs-mode!
> -----
> 
> The mode line shows the documentation for `cvs-mode' (without the !),
> which is a different function and has a different lambda list.
> 
> This is happening because the syntax table in the minibuffer never gets
> changed to the elisp table, which acknowledges all the valid symbol
> characters as part of the "symbol" class.
> 
> There are a couple different spots in the code to which you could
> attribute this lapse. For one, the elisp-mode.el function
> `elisp--current-symbol' isn't wrapped in a `with-syntax-table', unlike
> other similar definitions in the same file. I think anyone invoking this
> function could reasonably expect it to observe elisp syntax, so that's
> what my tiny patch addresses. This fixes the Eldoc problem.
> 
> But here's another weird thing further down the call stack.
> `read--expression' has a FIXME comment saying to turn on
> `emacs-lisp-mode' in the minibuffer -- which would also set the
> appropriate syntax table -- but it doesn't actually do it. I guess that
> must not work for whatever reason (since it has to have taken longer to
> write the comment than it would have taken to add the code). Should it
> be changed now so that it does set the major mode? Is there a problem
> with specialized major modes in the minibuffer? I hereby kick the can
> over to whoever knows more.
> 
> Daniel
> 
> 
> [2:text/x-patch Hide Save:0001-lisp-progmodes-elisp-mode.el-elisp-current-symbol-Se.patch (1kB)]
> 
> >From 1d4ed0e89b4ebc040609b7476128062685843c7f Mon Sep 17 00:00:00 2001
> From: Daniel Koning <dk <at> danielkoning.com>
> Date: Tue, 9 Jun 2020 19:24:11 -0500
> Subject: [PATCH] lisp/progmodes/elisp-mode.el (elisp--current-symbol): Set
>  syntax table.
> 
> Even if `elisp-current-symbol' is called from a buffer with a different
> active syntax table (such as the minibuffer in `read-from-minibuffer'),
> use `emacs-lisp-mode-syntax-table' to determine the symbol boundaries.
> 
> Copyright-paperwork-exempt: yes
> ---
>  lisp/progmodes/elisp-mode.el | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
> index f39ecf9b7b..65c01b2ce1 100644
> --- a/lisp/progmodes/elisp-mode.el
> +++ b/lisp/progmodes/elisp-mode.el
> @@ -1615,10 +1615,11 @@ elisp--beginning-of-sexp
>  
>  ;; returns nil unless current word is an interned symbol.
>  (defun elisp--current-symbol ()
> -  (let ((c (char-after (point))))
> -    (and c
> -         (memq (char-syntax c) '(?w ?_))
> -         (intern-soft (current-word)))))
> +  (with-syntax-table emacs-lisp-mode-syntax-table
> +    (let ((c (char-after (point))))
> +      (and c
> +           (memq (char-syntax c) '(?w ?_))
> +           (intern-soft (current-word))))))
>  
>  (defun elisp-function-argstring (arglist)
>    "Return ARGLIST as a string enclosed by ().
> -- 
> 2.20.1
> 

Message #11 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 41781 <at> debbugs.gnu.org, Daniel Koning <dk <at> danielkoning.com>
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Sat, 20 Jun 2020 12:51:19 -0400

>> (a) You're typing an elisp expression into `read-from-minibuffer', and
>> (b) the function name contains punctuation, such as ! or ?, whose
>> character class is "punctuation" and not "symbol" in the standard syntax
>> table.

When eldoc is activated in the minibuffer and uses the elisp backend
because we're reading an elisp expression, then the syntax-table should
also be set accordingly, indeed (not only for eldoc but also for
forward-sexp, ...).

Looks like a bug in the corresponding function (`read-expression` or nearby).

>> There are a couple different spots in the code to which you could
>> attribute this lapse. For one, the elisp-mode.el function
>> `elisp--current-symbol' isn't wrapped in a `with-syntax-table', unlike
>> other similar definitions in the same file. I think anyone invoking this
>> function could reasonably expect it to observe elisp syntax, so that's
>> what my tiny patch addresses. This fixes the Eldoc problem.

It's probably OK to do it as in your patch, yes.  Switching the
syntax-table can mess up `syntax-ppss`, so it's better if we can avoid
it, but in this specific case it seems unlikely to lead to a problem.

>> But here's another weird thing further down the call stack.
>> `read--expression' has a FIXME comment saying to turn on
>> `emacs-lisp-mode' in the minibuffer -- which would also set the
>> appropriate syntax table -- but it doesn't actually do it. I guess that
>> must not work for whatever reason (since it has to have taken longer to
>> write the comment than it would have taken to add the code). Should it
>> be changed now so that it does set the major mode?

I'm probably to blame for the comment.
I'm pretty sure just calling the major will break something.
I can't offhand tell you what, tho.  Writing the comment was faster than
trying it out and then seeing how to fix the corresponding problems.

>> Is there a problem with specialized major modes in the minibuffer?

There's the fact that it's not (never?) used, so it's a big unknown.
I think we *should* use major modes in the minibuffer (tho those major
modes will probably need to be custom tailored in most cases).
Any progress in that direction (e.g. just trying it to and reporting
the problems you encounter) will be welcome.


        Stefan

Message #14 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Koning <dk <at> danielkoning.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Mon, 22 Jun 2020 19:08:57 -0500

[Message part 1 (text/plain, inline)]

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> When eldoc is activated in the minibuffer and uses the elisp backend
> because we're reading an elisp expression, then the syntax-table should
> also be set accordingly, indeed (not only for eldoc but also for
> forward-sexp, ...).
>
> Looks like a bug in the corresponding function (`read-expression` or nearby).

OK, I've attached another patch for that function.

>>> For one, the elisp-mode.el function `elisp--current-symbol' isn't
>>> wrapped in a `with-syntax-table', unlike other similar definitions
>>> in the same file. I think anyone invoking this function could
>>> reasonably expect it to observe elisp syntax, so that's what my tiny
>>> patch addresses.
>
> It's probably OK to do it as in your patch, yes.
> 
If you change `read--expression', I don't know whether you'll think it
makes sense to add the extra overhead to `elisp--current-symbol'. It
does have a silent dependency on the syntax table, which is arguably an
inherent bug. But it's an internal function which only (right now) gets
called in one place.

> I'm pretty sure just calling the major will break something.
> I can't offhand tell you what, tho.  Writing the comment was faster than
> trying it out and then seeing how to fix the corresponding problems.

I can confirm that it suffers from the fairly significant drawback of
not working at all. Since the major replaces the minibuffer-specific
keymap, nothing ends up bound to `exit-minibuffer', so you can't even
enter an expression (or do anything else, like history paging). You
could bind `overriding-local-map' at the top of the function, but with
that and the syntax table you're getting close to implementing a de
facto new mode inside the de jure old one. At that point it might as
well be a real custom major mode, as you suggested. And then there's the
possibility that some people's `emacs-lisp-mode-hook' might contain code
that assumes it's in a real buffer and misbehaves in the minibuffer.

On the other hand, you could do what eshell does for `eshell-command'
and turn on its major mode in the minibuffer while selectively binding
C-j, C-m, M-p, etc., inside the setup hook. I don't like this design at
all: if I were to make changes to my `minibuffer-local-map' bindings,
eshell would silently ignore them. Any custom mode for minibuffer input
should inherit an existing minibuffer keymap, in my book at least.

Daniel

[0001-lisp-simple.el-read-expression-Set-syntax-table.patch (text/x-patch, attachment)]

Message #17 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Daniel Koning <dk <at> danielkoning.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Tue, 23 Jun 2020 00:17:27 -0400

>> Looks like a bug in the corresponding function (`read-expression` or nearby).
> OK, I've attached another patch for that function.

Looks good to me, thanks.

> If you change `read--expression', I don't know whether you'll think it
> makes sense to add the extra overhead to `elisp--current-symbol'.

I'm not worried about the overhead, but I don't like the potential
syntax-ppss interaction, so if it's not needed, I'd rather not go there.

> I can confirm that it suffers from the fairly significant drawback of
> not working at all.  Since the major replaces the minibuffer-specific
> keymap, nothing ends up bound to `exit-minibuffer', so you can't even
> enter an expression (or do anything else, like history paging).

Fun!  Thanks for trying ;-)

> You could bind `overriding-local-map' at the top of the function, but
> with that and the syntax table you're getting close to implementing
> a de facto new mode inside the de jure old one. At that point it might
> as well be a real custom major mode, as you suggested.

Right.  It's probably worth doing something here in the longer run, tho.
E.g. the `minibuffer-local-map` and friends should really be major mode
maps (and the inheritance between the maps should probably be reflected as
inheritance between corresponding modes).  Then you'd create an
`emacs-lisp-minibuffer-mode` by deriving from some "normal" `minibuffer-local-mode`.

> And then there's the possibility that some people's
> `emacs-lisp-mode-hook' might contain code that assumes it's in a real
> buffer and misbehaves in the minibuffer.

Indeed.

> On the other hand, you could do what eshell does for `eshell-command'
> and turn on its major mode in the minibuffer while selectively binding
> C-j, C-m, M-p, etc., inside the setup hook.  I don't like this design at
> all: if I were to make changes to my `minibuffer-local-map' bindings,
> eshell would silently ignore them.  Any custom mode for minibuffer input
> should inherit an existing minibuffer keymap, in my book at least.

Agreed.


        Stefan

Message #20 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Koning <dk <at> danielkoning.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Tue, 23 Jun 2020 08:58:11 -0500

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> Then you'd create an `emacs-lisp-minibuffer-mode` by deriving from
> some "normal" `minibuffer-local-mode`.

Do you suppose it might be sensible to rename `minibuffer-inactive-mode'
to `minibuffer-local-mode' right away (or `minibuffer-mode',
`minibuffer-base-mode', or some such)? While preserving the old name as
an alias, of course.

Right now, unless overridden by eshell or another package like it, the
minibuffer is always in `minibuffer-inactive-mode', and it's kind of
confusing that an active minibuffer calls itself inactive when you ask
for its major mode.

Daniel

Message #23 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Daniel Koning <dk <at> danielkoning.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Tue, 23 Jun 2020 10:09:05 -0400

>> Then you'd create an `emacs-lisp-minibuffer-mode` by deriving from
>> some "normal" `minibuffer-local-mode`.
> Do you suppose it might be sensible to rename
> `minibuffer-inactive-mode' to `minibuffer-local-mode' right away (or
> `minibuffer-mode', `minibuffer-base-mode', or some such)?

No, the `minibuffer-inactive-mode` is the mode used when *not* in the minibuffer.
Its main purpose is to install a special keymap (which can basically
only be really used when you have a separate minibuffer frame).

> Right now, unless overridden by eshell or another package like it, the
> minibuffer is always in `minibuffer-inactive-mode',

I don't think that's true in the sense of "you get the behavior of
minibuffer-inactive-mode" (most importantly you don't get its keymap).
It may be true in the sense of "that's what `major-mode` says", but
that's rather irrelevant.


        Stefan

Message #26 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Koning <dk <at> danielkoning.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Tue, 23 Jun 2020 11:42:39 -0500

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> Right now, unless overridden by eshell or another package like it, the
>> minibuffer is always in `minibuffer-inactive-mode',
>
> I don't think that's true in the sense of "you get the behavior of
> minibuffer-inactive-mode" (most importantly you don't get its keymap).
> It may be true in the sense of "that's what `major-mode` says", but
> that's rather irrelevant.

I see your point, but I doubt most users and elisp writers would agree
that it's irrelevant what the value of `major-mode' says in plain
English. We should be able to count on drawing some conclusions from it.

Generally, the use of one major for two very different keymaps and
program states (while circumventing the problems by changing things à la
carte) strikes me as a pattern worth deprecating. But I think we agree
on that.

Daniel

Message #29 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Daniel Koning <dk <at> danielkoning.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Tue, 23 Jun 2020 14:37:32 -0400

> I see your point, but I doubt most users and elisp writers would agree
> that it's irrelevant what the value of `major-mode' says in plain
> English. We should be able to count on drawing some conclusions from it.

What I meant is that the value of `major-mode` you see is just an
accident because the minibuffer-setup code doesn't bother to really set
a new major mode (or call kill-all-local-variables), so you get a stale
value in the `major-mode` variable.


        Stefan

Message #32 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Daniel Koning <dk <at> danielkoning.com>
Cc: monnier <at> iro.umontreal.ca, 41781 <at> debbugs.gnu.org
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Sat, 04 Jul 2020 10:57:48 +0300

> From: Daniel Koning <dk <at> danielkoning.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  41781 <at> debbugs.gnu.org
> Date: Mon, 22 Jun 2020 19:08:57 -0500
> 
> OK, I've attached another patch for that function.

Thanks, I installed this on the master branch.

Message #35 received at 41781 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 41781 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 Daniel Koning <dk <at> danielkoning.com>
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Wed, 05 Aug 2020 17:59:28 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Daniel Koning <dk <at> danielkoning.com>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  41781 <at> debbugs.gnu.org
>> Date: Mon, 22 Jun 2020 19:08:57 -0500
>> 
>> OK, I've attached another patch for that function.
>
> Thanks, I installed this on the master branch.

I've only skimmed this thread, but it seems like the patch that was
installed fixed the problem that was being discussed?  It's a bit
unclear...

So should this bug report be closed?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #40 received at 41781-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 41781-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, dk <at> danielkoning.com
Subject: Re: bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function
 when reading an expression from the minibuffer
Date: Sat, 08 Aug 2020 11:09:58 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: Daniel Koning <dk <at> danielkoning.com>,  monnier <at> iro.umontreal.ca,
>   41781 <at> debbugs.gnu.org
> Date: Wed, 05 Aug 2020 17:59:28 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Daniel Koning <dk <at> danielkoning.com>
> >> Cc: Eli Zaretskii <eliz <at> gnu.org>,  41781 <at> debbugs.gnu.org
> >> Date: Mon, 22 Jun 2020 19:08:57 -0500
> >> 
> >> OK, I've attached another patch for that function.
> >
> > Thanks, I installed this on the master branch.
> 
> I've only skimmed this thread, but it seems like the patch that was
> installed fixed the problem that was being discussed?  It's a bit
> unclear...
> 
> So should this bug report be closed?

I think it should be closed, indeed, so I have now done so.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.