Re: M-g in dired

Re: M-g in dired

[Richard Stallman]

    In most contexts M-g is a prefix key, but in dired it is bound to
    dired-goto-file.  I often run the command compile while visiting a
    dired buffer, but cannot jump to errors with M-g M-n as usual.

This binding is a leftover from a time when M-g had a different global
meaning.  We should get rid of this binding, and move the command to
another key.

What would be a good binding for it?
M-g f and M-g M-f, perhaps?

Re: M-g in dired

[Juri Linkov]

>     In most contexts M-g is a prefix key, but in dired it is bound to
>     dired-goto-file.  I often run the command compile while visiting a
>     dired buffer, but cannot jump to errors with M-g M-n as usual.
>
> This binding is a leftover from a time when M-g had a different global
> meaning.  We should get rid of this binding, and move the command to
> another key.
>
> What would be a good binding for it?
> M-g f and M-g M-f, perhaps?

Currently dired.el defines:

  (define-key dired-mode-map "\M-g" 'dired-goto-file)

and dired-x.el defines:

  (define-key dired-mode-map "\M-g" 'dired-goto-file)
  (define-key dired-mode-map "\M-G" 'dired-goto-subdir)

I recall I proposed two variants for dired-goto-file:
`M-g f' (and `M-g M-f') and `M-g g' (and `M-g M-g').
And you agreed to `M-g f' (and `M-g M-f').  But it's
not clear what to do with \M-G bound to dired-goto-subdir?

I also proposed to make the key binding `M-g f' global,
but this could be implemented ATR (after the release).

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Kim F. Storm]

Juri Linkov <juri@jurta.org> writes:

>>     In most contexts M-g is a prefix key, but in dired it is bound to
>>     dired-goto-file.  I often run the command compile while visiting a
>>     dired buffer, but cannot jump to errors with M-g M-n as usual.
>>
>> What would be a good binding for it?

>   (define-key dired-mode-map "\M-g" 'dired-goto-file)
>   (define-key dired-mode-map "\M-G" 'dired-goto-subdir)

What about M-t (and M-T)

.. transpose-words is useless in dired buffers.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: M-g in dired

[Miles Bader]

storm@cua.dk (Kim F. Storm) writes:
>>> What would be a good binding for it?
>
>>   (define-key dired-mode-map "\M-g" 'dired-goto-file)
>>   (define-key dired-mode-map "\M-G" 'dired-goto-subdir)
>
> What about M-t (and M-T)
>
> .. transpose-words is useless in dired buffers.

I like M-j (and M-J):  "J" is for "jump", and mirrors similar bindings
used in gnus.

[The global binding for M-j is similarly useless in dired.]

-Miles

-- 
I'm beginning to think that life is just one long Yoko Ono album; no rhyme
or reason, just a lot of incoherent shrieks and then it's over.  --Ian Wolff

Re: M-g in dired

[Kim F. Storm]

Miles Bader <miles.bader@necel.com> writes:

> storm@cua.dk (Kim F. Storm) writes:
>>>> What would be a good binding for it?
>>
>>>   (define-key dired-mode-map "\M-g" 'dired-goto-file)
>>>   (define-key dired-mode-map "\M-G" 'dired-goto-subdir)
>>
>> What about M-t (and M-T)
>>
>> .. transpose-words is useless in dired buffers.
>
> I like M-j (and M-J):  "J" is for "jump", and mirrors similar bindings
> used in gnus.

Excellent idea.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: M-g in dired

[Juri Linkov]

> I like M-j (and M-J):  "J" is for "jump", and mirrors similar bindings
> used in gnus.
>
> [The global binding for M-j is similarly useless in dired.]

If people find the global binding `M-g f' useless, then I suggest
using `j' in dired, because `j' is what is defined in Gnus, and
in dired most key bindings are just one key without `M-'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard Stallman]

    What about M-t (and M-T)

    .. transpose-words is useless in dired buffers.

I would not object to using M-t for this.

Miles wrote:

    I like M-j (and M-J):  "J" is for "jump", and mirrors similar bindings
    used in gnus.

    [The global binding for M-j is similarly useless in dired.]

I think that is more logical than M-t.  And more convenient than
a two-character key.

So let's switch to M-j in Dired.

Thanks.

Re: M-g in dired

[Juri Linkov]

>     I recall I proposed two variants for dired-goto-file:
>     `M-g f' (and `M-g M-f') and `M-g g' (and `M-g M-g').
>     And you agreed to `M-g f' (and `M-g M-f').
>
>     I also proposed to make the key binding `M-g f' global,
>     but this could be implemented ATR (after the release).
>
> That command makes no sense except in a Dired buffer,
> so I don't understand that proposal.

Let's say you are editing a file in the Emacs buffer.  When you need
to perform some operation on this file in Dired, how do you do this?

The quickest way to do this I know is to type C-x d RET, and to find
the edited file in the (sometimes very long) list of all files
in the same directory.  Then to type `n n ...' several times to put
point on the file, or to type `C-s filename' to arrive at it with the help
of isearch, or to type `M-g filename RET' (when M-g was not a prefix key).

With the global key binding `M-g f' all this is achieved by `M-g f M-n RET',
i.e. `M-n' puts the edited file name as default in the prompt, and after RET
it puts point just on the file in the dired buffer, and you can immediately
start performing necessary operations on this file.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Johan Bockgård]

Juri Linkov <juri@jurta.org> writes:

> Let's say you are editing a file in the Emacs buffer. When you need
> to perform some operation on this file in Dired, how do you do this?

,----
| C-x C-j runs the command dired-jump
|    which is an interactive autoloaded Lisp function in `dired-x'.
| It is bound to C-x C-j.
| (dired-jump &optional OTHER-WINDOW)
| 
| Jump to dired buffer corresponding to current buffer.
| If in a file, dired the current directory and move to file's line.
| If in Dired already, pop up a level and goto old directory's line.
| In case the proper dired file line cannot be found, refresh the dired
| buffer and try again.
`----

-- 
Johan Bockgård

Re: M-g in dired

[Juri Linkov]

> ,----
> | C-x C-j runs the command dired-jump
> |    which is an interactive autoloaded Lisp function in `dired-x'.
> | It is bound to C-x C-j.
> | (dired-jump &optional OTHER-WINDOW)
> | 
> | Jump to dired buffer corresponding to current buffer.
> | If in a file, dired the current directory and move to file's line.
> | If in Dired already, pop up a level and goto old directory's line.
> | In case the proper dired file line cannot be found, refresh the dired
> | buffer and try again.
> `----

Let me remind that this was discussed in the thread:
http://lists.gnu.org/archive/html/emacs-devel/2005-03/msg00441.html

The semi-conclusion was: `dired-jump' should be moved to dired.el,
but needs to ask for a file name, either when called with a prefix key
like M-0, or assigned to a separate key like M-g j.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Romain Francoise]

Juri Linkov <juri@jurta.org> writes:

> Let me remind that this was discussed in the thread:
> http://lists.gnu.org/archive/html/emacs-devel/2005-03/msg00441.html

> The semi-conclusion was: `dired-jump' should be moved to dired.el,
> but needs to ask for a file name, either when called with a prefix key
> like M-0, or assigned to a separate key like M-g j.

We can do that after the release.

-- 
Romain Francoise <romain@orebokech.com> | The sea! the sea! the open
it's a miracle -- http://orebokech.com/ | sea! The blue, the fresh, the
                                        | ever free! --Bryan W. Procter

Re: M-g in dired

[Richard Stallman]

    The quickest way to do this I know is to type C-x d RET, and to find
    the edited file in the (sometimes very long) list of all files
    in the same directory.

I think we want to make it easier to get the full visited file name
into the minibuffer after C-x d.  For instance, putting it into the
history after the default file name, so that you can get it by typing
M-n M-n.

    With the global key binding `M-g f' all this is achieved by `M-g f M-n RET'

Now I understand what you want, but it is not the same as the existing
command dired-goto-file.  That only moves point within a dired buffer.

What you want is not absurd as a command, but I don't think it belongs
in M-g.  Anyway, I think that the method using the history would be
better.

Re: M-g in dired

[Juri Linkov]

>     The quickest way to do this I know is to type C-x d RET, and to find
>     the edited file in the (sometimes very long) list of all files
>     in the same directory.
>
> I think we want to make it easier to get the full visited file name
> into the minibuffer after C-x d.  For instance, putting it into the
> history after the default file name, so that you can get it by typing
> M-n M-n.

C-x d already puts the file name of the current buffer into the default
value of the minibuffer accessible by the single M-n.  But when C-x d
accepts a file name, it has the different effect: it produces the dired
buffer with only one file whose name was given for C-x d (all other files
are filtered out by a wildcard).

For me this behavior is useless.  I'd like to display the dired buffer
with all files and to move point to the file with the name specified
for C-x d.

>     With the global key binding `M-g f' all this is achieved by `M-g f M-n RET'
>
> Now I understand what you want, but it is not the same as the existing
> command dired-goto-file.  That only moves point within a dired buffer.

The only difference from the existing dired-goto-file is that when
used as a global key it would create the dired buffer before moving
point to the given file name.

> What you want is not absurd as a command, but I don't think it belongs
> in M-g.

One reason to put it on M-g prefix was mnemonics "Go to File" for `M-g f'.
But perhaps reusing `C-x C-j' from dired-x is a better key.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard Stallman]

    C-x d already puts the file name of the current buffer into the default
    value of the minibuffer accessible by the single M-n.

Strange, it didn't work that way when I tried it.  It doesn't work for
me now.  Looking at the code, I see that dired-read-dir-and-switches,
when it calls read-file-name, passes default-directory as the default
file name.

Can you investigate how does this work when you try it?

      But when C-x d
    accepts a file name, it has the different effect: it produces the dired
    buffer with only one file whose name was given for C-x d (all other files
    are filtered out by a wildcard).

Yes, that's what it is supposed to do.

    For me this behavior is useless.  I'd like to display the dired buffer
    with all files and to move point to the file with the name specified
    for C-x d.

I see your point.  Hmm.

    The only difference from the existing dired-goto-file is that when
    used as a global key it would create the dired buffer before moving
    point to the given file name.

Maybe this does belong in M-g.  If so, it should be M-g d,
go to file in Dired.

Re: M-g in dired

[Juri Linkov]

>     C-x d already puts the file name of the current buffer into the default
>     value of the minibuffer accessible by the single M-n.
>
> Strange, it didn't work that way when I tried it.  It doesn't work for
> me now.  Looking at the code, I see that dired-read-dir-and-switches,
> when it calls read-file-name, passes default-directory as the default
> file name.
>
> Can you investigate how does this work when you try it?

Oops, this default was inserted by ffap.  When ffap-read-file-or-url
reads a directory/file name it uses the current buffer's file name as
default.  dired could do the same.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard Stallman]

    Oops, this default was inserted by ffap.  When ffap-read-file-or-url
    reads a directory/file name it uses the current buffer's file name as
    default.  dired could do the same.

