bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function when reading an expression from the minibuffer

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function when reading an expression from the minibuffer

Eli Zaretskii
Stefan, any comments?

> From: Daniel Koning <[hidden email]>
> 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 <[hidden email]>
> 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
>



Reply | Threaded
Open this post in threaded view
|

bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function when reading an expression from the minibuffer

Daniel Koning
Stefan Monnier <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function when reading an expression from the minibuffer

Stefan Monnier
>> 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




Reply | Threaded
Open this post in threaded view
|

bug#41781: 27.0.91; [PATCH] Eldoc describes the wrong function when reading an expression from the minibuffer

Stefan Monnier
> 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