bug#1319: Change in where-is-internal causing describe-key problems

bug#1319: Change in where-is-internal causing describe-key problems

[Geoff Gole]

The behaviour of where-is-internal has changed in CVS emacs. Enter
view mode, M-: (where-is-internal 'self-insert-command nil nil nil) :

  22.2  => ([491520] [458752] [442368] [425984] [409600]...)
  CVS   => ([(128 . 4194303)])

This change breaks describe-function-1, used internally in
describe-key, causing a bound key to be printed as a non-ascii
character in some conditions. To reproduce the problem:

  emacs -Q
  C-h k k
  Switch to the *Help* buffer
  C-h k k

where-is-internal seems to be buggy, but perhaps describe-function-1
should also be doing more checking. I'm not sure if it should handle
input of the form ([(128 . 4194303)]) or not.

bug#1319: Change in where-is-internal causing describe-key problems

[martin rudalics]

 > but perhaps describe-function-1
 > should also be doing more checking.

It chokes at the very last conjunct of

	  (if (and (eq function 'self-insert-command)
		   (eq (key-binding "a") 'self-insert-command)
		   (eq (key-binding "b") 'self-insert-command)
		   (eq (key-binding "c") 'self-insert-command))

because in `help-mode' "c" is bound to `View-leave'.  IIUC, the heuristic
above should make "many" in the text

	      (princ "It is bound to many ordinary text characters.\n")

sound reasonable but apparently fails miserably when one of a, b, or c
is bound to something else.

martin

bug#1319: Change in where-is-internal causing describe-key problems

[martin rudalics]

 > I saw that, but I thought it was more likely to be a cheap early
 > out than a test that was ever meant to be bulletproof.

Maybe you were right in thinking so, I don't know.

 > This code later in the describe-function-1 should handle the case
 > where that test fails but there are many keys bound to
 > self-insert-command:
 >
 >   (when keys
 >     (princ (if remapped ", which is bound to " "It is bound to "))
 >     ;; If lots of ordinary text characters run this command,
 >     ;; don't mention them one by one.
 >     (if (< (length non-modified-keys) 10)
 >         (princ (mapconcat 'key-description keys ", "))
 >       (dolist (key non-modified-keys)
 >         (setq keys (delq key keys)))
 >       (if keys
 >           (progn
 >             (princ (mapconcat 'key-description keys ", "))
 >             (princ ", and many ordinary text characters"))
 >         (princ "many ordinary text characters"))))
 >
 > And indeed in 22.2, it does. But the aforementioned change in
 > where-is-internal breaks it.

What precisely was the change in `where-is-internal' that broke it?

martin

bug#1319: Change in where-is-internal causing describe-key problems

[Geoff Gole]

The change in return value described in the original report:

> The behaviour of where-is-internal has changed in CVS emacs. Enter
> view mode, M-: (where-is-internal 'self-insert-command nil nil nil) :
>
> 22.2  => ([491520] [458752] [442368] [425984] [409600]...)
> CVS   => ([(128 . 4194303)])

Unfortunately I don't have the 22.2 source at the moment to
compare what differences in the code might cause that. If it
would be helpful, I'll try to figure out how to pull them from
cvs.

bug#1319: Change in where-is-internal causing describe-key problems

[martin rudalics]

 >> The behaviour of where-is-internal has changed in CVS emacs. Enter
 >> view mode, M-: (where-is-internal 'self-insert-command nil nil nil) :
 >>
 >> 22.2  => ([491520] [458752] [442368] [425984] [409600]...)
 >> CVS   => ([(128 . 4194303)])
 >
 > Unfortunately I don't have the 22.2 source at the moment to
 > compare what differences in the code might cause that. If it
 > would be helpful, I'll try to figure out how to pull them from
 > cvs.

 From your previous mail I concluded that you knew the change that caused
this.  I have no idea what the cons above stands for (some optimization,
I presume) so I'm totally lost here.

martin

bug#1319: Change in where-is-internal causing describe-key problems

[Stefan Monnier]

>> 22.2  => ([491520] [458752] [442368] [425984] [409600]...)
>> CVS   => ([(128 . 4194303)])

The 22.2 answer lists generic chars (these are chars that stand for
a whole charset).  In Emacs-23, generic chars have been dropped in favor
of using char-ranges, represented a a cons cell (START . END).


        Stefan

bug#1319: Change in where-is-internal causing describe-key problems

[martin rudalics]

>>> 22.2  => ([491520] [458752] [442368] [425984] [409600]...)
>>> CVS   => ([(128 . 4194303)])
>
> The 22.2 answer lists generic chars (these are chars that stand for
> a whole charset).  In Emacs-23, generic chars have been dropped in favor
> of using char-ranges, represented a a cons cell (START . END).

This would deserve a NEWS entry.

Anyway, we can either replace

	  (if (and (eq function 'self-insert-command)
		   (eq (key-binding "a") 'self-insert-command)
		   (eq (key-binding "b") 'self-insert-command)
		   (eq (key-binding "c") 'self-insert-command))
	      (princ "It is bound to many ordinary text characters.\n")
	    ...

by something like

	  (if (and (eq function 'self-insert-command)
		   (catch 'plenty
		     (let ((count 0))
		       (dolist (item (where-is-internal 'self-insert-command))
			 (when (vectorp item)
			   (let ((range (elt item 0)))
			     (when (consp range)
			       (setq count (+ count (- (cdr range) (car range))))
			       (when (> count 100)
				 (throw 'plenty count)))))))))
	      (princ "It is bound to many ordinary text characters.\n")
	    ...
	
or drop that "It is bound to many ordinary text characters" stuff.

martin, who'd drop it unless someone finds a better solution.

bug#1319: marked as done (Change in where-is-internal causing describe-key problems)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]


Your message dated Sat, 15 Nov 2008 01:22:27 -0500
with message-id <87y6zljyos.fsf@cyd.mit.edu>
and subject line Re: bug#1319: Change in where-is-internal causing describe-key problems
has caused the Emacs bug report #1319,
regarding Change in where-is-internal causing describe-key problems
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
1319: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1319
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 3575 bytes --]

From: "Geoff Gole" <geoffgole@gmail.com>
To: emacs-pretest-bug@gnu.org
Subject: Change in where-is-internal causing describe-key problems
Date: Sat, 8 Nov 2008 19:19:57 -0800
Message-ID: <f5bc73230811081919k1bed5c01q1ff37c159a7d1a69@mail.gmail.com>

The behaviour of where-is-internal has changed in CVS emacs. Enter
view mode, M-: (where-is-internal 'self-insert-command nil nil nil) :

  22.2  => ([491520] [458752] [442368] [425984] [409600]...)
  CVS   => ([(128 . 4194303)])

This change breaks describe-function-1, used internally in
describe-key, causing a bound key to be printed as a non-ascii
character in some conditions. To reproduce the problem:

  emacs -Q
  C-h k k
  Switch to the *Help* buffer
  C-h k k

where-is-internal seems to be buggy, but perhaps describe-function-1
should also be doing more checking. I'm not sure if it should handle
input of the form ([(128 . 4194303)]) or not.



[-- Attachment #3: Type: message/rfc822, Size: 1128 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: martin rudalics <rudalics@gmx.at>
Cc: 1319-done@emacsbugs.donarmstrong.com
Subject: Re: bug#1319: Change in where-is-internal causing describe-key problems
Date: Sat, 15 Nov 2008 01:22:27 -0500
Message-ID: <87y6zljyos.fsf@cyd.mit.edu>

I checked in a slightly simpler fix to describe-function-1.  It's
probably not necessary to lose too much hair over this part of the code.