The visited file name should not be the default for C-x d.  But it
would be nice if the visited file name were easily accessible
when you want it.  That's why I suggest putting it in the "future
history" list.

Re: M-g in dired

[Juri Linkov]

>     Oops, this default was inserted by ffap.  When ffap-read-file-or-url
>     reads a directory/file name it uses the current buffer's file name as
>     default.  dired could do the same.
>
> The visited file name should not be the default for C-x d.  But it
> would be nice if the visited file name were easily accessible
> when you want it.  That's why I suggest putting it in the "future
> history" list.

For reading a directory/file name, ffap uses the function
`completing-read' with the value of the `initial-input' arg set
to the directory name, and the `default' arg set to the current buffer's
file name.  This has the effect that typing RET returns the directory name,
not the file name from the default value.  And only after typing `M-n RET'
it returns the default file name.

Is it possible to achieve the same effect by `read-file-name'
(which is used by dired)?

Essentially, what is used by ffap is:

    (completing-read "Dired file: "
     nil "~/" nil '("~/" . 2) 'file-name-history "~/.emacs")

    Typing `C-x C-e RET' returns "~/"
    Typing `C-x C-e M-n RET' returns "~/.emacs"

But trying to do the same for `read-file-name':

    (read-file-name "Dired file: " "~/" "~/.emacs" nil nil nil)

    Typing `C-x C-e RET' returns "~/.emacs", not "~/"
    Typing `C-x C-e M-n RET' returns "~/.emacs"

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[David Hansen]

