bug#6690: 24.0.50; tmm-get-keymap is broken

bug#6690: 24.0.50; tmm-get-keymap is broken

[Drew Adams]

emacs -Q
 
In several of the `cond' branches the variable `str' is not defined, so
it is nil from the `let' default initialization.
 
Then this code raises an error "Wrong type argument: stringp, nil",
since `str' is not a string:
 
(unless (eq km 'ignore)
          (let ((binding (where-is-internal km nil t)))
            (when binding
              (setq binding (key-description binding))
              ;; Try to align the keybindings.
              (let ((colwidth (min 30 (- (/ (window-width) 2) 10))))
 
;; STR is nil - so cannot use `string-width' or `concat'.
 
                (setq str
                      (concat str
                              (make-string (max 2 (- colwidth
                                                     (string-width str)
                                                     (string-width binding)))
                                           ?\s)
                              binding))))))
 

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2010-07-19 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4) --no-opt --cflags -Ic:/xpm/include'
 

bug#6690: 24.0.50; tmm-get-keymap is broken

[Stefan Monnier]

> emacs -Q
> In several of the `cond' branches the variable `str' is not defined, so
> it is nil from the `let' default initialization.
 
> Then this code raises an error "Wrong type argument: stringp, nil",
> since `str' is not a string:
 
> (unless (eq km 'ignore)
>           (let ((binding (where-is-internal km nil t)))
>             (when binding
>               (setq binding (key-description binding))
>               ;; Try to align the keybindings.
>               (let ((colwidth (min 30 (- (/ (window-width) 2) 10))))
 
> ;; STR is nil - so cannot use `string-width' or `concat'.
 
>                 (setq str
>                       (concat str
>                               (make-string (max 2 (- colwidth
>                                                      (string-width str)
>                                                      (string-width binding)))
>                                            ?\s)
>                               binding))))))
 
Thanks for the report.  Indeed, the code is brittle in this regard.
I could easily work around the problem by checking `str' before taking
its width (concat should work just fine with nil), but before I do that,
I'd like to make sure the result (which would basically treat a nil as
equivalent to "") is right, so could you give some example where the
problem gets triggered (in all "reasonable" cases I can think of, str
ends up being non-nil since otherwise it means the menu entry would
have a valid command and a valid shortcut, but no name).


        Stefan

bug#6690: 24.0.50; tmm-get-keymap is broken

[Lars Ingebrigtsen]

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

> I'd like to make sure the result (which would basically treat a nil as
> equivalent to "") is right, so could you give some example where the
> problem gets triggered (in all "reasonable" cases I can think of, str
> ends up being non-nil since otherwise it means the menu entry would
> have a valid command and a valid shortcut, but no name).

More information was requested three years ago, but no further progress
has been made.  Is this bug still present in Emacs 24.3?

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

bug#6690: 24.0.50; tmm-get-keymap is broken

[Drew Adams]

> More information was requested three years ago, but no further
> progress has been made.  Is this bug still present in
> Emacs 24.3?

More precisely, Stefan said:

> Indeed, the code is brittle in this regard.
> I could easily work around the problem by checking `str'
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> before taking its width (concat should work just fine with nil),
> but before I do that, I'd like to make sure the result (which
> would basically treat a nil as equivalent to "") is right...

I have no way to determine that more that he does (and did).

Sorry, it's been 3 years and I don't have anything more to offer
about this.  If you don't want to make the code less brittle and
fix the case where the value is nil, so be it.

Personally, I don't use tmm anyway.  It's possible that I
happened to notice this problem when looking at the code.
Or perhaps I ran into it while experimenting with the code.
I really don't know.