bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[Markus Triska]

When I do:

   $ emacs -Q -f view-mode

and then "C-x * Q", nothing happens. ("C-x * *" works as expected.)

In GNU Emacs 23.0.60.7 (i386-apple-darwin8.11.1, GTK+ Version 2.12.9)
 of 2008-08-30 on v254-034.vps.tuwien.ac.at
Windowing system distributor `The XFree86 Project, Inc', version 11.0.40400000
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: nil
  value of $XMODIFIERS: nil
  locale-coding-system: nil
  default-enable-multibyte-characters: t

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[martin rudalics]

 > When I do:
 >
 >    $ emacs -Q -f view-mode
 >
 > and then "C-x * Q", nothing happens. ("C-x * *" works as expected.)

Does it work if you replace

             (when (string-match "abcdefhijklnopqrstuwxyz"
                                 (char-to-string (car x)))
               (define-key map (char-to-string (- (car x) ?a -1)) (cdr x)))

by something like

             (when (string-match "[a-z]" (char-to-string (car x)))
               (define-key map (upcase (char-to-string (car x))) (cdr x)))

in the definition of `calc-dispatch-map'?

martin

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[martin rudalics]

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

 >  > When I do:
 >  >
 >  >    $ emacs -Q -f view-mode
 >  >
 >  > and then "C-x * Q", nothing happens. ("C-x * *" works as expected.)

Any objections against the attached patch?

martin