On Thu, 18 May 2006 22:04:57 -0400 Richard Stallman wrote:

>     In most contexts M-g is a prefix key, but in dired it is bound to
>     dired-goto-file.  I often run the command compile while visiting a
>     dired buffer, but cannot jump to errors with M-g M-n as usual.
>
> This binding is a leftover from a time when M-g had a different global
> meaning.  We should get rid of this binding, and move the command to
> another key.

BTW gnus uses M-g too in summary and group buffers.

David

Re: M-g in dired

[Reiner Steib]

On Fri, May 19 2006, David Hansen wrote:

> On Thu, 18 May 2006 22:04:57 -0400 Richard Stallman wrote:
>
>>     In most contexts M-g is a prefix key, but in dired it is bound to
>>     dired-goto-file.  I often run the command compile while visiting a
>>     dired buffer, but cannot jump to errors with M-g M-n as usual.
>>
>> This binding is a leftover from a time when M-g had a different global
>> meaning.  We should get rid of this binding, and move the command to
>> another key.
>
> BTW gnus uses M-g too in summary and group buffers.

I don't see any need to have the bindings for goto-line, next-error
and previous-error available in Gnus Group buffers.

(I think we already discussed this when M-g was changes in Emacs.)

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: M-g in dired

[Miles Bader]

