unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history
@ 2021-01-15  0:32 James N. V. Cash
  2021-01-15  2:31 ` Drew Adams
  2021-01-19  6:09 ` Lars Ingebrigtsen
  0 siblings, 2 replies; 6+ messages in thread
From: James N. V. Cash @ 2021-01-15  0:32 UTC (permalink / raw)
  To: 45879


It seems that the new describe-keymaps function in help-fns.el is
missing a defvar for keymap-name-history. This results in an error
when using, e.g. Helm as the completing-read function.

The attached patch adds the defvar to help-fns.el.

---
 lisp/help-fns.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index d559221a82..7be2826361 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -76,6 +76,9 @@ help-definition-prefixes
   ;; costly, really).
   "Radix-tree representation replacing `definition-prefixes'.")
 
+(defvar keymap-name-history nil
+  "History for input to `describe-keymap'.")
+
 (defun help-definition-prefixes ()
   "Return the up-to-date radix-tree form of `definition-prefixes'."
   (when (> (hash-table-count definition-prefixes) 0)
-- 
2.25.1






^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history
  2021-01-15  0:32 bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history James N. V. Cash
@ 2021-01-15  2:31 ` Drew Adams
  2021-01-15 15:08   ` James N. V. Cash
  2021-01-19  6:09 ` Lars Ingebrigtsen
  1 sibling, 1 reply; 6+ messages in thread
From: Drew Adams @ 2021-01-15  2:31 UTC (permalink / raw)
  To: James N. V. Cash, 45879

> It seems that the new describe-keymaps function in help-fns.el is
> missing a defvar for keymap-name-history. This results in an error
> when using, e.g. Helm as the completing-read function.
> 
> The attached patch adds the defvar to help-fns.el.

FWIW, I don't think it's required for the HIST arg
of `completing-read' (or `read-from-minibuffer') to
be predefined.  It can be any symbol, which is taken
as a variable - no need for a defvar.

Now maybe it can be considered good style to provide
a defvar, especially to provide a doc string.
That's another story.

But it sounds (just a guess) like it's Helm that has
a bug here.  Again though, it's good to provide a
defvar anyway.

E.g. in `emacs -Q':

 (completing-read "aaa: " obarray nil nil nil 'toto)

No error occurs, and `toto' gets populated correctly
as a variable, starting with an initial value of nil,
and regardless of input.

(And the byte-compiler issues no warning either.)

I thought this "feature" was documented, but I don't
find it now in the Elisp manual or doc strings.
Perhaps it's there somewhere.  (Not very important
anyway.)





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history
  2021-01-15  2:31 ` Drew Adams
@ 2021-01-15 15:08   ` James N. V. Cash
  0 siblings, 0 replies; 6+ messages in thread
From: James N. V. Cash @ 2021-01-15 15:08 UTC (permalink / raw)
  To: Drew Adams, 45879

Drew Adams <drew.adams@oracle.com> writes:

> FWIW, I don't think it's required for the HIST arg
> of `completing-read' (or `read-from-minibuffer') to
> be predefined.  It can be any symbol, which is taken
> as a variable - no need for a defvar.

Ah, okay -- reading the help for `completing-read', it says that HIST
"can be a symbol, which is the history list variable to use", which I
took to mean that it should refer to a variable.

> E.g. in `emacs -Q':
>
>  (completing-read "aaa: " obarray nil nil nil 'toto)
>
> No error occurs, and `toto' gets populated correctly
> as a variable, starting with an initial value of nil,
> and regardless of input.
>
> (And the byte-compiler issues no warning either.)

Indeed, I noticed this as well, but I wasn't sure if that was intended,
or just a coincidence of implementation that it happened to work.

> But it sounds (just a guess) like it's Helm that has
> a bug here.  Again though, it's good to provide a
> defvar anyway.

Hah, funnily enough, Helm also says that this an Emacs bug:
https://github.com/emacs-helm/helm/issues/2327#issuecomment-647912917

😆 Not entirely sure how to resolve this then, other than just defining
the variable myself in my own config.

> I thought this "feature" was documented, but I don't
> find it now in the Elisp manual or doc strings.
> Perhaps it's there somewhere.  (Not very important
> anyway.)

If this is indeed intended behaviour, I can submit a patch to clarify
the docstring for completing-read.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history
  2021-01-15  0:32 bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history James N. V. Cash
  2021-01-15  2:31 ` Drew Adams
@ 2021-01-19  6:09 ` Lars Ingebrigtsen
  2021-01-19 15:11   ` James N. V. Cash
  1 sibling, 1 reply; 6+ messages in thread
From: Lars Ingebrigtsen @ 2021-01-19  6:09 UTC (permalink / raw)
  To: James N. V. Cash; +Cc: 45879

"James N. V. Cash" <james.nvc@gmail.com> writes:

> It seems that the new describe-keymaps function in help-fns.el is
> missing a defvar for keymap-name-history. This results in an error
> when using, e.g. Helm as the completing-read function.
>
> The attached patch adds the defvar to help-fns.el.

Thanks; applied to Emacs 28.

As Drew pointed out, `completing-read' helpfully defines variables if
they aren't already, but I think we should define them, because not
doing so leads to the problems we see in Helm, for instance.

This change was small enough to apply without assigning copyright to the
FSF, but for future patches you want to submit, it might make sense to
get the paperwork started now, so that subsequent patches can be applied
speedily. Would you be willing to sign such paperwork?

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





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history
  2021-01-19  6:09 ` Lars Ingebrigtsen
@ 2021-01-19 15:11   ` James N. V. Cash
  2021-01-19 15:23     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 6+ messages in thread
From: James N. V. Cash @ 2021-01-19 15:11 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 45879

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thanks; applied to Emacs 28.

Thank-you!

> This change was small enough to apply without assigning copyright to the
> FSF, but for future patches you want to submit, it might make sense to
> get the paperwork started now, so that subsequent patches can be applied
> speedily. Would you be willing to sign such paperwork?

Absolutely, it would be my pleasure





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history
  2021-01-19 15:11   ` James N. V. Cash
@ 2021-01-19 15:23     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 6+ messages in thread
From: Lars Ingebrigtsen @ 2021-01-19 15:23 UTC (permalink / raw)
  To: James N. V. Cash; +Cc: 45879

James N. V. Cash <james.nvc@gmail.com> writes:

>> This change was small enough to apply without assigning copyright to the
>> FSF, but for future patches you want to submit, it might make sense to
>> get the paperwork started now, so that subsequent patches can be applied
>> speedily. Would you be willing to sign such paperwork?
>
> Absolutely, it would be my pleasure

Great!  Here's the form to get started:



Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]

[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]

[For the copyright registration, what country are you a citizen of?]

[What year were you born?]

[Please write your email address here.]

[Please write your postal address here.]

[Which files have you changed so far, and which new files have you written
so far?]






^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-01-19 15:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-15  0:32 bug#45879: 28.0.50; [PATCH] missing defvar for keymap-name-history James N. V. Cash
2021-01-15  2:31 ` Drew Adams
2021-01-15 15:08   ` James N. V. Cash
2021-01-19  6:09 ` Lars Ingebrigtsen
2021-01-19 15:11   ` James N. V. Cash
2021-01-19 15:23     ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).