Define-key doc string - binding changes always if it exists

Define-key doc string - binding changes always if it exists

[Lennart Borgman]

I found the doc string for define-key a bit strange. It kind of suggests 
that if the keymap is not sparse then it is not changed. I suggest the 
following patch:

Index: keymap.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keymap.c,v
retrieving revision 1.306
diff -u -r1.306 keymap.c
--- keymap.c    7 Aug 2005 12:33:17 -0000    1.306
+++ keymap.c    10 Aug 2005 19:42:37 -0000
@@ -1105,9 +1105,8 @@
     (DEFN should be a valid definition in its own right),
  or a cons (MAP . CHAR), meaning use definition of CHAR in keymap MAP.
 
-If KEYMAP is a sparse keymap with a binding for KEY, the existing
-binding is altered.  If there is no binding for KEY, the new pair
-binding KEY to DEF is added at the front of KEYMAP.  */)
+If KEYMAP is a sparse keymap and there is no binding for KEY, the new
+pair binding KEY to DEF is added at the front of KEYMAP.  */)
      (keymap, key, def)
      Lisp_Object keymap;
      Lisp_Object key;

Re: Define-key doc string - binding changes always if it exists

[Kevin Rodgers]

Lennart Borgman wrote:
 > I found the doc string for define-key a bit strange. It kind of suggests
 > that if the keymap is not sparse then it is not changed. I suggest the
 > following patch:

I don't agree.  You've changed:

 > -If KEYMAP is a sparse keymap with a binding for KEY, the existing
 > -binding is altered.  If there is no binding for KEY, the new pair
 > -binding KEY to DEF is added at the front of KEYMAP.  */)

to:

 > +If KEYMAP is a sparse keymap and there is no binding for KEY, the new
 > +pair binding KEY to DEF is added at the front of KEYMAP.  */)

The original does not say anything about full keymaps (because there is
nothing to add), whereas your version doesn't say anything about a KEY
that's already bound in a sparse KEYMAP.

Your version is close to what's in Emacs 21, which was corrected to
distinguish between a new binding and an altered binding:

| If KEYMAP is a sparse keymap, the pair binding KEY to DEF is added at
| the front of KEYMAP.

-- 
Kevin Rodgers

Re: Define-key doc string - binding changes always if it exists

[Lennart Borgman]

Kevin Rodgers wrote:

> I don't agree.  You've changed:
>
> > -If KEYMAP is a sparse keymap with a binding for KEY, the existing
> > -binding is altered.  If there is no binding for KEY, the new pair
> > -binding KEY to DEF is added at the front of KEYMAP.  */)
>
> to:
>
> > +If KEYMAP is a sparse keymap and there is no binding for KEY, the new
> > +pair binding KEY to DEF is added at the front of KEYMAP.  */)
>
> The original does not say anything about full keymaps (because there is
> nothing to add), whereas your version doesn't say anything about a KEY
> that's already bound in a sparse KEYMAP.

I thought that in the same way there is nothing to add about full 
keymaps there is nothing to add in the case when there is a binding. It 
behaves the same for both full and sparse keymaps.

>
> Your version is close to what's in Emacs 21, which was corrected to
> distinguish between a new binding and an altered binding:
>
> | If KEYMAP is a sparse keymap, the pair binding KEY to DEF is added at
> | the front of KEYMAP.

Yes, but with the important distinction that it is more specific, it 
tries to say clearly that it is just for the case when there is no 
binding. However if people do not find this clear then of course we 
should not change it. Though I personally think it is more clear... ;-)

This is no big issue of course.

Re: Define-key doc string - binding changes always if it exists

[Stefan Monnier]

>      (DEFN should be a valid definition in its own right),
>   or a cons (MAP . CHAR), meaning use definition of CHAR in keymap MAP.
>  -If KEYMAP is a sparse keymap with a binding for KEY, the existing
> -binding is altered.  If there is no binding for KEY, the new pair
> -binding KEY to DEF is added at the front of KEYMAP.  */)
> +If KEYMAP is a sparse keymap and there is no binding for KEY, the new
> +pair binding KEY to DEF is added at the front of KEYMAP.  */)

The behavior should be the same with sparse maps as for dense maps,
at least from the function's user point of view, so the docstring shouldn't
need to distinguish.
Instead of the above patch, I'd remove the word "sparse".
Dense maps also use pairs (KEY . DEF) for non-char keys.


        Stefan