David Hansen <david.hansen@gmx.net> writes:
>> This binding is a leftover from a time when M-g had a different global
>> meaning.  We should get rid of this binding, and move the command to
>> another key.
>
> BTW gnus uses M-g too in summary and group buffers.

This was discussed at the time Emacs adopted the M-g prefix, and I think
the conclusion was that it's not a big deal -- though it's too ingrained /
important a binding to change in Gnus, people are not that likely to want
to use the M-g prefix while reading news/mail.

-Miles
-- 
.Numeric stability is probably not all that important when you're guessing.

M-g in dired

[Juri Linkov]

`M-g M-n' is a nice key binding for `next-error' (easy to type and
mnemonic), but has one problem.  Just imagine a quite likely scenario:
the user switches to the dired buffer, invokes `compile' or `grep', and
types `M-g M-n' to go to the first error displayed in the output buffer
in another window.  Now instead of going to the first error, the message
"End of history; no default available" is displayed in the echo area.
That's because `M-g' is bound to `dired-goto-file' in the dired buffer,
which activates the minibuffer, and the second key `M-n' typed by the
user is bound in the minibuffer to `next-history-element' (which signals
an error for no default value).

It seems `M-g' in dired buffers should be rebound for `dired-goto-file'
to a key sequence with the `M-g' prefix.  I see at least two variants:

1. Rebind `dired-goto-file' to `M-g g' (and `M-g M-g').  The global
   binding for `M-g g' is `goto-line' which doesn't seem to be useful
   in dired buffers.  So dired buffers could have a local binding
   `M-g g' for `dired-goto-file'.

2. Rebind `dired-goto-file' to `M-g f' (and `M-g M-f').  This key
   sequence could serve as a global key binding.  Typing it in any
   buffer could create a dired buffer and move point to the file
   specified in the minibuffer (with the default value being the
   current buffer's file name).

It is not unreasonable to have both bindings.

BTW, there is a bug related to key suggestions: `M-x next-error RET'
typed in a dired buffer suggests the key binding `M-g n' even though
`M-g' is not a prefix key in dired buffers.  Sometimes it may suggest
`C-x `' when `where-is-internal' returns the first keybindings.  Since
it's not possible to control which keybinding `where-is-internal'
returns, to reproduce this bug `C-x `' should be unbound globally,
e.g. by (define-key ctl-x-map "`" nil).  After that

(key-description (where-is-internal 'next-error overriding-local-map t))

evaluates in a dired buffer to "M-g n", but currently it is not
possible to use this key in dired buffers.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Kim F. Storm]

Juri Linkov <juri@jurta.org> writes:

There is a similar problem in GNUS.

I get hit by doing M-g in an article buffer every now and then...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: M-g in dired

[Richard M. Stallman]

    2. Rebind `dired-goto-file' to `M-g f' (and `M-g M-f').

That seems reasonable--in Dired mode alone.

							     This key
       sequence could serve as a global key binding.  Typing it in any
       buffer could create a dired buffer and move point to the file
       specified in the minibuffer (with the default value being the
       current buffer's file name).

That seems artificial and not particularly useful.


So, would you like to do the first part?

Re: M-g in dired

[Richard M. Stallman]

    BTW, there is a bug related to key suggestions: `M-x next-error RET'
    typed in a dired buffer suggests the key binding `M-g n' even though
    `M-g' is not a prefix key in dired buffers.

That seems like a bug.  where-is-internal should never suggest a key
which isn't really bound to the desired command.

ISTR there is code in where-is-internal that tries to verify this.
If so, that code must now be broken.  I am overwhelmed now; could
someone please try debugging this?

Re: M-g in dired

[Richard Stallman]

[I sent this message two weeks ago but did not get a response.
Could we get the discussion moving again?]

    BTW, there is a bug related to key suggestions: `M-x next-error RET'
    typed in a dired buffer suggests the key binding `M-g n' even though
    `M-g' is not a prefix key in dired buffers.

That seems like a bug.  where-is-internal should never suggest a key
which isn't really bound to the desired command.

ISTR there is code in where-is-internal that tries to verify this.
If so, that code must now be broken.  I am overwhelmed now; could
someone please try debugging this?

Re: M-g in dired

[Juri Linkov]

>     BTW, there is a bug related to key suggestions: `M-x next-error RET'
>     typed in a dired buffer suggests the key binding `M-g n' even though
>     `M-g' is not a prefix key in dired buffers.
>
> That seems like a bug.  where-is-internal should never suggest a key
> which isn't really bound to the desired command.
>
> ISTR there is code in where-is-internal that tries to verify this.
> If so, that code must now be broken.  I am overwhelmed now; could
> someone please try debugging this?

where-is-internal verifies only the whole key sequence, but it seems
that verifying if a shorter key can shadow a key sequence is not
implemented.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard M. Stallman]

    where-is-internal verifies only the whole key sequence, but it seems
    that verifying if a shorter key can shadow a key sequence is not
    implemented.

Thanks for figuring out what's wrong.

Would you like to try modifying that code so it detects the other case?

Re: M-g in dired

[Juri Linkov]

> Would you like to try modifying that code so it detects the other case?

I think the most appropriate place to modify is `shadow_lookup'.
The second part of the patch below changes it to return nil
when one of prefix keys is found in the keymap with higher priority.
This allows to find a shorter key in the local keymap rather than in
the global keymap.  For example, looking for "M-g n" in two keymaps

    (lookup-key dired-mode-map [134217831 110]) => 1
    (lookup-key global-map [134217831 110]) => next-error

will return nil, because lookup-key in dired-mode-map returns a number,
since "M-g" in dired-mode-map is bound to `dired-goto-file'.

However, this change also requires one change in the value returned by
`lookup-key'.  Currently, it returns a number even when it can't find
a key definition and the tested key sequence is two keys or longer.
So, for example, in dired-mode-map where "C-x `" is undefined,
`lookup-key' returns 1:

    (lookup-key dired-mode-map [24 96]) => 1

It seems this is not correct.  And the documentation says nothing about
such case.  The first part of the patch changes `lookup-key' to return
nil, e.g.:

    (lookup-key dired-mode-map [24 96]) => nil

I can't estimate if there is a code that relies on the current
return value of `lookup-key'.  If so, then perhaps a new special
argument could be added to `lookup-key' to change the semantic of
the return value?

Index: src/keymap.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keymap.c,v
retrieving revision 1.307
diff -c -r1.307 keymap.c
*** src/keymap.c	12 Sep 2005 10:26:35 -0000	1.307
--- src/keymap.c	1 Nov 2005 09:16:18 -0000
***************
*** 1262,1269 ****
  
        keymap = get_keymap (cmd, 0, 1);
        if (!CONSP (keymap))
! 	RETURN_UNGCPRO (make_number (idx));
! 
        QUIT;
      }
  }