[-- Attachment #2: 835.diff --]
[-- Type: text/plain, Size: 1108 bytes --]

*** calc/calc.el.~1.115.~	2008-08-23 09:41:44.515625000 +0200
--- calc/calc.el	2008-09-01 19:43:05.515625000 +0200
***************
*** 1097,1105 ****
    (let ((map (make-keymap)))
      (mapc (lambda (x)
              (define-key map (char-to-string (car x)) (cdr x))
!             (when (string-match "abcdefhijklnopqrstuwxyz"
                                  (char-to-string (car x)))
!               (define-key map (char-to-string (- (car x) ?a -1)) (cdr x)))
              (define-key map (format "\e%c" (car x)) (cdr x)))
            '( ( ?a . calc-embedded-activate )
               ( ?b . calc-big-or-small )
--- 1097,1105 ----
    (let ((map (make-keymap)))
      (mapc (lambda (x)
              (define-key map (char-to-string (car x)) (cdr x))
!             (when (string-match "[abcdefhijklnopqrstuwxyz]"
                                  (char-to-string (car x)))
!               (define-key map (upcase (char-to-string (car x))) (cdr x)))
              (define-key map (format "\e%c" (car x)) (cdr x)))
            '( ( ?a . calc-embedded-activate )
               ( ?b . calc-big-or-small )

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[Chong Yidong]

martin rudalics <rudalics@gmx.at> wrote:

> Any objections against the attached patch?

*** calc/calc.el.~1.115.~       2008-08-23 09:41:44.515625000 +0200
--- calc/calc.el                2008-09-01 19:43:05.515625000 +0200
***************
*** 1097,1105 ****
    (let ((map (make-keymap)))
      (mapc (lambda (x)
              (define-key map (char-to-string (car x)) (cdr x))
!             (when (string-match "abcdefhijklnopqrstuwxyz"
                                  (char-to-string (car x)))
!               (define-key map (char-to-string (- (car x) ?a -1)) (cdr
x)))
              (define-key map (format "\e%c" (car x)) (cdr x)))
            '( ( ?a . calc-embedded-activate )
               ( ?b . calc-big-or-small )
--- 1097,1105 ----
    (let ((map (make-keymap)))
      (mapc (lambda (x)
              (define-key map (char-to-string (car x)) (cdr x))
!             (when (string-match "[abcdefhijklnopqrstuwxyz]"
                                  (char-to-string (car x)))
!               (define-key map (upcase (char-to-string (car x))) (cdr
x)))
              (define-key map (format "\e%c" (car x)) (cdr x)))
            '( ( ?a . calc-embedded-activate )
               ( ?b . calc-big-or-small )

Indeed, the argument to string-match in the original code looks like a
typo (fixed by the first line of the patch).

The second part of the patch is not clear to me.  What the original code
does is the following: for a character such as "n", it adds "C-n" to the
keymap as well (it's a hack that relies on the specific properties of
the ASCII table, and needs at least a comment).  That's not what the
patched code does: it adds "N" to the keymap.  Martin, what's the reason
for this change?

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[martin rudalics]

 > !               (define-key map (char-to-string (- (car x) ?a -1)) (cdr
 > x)))
[...]
 > !               (define-key map (upcase (char-to-string (car x))) (cdr
 > x)))

 > What the original code
 > does is the following: for a character such as "n", it adds "C-n" to the
 > keymap as well (it's a hack that relies on the specific properties of
 > the ASCII table, and needs at least a comment).  That's not what the
 > patched code does: it adds "N" to the keymap.  Martin, what's the reason
 > for this change?

That was my intention: AFAICT `view-mode' took `Q' (and, for example,
`E') away from calc and I simply tried to give it back to calc.  But
maybe this part of the code really intended to add a "C-" ...

martin

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[Chong Yidong]

martin rudalics <rudalics@gmx.at> writes:

>> ! (define-key map (char-to-string (- (car x) ?a -1)) (cdr x)))
> [...]
>> ! (define-key map (upcase (char-to-string (car x))) (cdr x)))
>
>> What the original code does is the following: for a character such as
>> "n", it adds "C-n" to the keymap as well (it's a hack that relies on
>> the specific properties of the ASCII table, and needs at least a
>> comment).  That's not what the patched code does: it adds "N" to the
>> keymap.  Martin, what's the reason for this change?
>
> That was my intention: AFAICT `view-mode' took `Q' (and, for example,
> `E') away from calc and I simply tried to give it back to calc.  But
> maybe this part of the code really intended to add a "C-" ...

Yeah, it's somewhat opaque code.  The way the ASCII table is set up is
that character 1 is the character ^A, character 2 is the character ^B,
and so forth, until character 65 is the unmodified character A.  Hence
(char-to-string (- ?n ?a -1)) gives 14 == ^N.  (Also, ASCII control
characters are case insensitive: there's no separate ^n character.)

I suppose this means we have to find a separate fix for the original
bug...

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[martin rudalics]

 > Yeah, it's somewhat opaque code.  The way the ASCII table is set up is
 > that character 1 is the character ^A, character 2 is the character ^B,
 > and so forth, until character 65 is the unmodified character A.  Hence
 > (char-to-string (- ?n ?a -1)) gives 14 == ^N.  (Also, ASCII control
 > characters are case insensitive: there's no separate ^n character.)

This makes sense iff the designer wanted to correct an accidentally held
down control key behavior (the code obviously never lived up to that due
to the regexp bug).

 > I suppose this means we have to find a separate fix for the original
 > bug...

... well, nothing hinders us assigning n, C-n and N to the same key.
But I'd like to hear Jay's comments first.

martin

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[Jay Belanger]

martin rudalics <rudalics@gmx.at> writes:
...
>> I suppose this means we have to find a separate fix for the original
>> bug...
>
> ... well, nothing hinders us assigning n, C-n and N to the same key.
> But I'd like to hear Jay's comments first.

The Calc manual mentions that all three act the same in
calc-dispatch-map, so they all should all be available.  That's just a
matter of adding one line to Martin's patch.
To make it less opaque, perhaps C-letter could be handled with something
like 
  (define-key map (vector (list 'control (car x))) (cdr x)))
(or something better) instead of
  (define-key map (char-to-string (- (car x) ?a -1)) (cdr x)))

Jay

bug#835: 23.0.60; Quick calc is quietly disabled in view-mode

[martin rudalics]

 > The Calc manual mentions that all three act the same in
 > calc-dispatch-map, so they all should all be available.  That's just a
 > matter of adding one line to Martin's patch.
 > To make it less opaque, perhaps C-letter could be handled with something
 > like
 >   (define-key map (vector (list 'control (car x))) (cdr x)))
 > (or something better) instead of
 >   (define-key map (char-to-string (- (car x) ?a -1)) (cdr x)))

I checked in a fix along these lines.  Please have a look.

martin