bug#65128: 30.0.50; Strange code in help-fns.el

bug#65128: 30.0.50; Strange code in help-fns.el

[Ihor Radchenko]

Hi,

I am looking at lisp/help-fns.el:describe-keymap and seeing

 (let ((sym nil))
    (unless sym
    ...

It looks like an oversight - sym will always be nil there.

In GNU Emacs 30.0.50 (build 5, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.8) of 2023-08-04 built on localhost
Repository revision: ce48073f1597ceecb82800e71c89b53badc9f9d0
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: Gentoo Linux

Configured using:
 'configure --with-native-compilation=aot'


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#65128: 30.0.50; Strange code in help-fns.el

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Date: Mon, 07 Aug 2023 16:30:56 +0000
> 
> Hi,
> 
> I am looking at lisp/help-fns.el:describe-keymap and seeing
> 
>  (let ((sym nil))
>     (unless sym
>     ...
> 
> It looks like an oversight - sym will always be nil there.

Adding Stefan, who wrote that function.

bug#65128: 30.0.50; Strange code in help-fns.el

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> > I am looking at lisp/help-fns.el:describe-keymap and seeing
> >
> >  (let ((sym nil))
> >     (unless sym
> >     ...
> >
> > It looks like an oversight - sym will always be nil there.
>
> Adding Stefan, who wrote that function.

Thanks, now fixed on master.

PS. For the record, this function was originally written by Drew, and
integrated into Emacs by me.  I do get credit for the stylistic
mistake here, however.

bug#65128: 30.0.50; Strange code in help-fns.el

[Drew Adams]

> > > I am looking at lisp/help-fns.el:describe-keymap and seeing
> > >  (let ((sym nil))
> > >     (unless sym ...
> > >
> > > It looks like an oversight - sym will always be nil there.
> >
> > Adding Stefan, who wrote that function.
> 
> Thanks, now fixed on master.
> 
> PS. For the record, this function was originally written by Drew, and
> integrated into Emacs by me.  I do get credit for the stylistic
> mistake here, however.

;-) Thanks for the attribution and mea culpa.

Yes, I wrote `describe-keymap'.  My version
is in library `help-fns+.el':

 https://www.emacswiki.org/emacs/download/help-fns%2b.el

My code differs here: it tests SYM because
there's conditional code that can set it:

(let ((sym nil))
  (when search-symbols-p ; <==================
    (setq sym (catch 'describe-keymap ; <====
                (mapatoms 
                 (lambda (symb) 
                   (when (and (boundp symb)
                              (eq (symbol-value symb) keymap)
                              (not (eq symb 'keymap))
                              (throw 'describe-keymap symb)))))
                nil)))
  (unless sym ; <=============================
    (setq sym  (cl-gentemp "KEYMAP OBJECT (no variable) "))
    (set sym keymap))
  (setq keymap  sym))

Vanilla code:

(let ((sym nil))
        (unless sym
          (setq sym (cl-gentemp "KEYMAP OBJECT (no variable) "))
          (setq used-gentemp t)
          (set sym keymap))
        (setq keymap sym))

In my code, `describe-keymap' takes optional arg
SEARCH-SYMBOLS-P.  That's used non-interactively
to search all vars for one whose value is KEYMAP,
when KEYMAP isn't a symbol.  It tells you a var
(if there is one) that corresponds to the KEYMAP.
It's a feature that was suggested by a user.

Vanilla Emacs didn't want/include the feature, so
yes, the `(unless sym' test there is gratuitous
(but harmless).