--- 1262,1273 ----
  
        keymap = get_keymap (cmd, 0, 1);
        if (!CONSP (keymap))
! 	{
! 	  if (!NILP (cmd))
! 	    RETURN_UNGCPRO (make_number (idx));
! 	  else
! 	    return Qnil;
! 	}
        QUIT;
      }
  }
***************
*** 2377,2383 ****
    for (tail = shadow; CONSP (tail); tail = XCDR (tail))
      {
        value = Flookup_key (XCAR (tail), key, flag);
!       if (!NILP (value) && !NATNUMP (value))
  	return value;
      }
    return Qnil;
--- 2381,2389 ----
    for (tail = shadow; CONSP (tail); tail = XCDR (tail))
      {
        value = Flookup_key (XCAR (tail), key, flag);
!       if (NATNUMP (value))
! 	return Qnil;
!       if (!NILP (value))
  	return value;
      }
    return Qnil;

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Andreas Schwab]

Juri Linkov <juri@jurta.org> writes:

> --- 1262,1273 ----
>   
>         keymap = get_keymap (cmd, 0, 1);
>         if (!CONSP (keymap))
> ! 	{
> ! 	  if (!NILP (cmd))
> ! 	    RETURN_UNGCPRO (make_number (idx));
> ! 	  else
> ! 	    return Qnil;

You still have to use RETURN_UNGCPRO here to undo the GCPRO.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: M-g in dired

[Juri Linkov]

> You still have to use RETURN_UNGCPRO here to undo the GCPRO.

Thanks.  Here is the correct patch:

Index: src/keymap.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keymap.c,v
retrieving revision 1.307
diff -c -r1.307 keymap.c
*** src/keymap.c	12 Sep 2005 10:26:35 -0000	1.307
--- src/keymap.c	1 Nov 2005 19:55:45 -0000
***************
*** 1262,1269 ****
  
        keymap = get_keymap (cmd, 0, 1);
        if (!CONSP (keymap))
! 	RETURN_UNGCPRO (make_number (idx));
! 
        QUIT;
      }
  }
--- 1262,1273 ----
  
        keymap = get_keymap (cmd, 0, 1);
        if (!CONSP (keymap))
! 	{
! 	  if (!NILP (cmd))
! 	    RETURN_UNGCPRO (make_number (idx));
! 	  else
! 	    RETURN_UNGCPRO (Qnil);
! 	}
        QUIT;
      }
  }
***************
*** 2377,2383 ****
    for (tail = shadow; CONSP (tail); tail = XCDR (tail))
      {
        value = Flookup_key (XCAR (tail), key, flag);
!       if (!NILP (value) && !NATNUMP (value))
  	return value;
      }
    return Qnil;
--- 2381,2389 ----
    for (tail = shadow; CONSP (tail); tail = XCDR (tail))
      {
        value = Flookup_key (XCAR (tail), key, flag);
!       if (NATNUMP (value))
! 	return Qnil;
!       if (!NILP (value))
  	return value;
      }
    return Qnil;

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard M. Stallman]

Please install it in a few days if no further problems are found.
And thanks.

Re: M-g in dired

[Richard M. Stallman]

    > Would you like to try modifying that code so it detects the other case?

    I think the most appropriate place to modify is `shadow_lookup'.

That sounds right to me.

    So, for example, in dired-mode-map where "C-x `" is undefined,
    `lookup-key' returns 1:

	(lookup-key dired-mode-map [24 96]) => 1

    It seems this is not correct.

I agree, that is a bug.  Since C-x is a valid prefix key,
the value should not be 1.  It should be whatever
the C-x subkeymap gives for `.

Could you fix that?

    I can't estimate if there is a code that relies on the current
    return value of `lookup-key'.  If so, then perhaps a new special
    argument could be added to `lookup-key' to change the semantic of
    the return value?

Could you study the code some more to try to determine this?

Re: M-g in dired

[Juri Linkov]

> Since C-x is a valid prefix key, the value should not be 1.
> It should be whatever the C-x subkeymap gives for `.

This is true only for the global map, where "C-x `" is defined.
So for the global map all is correct already:

  (lookup-key global-map "\C-x") => Control-X-prefix
  (lookup-key global-map "\C-x`") => next-error
  (lookup-key global-map "\C-x`abcdef") => 2

But in dired-mode-map, where "C-x `" is undefined, lookup-key
currently returns:

  (lookup-key dired-mode-map "\C-x") => nil
  (lookup-key dired-mode-map "\C-x`") => 1

The return value `1' of the last expression seems wrong.

The docstring of `lookup-key' says:

  A number as value means key is "too long"; that is, characters or
  symbols in it except for the last one fail to be a valid sequence of
  prefix characters in keymap.  The number is how many characters at
  the front of key it takes to reach a non-prefix command.

This docstring is not very clear.  The documentation in the Emacs Lisp
is better:

  If the string or vector KEY is not a valid key sequence according to
  the prefix keys specified in KEYMAP, it must be "too long" and have
  extra events at the end that do not fit into a single key sequence.
  Then the value is a number, the number of events at the front of KEY
  that compose a complete key.

For the case of dired-mode-map where lookup-key returns 1 for "\C-x`",
1 is not a number of events at the front of "\C-x`" that compose
a complete key, because "C-x" alone is not a valid key in dired-mode-map.

My patch changes the return value from 1 to nil:

  (lookup-key dired-mode-map "\C-x`") => nil

> Could you study the code some more to try to determine this?

I have grepped the Emacs source tree, and almost everywhere the return
value of lookup-key is checked for both nil and an integer in the same
condition.

There are only two functions where its return value is compared
separately for an integer value: `emerge-force-define-key' and
`emerge-define-key-if-possible' in lisp/emerge.el.  They undefine
the prefix key before redefining it with a longer key sequence.
With my patch these functions will work right too.  Currently they try
to undefine key sequences that are not defined, but with my patch, they
will not perform this useless operation.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Stefan Monnier]

> But in dired-mode-map, where "C-x `" is undefined, lookup-key
> currently returns:

>   (lookup-key dired-mode-map "\C-x") => nil
>   (lookup-key dired-mode-map "\C-x`") => 1

> The return value `1' of the last expression seems wrong.

No, it is correct.  `lookup-key' only looks at the keymap passed as argument
and doesn't (and shouldn't) try and guess which other keymaps might/will be
active together with this keymap.

> This docstring is not very clear.  The documentation in the Emacs Lisp
> is better:

>   If the string or vector KEY is not a valid key sequence according to
>   the prefix keys specified in KEYMAP, it must be "too long" and have
>   extra events at the end that do not fit into a single key sequence.
>   Then the value is a number, the number of events at the front of KEY
>   that compose a complete key.

> For the case of dired-mode-map where lookup-key returns 1 for "\C-x`",
> 1 is not a number of events at the front of "\C-x`" that compose
> a complete key, because "C-x" alone is not a valid key in dired-mode-map.

It doesn't say "valid key" it says "valid key sequence".  "C-x" is a valid
key sequence in this context.  It is not bound to anything, but it is
a valid sequence.  C-x ` is not a valid sequence on the other hand, because
read-key-sequence would have immediately stopped after C-x.


        Stefan

Re: M-g in dired

[Juri Linkov]

>> This docstring is not very clear.  The documentation in the Emacs Lisp
>> is better:
>
>>   If the string or vector KEY is not a valid key sequence according to
>>   the prefix keys specified in KEYMAP, it must be "too long" and have
>>   extra events at the end that do not fit into a single key sequence.
>>   Then the value is a number, the number of events at the front of KEY
>>   that compose a complete key.
>
>> For the case of dired-mode-map where lookup-key returns 1 for "\C-x`",
>> 1 is not a number of events at the front of "\C-x`" that compose
>> a complete key, because "C-x" alone is not a valid key in dired-mode-map.
>
> It doesn't say "valid key" it says "valid key sequence".  "C-x" is a valid
> key sequence in this context.  It is not bound to anything, but it is
> a valid sequence.  C-x ` is not a valid sequence on the other hand, because
> read-key-sequence would have immediately stopped after C-x.

>From the docstring I understood that "valid key sequence" means
a valid and bound key sequence, due to the sentence:

  The number is how many characters at the front of key it takes
  to reach a non-prefix command.

To me "a non-prefix command" implies a bound command key sequence.
That's why I changed the code to return the integer value only
on the condition `if (!NILP (cmd))'.

However, if this is not what the docstring meant, and leaving the logic
of the current return value is desirable, then maybe `lookup-key' needs
an additional argument defining another logic of its return value.

Otherwise, almost all code in `lookup-key' should be duplicated in
`shadow_lookup'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard M. Stallman]

    >From the docstring I understood that "valid key sequence" means
    a valid and bound key sequence, due to the sentence:

      The number is how many characters at the front of key it takes
      to reach a non-prefix command.

I will change that to say "a non-prefix key".
But you know you really should look at the manual
in cases like this.

    However, if this is not what the docstring meant, and leaving the logic
    of the current return value is desirable, then maybe `lookup-key' needs
    an additional argument defining another logic of its return value.

Why?

    Otherwise, almost all code in `lookup-key' should be duplicated in
    `shadow_lookup'.

Why?

Re: M-g in dired

[Juri Linkov]

>     However, if this is not what the docstring meant, and leaving the logic
>     of the current return value is desirable, then maybe `lookup-key' needs
>     an additional argument defining another logic of its return value.
>
> Why?
>
>     Otherwise, almost all code in `lookup-key' should be duplicated in
>     `shadow_lookup'.
>
> Why?

To fix the bug, I think changes should be made in `shadow_lookup'.
Let's see how it currently works for the case when dired-mode-map
defines the "M-g" key, and the global-map defines "M-gn":

`shadow_lookup' iterates over two maps (dired and global) and calls
`Flookup_key' on them.  When `Flookup_key' returns 1 for the key
"M-gn" on the dired map, the loop in `shadow_lookup' skips it
and advances to the next iteration on the global map where it finds
the command `next-error' bound to "M-gn", and returns it.

This causes the current code to assume that "M-gn" in the global map
is not shadowed by "M-g" in the dired map.  It fails due to the value 1
returned by `Flookup_key', which is unsuitable to decide whether "M-g"
is bound in the dired map or not, and thus whether it shadows "M-gn"
in the global map or not.

To fix this, I intended to use the same code as `Flookup_key' uses
to process key sequences, but additionally to check whether a shorter
prefix key is bound or not.  This can be achieved either by adding
a new argument to `Flookup_key' to check if a key is bound and to
return the corresponding return value (and not to change its current
default semantics of the return value), or to duplicate most code of
`Flookup_key' in `shadow_lookup' to do this.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard M. Stallman]

    `shadow_lookup' iterates over two maps (dired and global) and calls
    `Flookup_key' on them.  When `Flookup_key' returns 1 for the key
    "M-gn" on the dired map, the loop in `shadow_lookup' skips it

That's the place that has to be changed.

When this call to `Flookup_key' returns a number N, `shadow_lookup'
should look up a sub-key-sequence of the first N events.  If that
returns non-nil, the longer key is shadowed, so don't mention it at all.
If that returns non-nil, the longer key is not shadowed.

That avoids duplicating lots of code and avoids changing the calling
convention of Flookup_key.

Re: M-g in dired

[Juri Linkov]

>     `shadow_lookup' iterates over two maps (dired and global) and calls
>     `Flookup_key' on them.  When `Flookup_key' returns 1 for the key
>     "M-gn" on the dired map, the loop in `shadow_lookup' skips it
>
> That's the place that has to be changed.
>
> When this call to `Flookup_key' returns a number N, `shadow_lookup'
> should look up a sub-key-sequence of the first N events.  If that
> returns non-nil, the longer key is shadowed, so don't mention it at all.
> If that returns non-nil, the longer key is not shadowed.

This is what I considered initially as one variant, but wanted to make
the returned value of `Flookup_key' more useful.  If this is not needed,
then the patch below changes only `shadow_lookup'.  I tested it, and
it works.

Index: src/keymap.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keymap.c,v
retrieving revision 1.307
diff -c -r1.307 keymap.c
*** src/keymap.c	12 Sep 2005 10:26:35 -0000	1.307
--- src/keymap.c	7 Nov 2005 14:17:55 -0000
***************
*** 2377,2383 ****
    for (tail = shadow; CONSP (tail); tail = XCDR (tail))
      {
        value = Flookup_key (XCAR (tail), key, flag);
!       if (!NILP (value) && !NATNUMP (value))
  	return value;
      }
    return Qnil;
--- 2377,2389 ----
    for (tail = shadow; CONSP (tail); tail = XCDR (tail))
      {
        value = Flookup_key (XCAR (tail), key, flag);
!       if (NATNUMP (value))
! 	{
! 	  value = Flookup_key (XCAR (tail), Fsubstring (key, 0, value), flag);
! 	  if (!NILP (value))
! 	    return Qnil;
! 	}
!       else if (!NILP (value))
  	return value;
      }
    return Qnil;

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Stefan Monnier]

>> `shadow_lookup' iterates over two maps (dired and global) and calls
>> `Flookup_key' on them.  When `Flookup_key' returns 1 for the key
>> "M-gn" on the dired map, the loop in `shadow_lookup' skips it
>> 
>> That's the place that has to be changed.
>> 
>> When this call to `Flookup_key' returns a number N, `shadow_lookup'
>> should look up a sub-key-sequence of the first N events.  If that
>> returns non-nil, the longer key is shadowed, so don't mention it at all.
>> If that returns non-nil, the longer key is not shadowed.

> This is what I considered initially as one variant, but wanted to make
> the returned value of `Flookup_key' more useful.  If this is not needed,
> then the patch below changes only `shadow_lookup'.  I tested it, and
> it works.

A compomise might be to return 1 for (lookup-key map [e1 e2 e3]) if [e1 e2] is
bound to a command, and to return -1 if [e1] is bound to a prefix key
and [e1 e2] is not bound to anything.

Callers might need to be updated but a quick `abs' is all it takes to
recover the current info.


        Stefan

Re: M-g in dired

[Juri Linkov]

> A compromise might be to return 1 for (lookup-key map [e1 e2 e3]) if
> [e1 e2] is bound to a command, and to return -1 if [e1] is bound to
> a prefix key and [e1 e2] is not bound to anything.
>
> Callers might need to be updated but a quick `abs' is all it takes
> to recover the current info.

Since this is an incompatible change, there is a risk of breaking code
outside Emacs.  My first proposal was backward incompatible too, but
the risk was smaller.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: M-g in dired

[Richard M. Stallman]

I've decided not to change the specs of lookup-key.  
Please let's let that thread drop.

Re: M-g in dired

[Richard M. Stallman]

    This is what I considered initially as one variant, but wanted to make
    the returned value of `Flookup_key' more useful.  If this is not needed,
    then the patch below changes only `shadow_lookup'.  I tested it, and
    it works.

Please install your patch.

Re: M-g in dired

[Richard M. Stallman]

    But in dired-mode-map, where "C-x `" is undefined, lookup-key
    currently returns:

      (lookup-key dired-mode-map "\C-x") => nil
      (lookup-key dired-mode-map "\C-x`") => 1

    The return value `1' of the last expression seems wrong.

It is correct, because C-x ` is too long to be a valid key sequence.

    For the case of dired-mode-map where lookup-key returns 1 for "\C-x`",
    1 is not a number of events at the front of "\C-x`" that compose
    a complete key, because "C-x" alone is not a valid key in dired-mode-map.

Yes it is.  Every one-character key is valid, always.

So there is no bug in lookup-key.  No need to consider changing it.