unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* longlines-mode doesn't seem to work with some non-english text
@ 2006-02-10  9:34 Miles Bader
  2006-02-11  4:35 ` Chong Yidong
  2006-09-09 15:08 ` local keymap patch for key-binding Chong Yidong
  0 siblings, 2 replies; 55+ messages in thread
From: Miles Bader @ 2006-02-10  9:34 UTC (permalink / raw)
  Cc: emacs-devel

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

E.g. the text in following attachment.

It fills fine with M-x fill-paragraph, but longlines-mode doesn't seem
to do anything (it remains one long line).

Thanks,

-Miles


[-- Attachment #2: Text that longlines-mode won't fill --]
[-- Type: text/plain, Size: 393 bytes --]

スカイソフトサイトが2006年1月12日に外部より不正な攻撃を受けました。1月12日の被害は軽微で、修復と暫定的な攻撃防御策を即日実施いたしました。1月12日・13日に過去1ヶ月間にわたり同様の攻撃による被害が無いか集中的に調査しましたが、同様の攻撃を受けた形跡が発見されなかったため、さらに遡って調査を継続しながらサイト運営を続けてまいりました。

[-- Attachment #3: Type: text/plain, Size: 152 bytes --]



-- 
The car has become... an article of dress without which we feel uncertain,
unclad, and incomplete.  [Marshall McLuhan, Understanding Media, 1964]

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: longlines-mode doesn't seem to work with some non-english text
  2006-02-10  9:34 longlines-mode doesn't seem to work with some non-english text Miles Bader
@ 2006-02-11  4:35 ` Chong Yidong
  2006-02-13  0:32   ` Kevin Ryde
  2006-09-09 15:08 ` local keymap patch for key-binding Chong Yidong
  1 sibling, 1 reply; 55+ messages in thread
From: Chong Yidong @ 2006-02-11  4:35 UTC (permalink / raw)
  Cc: emacs-devel

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

> E.g. the text in following attachment.
>
> It fills fine with M-x fill-paragraph, but longlines-mode doesn't seem
> to do anything (it remains one long line).

This is a rather difficult case to make Longlines mode handle.  (Btw,
fill-paragraph doesn't do a perfect job either -- if you refill with a
different fill-column, it inserts spurious spaces where the newlines
had previously been inserted after ascii characters.)

I'll look into it, but it may take a while...

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: longlines-mode doesn't seem to work with some non-english text
  2006-02-11  4:35 ` Chong Yidong
@ 2006-02-13  0:32   ` Kevin Ryde
  0 siblings, 0 replies; 55+ messages in thread
From: Kevin Ryde @ 2006-02-13  0:32 UTC (permalink / raw)


Chong Yidong <cyd@stupidchicken.com> writes:
>
> Miles Bader <miles.bader@necel.com> writes:
>
>> E.g. the text in following attachment.
>>
>> It fills fine with M-x fill-paragraph, but longlines-mode doesn't seem
>> to do anything (it remains one long line).
>
> This is a rather difficult case to make Longlines mode handle.

I thought fill-paragraph staying one line was a feature, I've been
using it to collapse runs of multiple spaces.  ... Though I was going
to complain that it collapses two spaces after a full-stop down to one
space if the full stop is at the end of a line.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* local keymap patch for key-binding
  2006-02-10  9:34 longlines-mode doesn't seem to work with some non-english text Miles Bader
  2006-02-11  4:35 ` Chong Yidong
@ 2006-09-09 15:08 ` Chong Yidong
  2006-09-09 15:15   ` David Kastrup
  1 sibling, 1 reply; 55+ messages in thread
From: Chong Yidong @ 2006-09-09 15:08 UTC (permalink / raw)


I have written a patch to address the following item in FOR-RELEASE:

    ** Make key-binding use the maps specified by positions given in
       the events.

If a vector is passed to key-binding, and the first element of the
vector looks like a valid click event, and the position specified by
that click event contains a `keymap' property, then we look in that
keymap first.

Any objections to checking this in?

*** emacs/src/keymap.c.~1.330.~	2006-07-24 11:48:55.000000000 -0400
--- emacs/src/keymap.c	2006-09-09 11:06:23.000000000 -0400
***************
*** 1576,1581 ****
--- 1576,1606 ----
  
    GCPRO1 (key);
  
+ #ifdef HAVE_MOUSE
+   if (VECTORP (key)
+       && ASIZE (key) > 0
+       && CONSP (AREF (key, 0))
+       && SYMBOLP (XCAR (AREF (key, 0)))
+       && CONSP (XCDR (AREF (key, 0))))
+     {
+       Lisp_Object map, obj, pos = XCAR (XCDR (AREF (key, 0)));
+ 
+       if (XINT (Flength (pos)) == 10 && INTEGERP (XCAR (XCDR (pos))))
+ 	{
+ 	  obj = Fnth (make_number(4), pos);
+ 	  map = Fget_char_property (XCAR (XCDR (pos)),
+ 				    Qkeymap,
+ 				    NILP (obj) ? Fwindow_buffer (XCAR (pos)) : obj);
+ 	  if (!NILP (Fkeymapp (map)))
+ 	    {
+ 	      value = Flookup_key (map, key, accept_default);
+ 	      if (! NILP (value) && !INTEGERP (value))
+ 		goto done;
+ 	    }
+ 	}
+     }
+ #endif /* HAVE_MOUSE  */
+ 
    if (!NILP (current_kboard->Voverriding_terminal_local_map))
      {
        value = Flookup_key (current_kboard->Voverriding_terminal_local_map,

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-09 15:08 ` local keymap patch for key-binding Chong Yidong
@ 2006-09-09 15:15   ` David Kastrup
  2006-09-09 15:26     ` Chong Yidong
  2006-09-10 12:44     ` Chong Yidong
  0 siblings, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-09 15:15 UTC (permalink / raw)
  Cc: emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> I have written a patch to address the following item in FOR-RELEASE:
>
>     ** Make key-binding use the maps specified by positions given in
>        the events.
>
> If a vector is passed to key-binding, and the first element of the
> vector looks like a valid click event, and the position specified by
> that click event contains a `keymap' property, then we look in that
> keymap first.
>
> Any objections to checking this in?

It does too little.  Check out what read-key-sequence (defined in
keyboard.c) does with regard to mouse events (EVENT_HAS_PARAMETERS).
The problem is that read-key-sequence does such a load of other stuff
that it is hard to extract the material and transfer it to
key-binding.

The problem is that keymaps may be provided by text properties and
overlays, and by keymap properties on strings that display as the
display or before-string or after-string properties of text properties
or overlays.

So one really wants to steal the logic from read-key-sequence without
stealing most of the complications...

Basically
(lookup-binding (read-key-sequence))
should arrive at the same conclusion as that which read-key-sequence
stores in the read_key_sequence_cmd variable as a side effect.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-09 15:15   ` David Kastrup
@ 2006-09-09 15:26     ` Chong Yidong
  2006-09-10 12:44     ` Chong Yidong
  1 sibling, 0 replies; 55+ messages in thread
From: Chong Yidong @ 2006-09-09 15:26 UTC (permalink / raw)
  Cc: emacs-devel

David Kastrup <dak@gnu.org> writes:

> The problem is that keymaps may be provided by text properties and
> overlays, and by keymap properties on strings that display as the
> display or before-string or after-string properties of text properties
> or overlays.

Why wouldn't this be caught by the call to Fget_char_property, called
on the object (string or buffer) specified by the supplied event?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-09 15:15   ` David Kastrup
  2006-09-09 15:26     ` Chong Yidong
@ 2006-09-10 12:44     ` Chong Yidong
  2006-09-10 13:25       ` David Kastrup
  2006-09-10 18:52       ` Richard Stallman
  1 sibling, 2 replies; 55+ messages in thread
From: Chong Yidong @ 2006-09-10 12:44 UTC (permalink / raw)
  Cc: emacs-devel

David Kastrup <dak@gnu.org> writes:

> It does too little.  Check out what read-key-sequence (defined in
> keyboard.c) does with regard to mouse events (EVENT_HAS_PARAMETERS).
> The problem is that read-key-sequence does such a load of other stuff
> that it is hard to extract the material and transfer it to
> key-binding.
>
> The problem is that keymaps may be provided by text properties and
> overlays, and by keymap properties on strings that display as the
> display or before-string or after-string properties of text properties
> or overlays.

I just verified that my patch truly works for keymaps on display
strings, text properties, and overlays.  For example, with

(with-output-to-temp-buffer "Testo"
  (set-buffer "Testo")
  (erase-buffer)
  (let* ((map (make-sparse-keymap))
	 (str (propertize "foostring" 'keymap map 'mouse-face 'highlight))
	 ovr)
    (define-key map [mouse-1] 'delete-window)
    (insert "1234567890")
    (setq ovr (make-overlay 2 3))
    (overlay-put ovr 'keymap map)
    (overlay-put ovr 'mouse-face 'highlight)
    (put-text-property 5 6 'display str)
    (put-text-property 8 9 'keymap map)
    (put-text-property 8 9 'mouse-face 'highlight)))

C-h k followed by mouse-1 on the overlay, display string, or text
propertized region all report the correct action for the given local
keymap.  The reason is that the mouse event passed to `key-binding'
contains all the information you need to reconstruct the binding
directly (including whether or not the object we are looking at is a
buffer position or a string.)

So it seems like there's no problem.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-10 12:44     ` Chong Yidong
@ 2006-09-10 13:25       ` David Kastrup
  2006-09-11  2:36         ` Chong Yidong
  2006-09-10 18:52       ` Richard Stallman
  1 sibling, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-10 13:25 UTC (permalink / raw)
  Cc: emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> It does too little.  Check out what read-key-sequence (defined in
>> keyboard.c) does with regard to mouse events (EVENT_HAS_PARAMETERS).
>> The problem is that read-key-sequence does such a load of other stuff
>> that it is hard to extract the material and transfer it to
>> key-binding.
>>
>> The problem is that keymaps may be provided by text properties and
>> overlays, and by keymap properties on strings that display as the
>> display or before-string or after-string properties of text properties
>> or overlays.
>
> I just verified that my patch truly works for keymaps on display
> strings, text properties, and overlays.  For example, with
>
> (with-output-to-temp-buffer "Testo"
>   (set-buffer "Testo")
>   (erase-buffer)
>   (let* ((map (make-sparse-keymap))
> 	 (str (propertize "foostring" 'keymap map 'mouse-face 'highlight))
> 	 ovr)
>     (define-key map [mouse-1] 'delete-window)
>     (insert "1234567890")
>     (setq ovr (make-overlay 2 3))
>     (overlay-put ovr 'keymap map)
>     (overlay-put ovr 'mouse-face 'highlight)
>     (put-text-property 5 6 'display str)
>     (put-text-property 8 9 'keymap map)
>     (put-text-property 8 9 'mouse-face 'highlight)))
>
> C-h k followed by mouse-1 on the overlay, display string, or text
> propertized region all report the correct action for the given local
> keymap.

Uh, C-h k has extra code doing this sort of lookup.  It does not rely
on key-binding.  So you should not test this with C-h k unless you
have previously dumbed it down with
(defun string-key-binding (&rest ignore) nil)
after help.el has been loaded.

> The reason is that the mouse event passed to `key-binding' contains
> all the information you need to reconstruct the binding directly
> (including whether or not the object we are looking at is a buffer
> position or a string.)
>
> So it seems like there's no problem.

Amazing.  Do you have an idea why the stuff in read-key-sequence
concerning mouse related maps appears so much more complicated?  Or is
it just me?  Some complication, of course, is due to read-key-sequence
having to _assemble_ a key sequence instead of merely looking it up.
But still...

Concretely, we have the following in keyboard.c concerning mouse
events, and I'll add my own comments with // in between.

      if (EVENT_HAS_PARAMETERS (key))
	{
	  Lisp_Object kind;
	  Lisp_Object string;

	  kind = EVENT_HEAD_KIND (EVENT_HEAD (key));
	  if (EQ (kind, Qmouse_click))
	    {
	      Lisp_Object window, posn;

	      window = POSN_WINDOW      (EVENT_START (key));
	      posn   = POSN_POSN (EVENT_START (key));

// Fake prefixes probably not relevant for keybinding-lookup
	      if (CONSP (posn)
		  || (!NILP (fake_prefixed_keys)
		      && !NILP (Fmemq (key, fake_prefixed_keys))))
		{
// elided
		}

	      /* Key sequences beginning with mouse clicks are
		 read using the keymaps in the buffer clicked on,
		 not the current buffer.  If we're at the
		 beginning of a key sequence, switch buffers.  */

// Do you change the buffer accordingly in your patch?

	      if (last_real_key_start == 0
		  && WINDOWP (window)
		  && BUFFERP (XWINDOW (window)->buffer)
		  && XBUFFER (XWINDOW (window)->buffer) != current_buffer)
		{
		  XVECTOR (raw_keybuf)->contents[raw_keybuf_count++] = key;
		  keybuf[t] = key;
		  mock_input = t + 1;

		  /* Arrange to go back to the original buffer once we're
		     done reading the key sequence.  Note that we can't
		     use save_excursion_{save,restore} here, because they
		     save point as well as the current buffer; we don't
		     want to save point, because redisplay may change it,
		     to accommodate a Fset_window_start or something.  We
		     don't want to do this at the top of the function,
		     because we may get input from a subprocess which
		     wants to change the selected window and stuff (say,
		     emacsclient).  */
		  record_unwind_protect (Fset_buffer, Fcurrent_buffer ());

		  if (! FRAME_LIVE_P (XFRAME (selected_frame)))
		    Fkill_emacs (Qnil);
		  set_buffer_internal (XBUFFER (XWINDOW (window)->buffer));
		  orig_local_map = get_local_map (PT, current_buffer,
						  Qlocal_map);
		  orig_keymap = get_local_map (PT, current_buffer, Qkeymap);
		  goto replay_sequence;

// This starts the whole lookup sequence with new values for local-map
// and, well, I don't quite understand what Qkeymap is.
		}

	      /* For a mouse click, get the local text-property keymap
		 of the place clicked on, rather than point.  */
	      if (last_real_key_start == 0
		  && CONSP (XCDR (key))
		  && ! localized_local_map)
		{
		  Lisp_Object map_here, start, pos;

		  localized_local_map = 1;
		  start = EVENT_START (key);

		  if (CONSP (start) && POSN_INBUFFER_P (start))
		    {
		      pos = POSN_BUFFER_POSN (start);
		      if (INTEGERP (pos)
			  && XINT (pos) >= BEG && XINT (pos) <= Z)
			{
			  map_here = get_local_map (XINT (pos),
						    current_buffer, Qlocal_map);
			  if (!EQ (map_here, orig_local_map))
			    {
			      orig_local_map = map_here;
			      keybuf[t] = key;
			      mock_input = t + 1;

			      goto replay_sequence;
			    }
			  map_here = get_local_map (XINT (pos),
						     current_buffer, Qkeymap);
			  if (!EQ (map_here, orig_keymap))
			    {
			      orig_keymap = map_here;
			      keybuf[t] = key;
			      mock_input = t + 1;

			      goto replay_sequence;
			    }
			}
		    }
		}

// Another class of events: but they mostly don't require looking in
// other maps

	      /* Expand mode-line and scroll-bar events into two events:
		 use posn as a fake prefix key.  */
	      if (SYMBOLP (posn)
		  && (NILP (fake_prefixed_keys)
		      || NILP (Fmemq (key, fake_prefixed_keys))))
		{
		  if (t + 1 >= bufsize)
		    error ("Key sequence too long");

		  keybuf[t]     = posn;
		  keybuf[t + 1] = key;
		  mock_input    = t + 2;

		  /* Record that a fake prefix key has been generated
		     for KEY.  Don't modify the event; this would
		     prevent proper action when the event is pushed
		     back into unread-command-events.  */
		  fake_prefixed_keys = Fcons (key, fake_prefixed_keys);

// Except in the following case:

		  /* If on a mode line string with a local keymap,
		     reconsider the key sequence with that keymap.  */
		  if (string = POSN_STRING (EVENT_START (key)),
		      (CONSP (string) && STRINGP (XCAR (string))))
		    {
		      Lisp_Object pos, map, map2;

		      pos = XCDR (string);
		      string = XCAR (string);
                      if (XINT (pos) >= 0
			  && XINT (pos) < SCHARS (string))
                        {
                          map = Fget_text_property (pos, Qlocal_map, string);
                          if (!NILP (map))
                            orig_local_map = map;
                          map2 = Fget_text_property (pos, Qkeymap, string);
                          if (!NILP (map2))
                            orig_keymap = map2;
                          if (!NILP (map) || !NILP (map2))
                            goto replay_sequence;
                        }
		    }

		  goto replay_key;
		}
	      else if (NILP (from_string)
		       && (string = POSN_STRING (EVENT_START (key)),
			   (CONSP (string) && STRINGP (XCAR (string)))))
		{
		  /* For a click on a string, i.e. overlay string or a
		     string displayed via the `display' property,
		     consider `local-map' and `keymap' properties of
		     that string.  */
		  Lisp_Object pos, map, map2;

		  pos = XCDR (string);
		  string = XCAR (string);
		  if (XINT (pos) >= 0
		      && XINT (pos) < SCHARS (string))
		    {
		      map = Fget_text_property (pos, Qlocal_map, string);
		      if (!NILP (map))
			orig_local_map = map;
		      map2 = Fget_text_property (pos, Qkeymap, string);
		      if (!NILP (map2))
			orig_keymap = map2;

		      if (!NILP (map) || !NILP (map2))
			{
			  from_string = string;
			  goto replay_sequence;
			}
		    }
		}
	    }

// Menu bar: seemingly does not require other maps.

	  else if (CONSP (XCDR (key))
		   && CONSP (EVENT_START (key))
		   && CONSP (XCDR (EVENT_START (key))))
	    {
	      Lisp_Object posn;

	      posn = POSN_POSN (EVENT_START (key));
	      /* Handle menu-bar events:
		 insert the dummy prefix event `menu-bar'.  */
	      if (EQ (posn, Qmenu_bar) || EQ (posn, Qtool_bar))
		{
		  if (t + 1 >= bufsize)
		    error ("Key sequence too long");
		  keybuf[t] = posn;
		  keybuf[t+1] = key;

		  /* Zap the position in key, so we know that we've
		     expanded it, and don't try to do so again.  */
		  POSN_SET_POSN (EVENT_START (key),
				 Fcons (posn, Qnil));

		  mock_input = t + 2;
		  goto replay_sequence;
		}
	      else if (CONSP (posn))
		{
		  /* We're looking at the second event of a
		     sequence which we expanded before.  Set
		     last_real_key_start appropriately.  */
		  if (last_real_key_start == t && t > 0)
		    last_real_key_start = t - 1;
		}
	    }
	}


If it seems to you that your patch gets the same cases (and with the
same order of priorities), that would be great!

Another thing: I had proposed an additional optional "LOCATION"
argument for key-binding.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-10 12:44     ` Chong Yidong
  2006-09-10 13:25       ` David Kastrup
@ 2006-09-10 18:52       ` Richard Stallman
  2006-09-11  2:39         ` Chong Yidong
  1 sibling, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-10 18:52 UTC (permalink / raw)
  Cc: emacs-devel

Please install your patch (and remove that item from FOR-RELEASE).

Can you put text in the Lisp Manual, under key-binding, to explain this?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-10 13:25       ` David Kastrup
@ 2006-09-11  2:36         ` Chong Yidong
  2006-09-11  6:25           ` David Kastrup
  2006-09-11  7:05           ` David Kastrup
  0 siblings, 2 replies; 55+ messages in thread
From: Chong Yidong @ 2006-09-11  2:36 UTC (permalink / raw)
  Cc: emacs-devel

David Kastrup <dak@gnu.org> writes:

>> C-h k followed by mouse-1 on the overlay, display string, or text
>> propertized region all report the correct action for the given local
>> keymap.
>
> Uh, C-h k has extra code doing this sort of lookup.  It does not rely
> on key-binding.

It also works with (setq foo (read-key-sequence ""))

> Do you have an idea why the stuff in read-key-sequence concerning
> mouse related maps appears so much more complicated?  Or is it just
> me?  Some complication, of course, is due to read-key-sequence
> having to _assemble_ a key sequence instead of merely looking it up.

What you said.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-10 18:52       ` Richard Stallman
@ 2006-09-11  2:39         ` Chong Yidong
  0 siblings, 0 replies; 55+ messages in thread
From: Chong Yidong @ 2006-09-11  2:39 UTC (permalink / raw)
  Cc: emacs-devel

Richard Stallman <rms@gnu.org> writes:

> Please install your patch (and remove that item from FOR-RELEASE).

Done.

> Can you put text in the Lisp Manual, under key-binding, to explain this?

Done.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11  2:36         ` Chong Yidong
@ 2006-09-11  6:25           ` David Kastrup
  2006-09-11  7:05           ` David Kastrup
  1 sibling, 0 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-11  6:25 UTC (permalink / raw)
  Cc: emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> David Kastrup <dak@gnu.org> writes:
>
>>> C-h k followed by mouse-1 on the overlay, display string, or text
>>> propertized region all report the correct action for the given local
>>> keymap.
>>
>> Uh, C-h k has extra code doing this sort of lookup.  It does not rely
>> on key-binding.
>
> It also works with (setq foo (read-key-sequence ""))

I don't understand at all what you are trying to say here.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11  2:36         ` Chong Yidong
  2006-09-11  6:25           ` David Kastrup
@ 2006-09-11  7:05           ` David Kastrup
  2006-09-11  8:52             ` Kim F. Storm
  1 sibling, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-11  7:05 UTC (permalink / raw)
  Cc: emacs-devel

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

Chong Yidong <cyd@stupidchicken.com> writes:

> David Kastrup <dak@gnu.org> writes:
>
>>> C-h k followed by mouse-1 on the overlay, display string, or text
>>> propertized region all report the correct action for the given local
>>> keymap.
>>
>> Uh, C-h k has extra code doing this sort of lookup.  It does not rely
>> on key-binding.
>
> It also works with (setq foo (read-key-sequence ""))
>
>> Do you have an idea why the stuff in read-key-sequence concerning
>> mouse related maps appears so much more complicated?  Or is it just
>> me?  Some complication, of course, is due to read-key-sequence
>> having to _assemble_ a key sequence instead of merely looking it up.
>
> What you said.

Well, it does not seem just as easy as that.  I now can't start
RefTeX anymore without getting the error:


[-- Attachment #2: Type: text/plain, Size: 1511 bytes --]

Debugger entered--Lisp error: (wrong-type-argument sequencep mouse-2)
  key-binding([(shift mouse-2)])
  byte-code("Æ\x18Ç\x19\b:ƒ^[\0\b@\x11È\n	@	A#ˆ\bA‰\x10‚\x05\0*ÉÊ!„(\0È\nËÌ#ˆÍÎÏ\"ˆ\vƒM\0Ð\x1cÇ\x19\f:ƒL\0\f@\x11È\n	@	A#ˆ\fA‰\x14‚6\0*Ñ\rB\x15ÒÑ!„\\\0ÓÑÇ\"ˆÔÑ!ˆÕÖ!‡" [--cl-var-- x reftex-mode-map reftex-extra-bindings --cl-var-- current-load-list (("\x03=" . reftex-toc) ("\x03-" . reftex-toc-recenter) ("\x03(" . reftex-label) ("\x03)" . reftex-reference) ("\x03[" . reftex-citation) ("\x03<" . reftex-index) ("\x03>" . reftex-display-index) ("\x03/" . reftex-index-selection-or-word) ("\x03\\" . reftex-index-phrase-selection-or-word) ("\x03|" . reftex-index-visit-phrases-buffer) ("\x03&" . reftex-view-crossref)) nil define-key key-binding [(shift mouse-2)] [(shift mouse-2)] reftex-mouse-view-crossref eval-after-load "bibtex" (define-key bibtex-mode-map "\x03&" (quote reftex-view-crossref-from-bibtex)) (("\x03t" . reftex-toc) ("\x03l" . reftex-label) ("\x03r" . reftex-reference) ("\x03c" . reftex-citation) ("\x03v" . reftex-view-crossref) ("\x03g" . reftex-grep-document) ("\x03s" . reftex-search-document)) reftex-isearch-minor-mode default-boundp set-default make-variable-buffer-local require easymenu] 5)
  turn-on-reftex()
  run-hooks(text-mode-hook TeX-mode-hook LaTeX-mode-hook)
  apply(run-hooks (text-mode-hook TeX-mode-hook LaTeX-mode-hook))
  TeX-run-mode-hooks(text-mode-hook TeX-mode-hook LaTeX-mode-hook)
  latex-mode()
  call-interactively(latex-mode)
  execute-extended-command(nil)
  call-interactively(execute-extended-command)

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11  7:05           ` David Kastrup
@ 2006-09-11  8:52             ` Kim F. Storm
  2006-09-11  9:48               ` David Kastrup
  0 siblings, 1 reply; 55+ messages in thread
From: Kim F. Storm @ 2006-09-11  8:52 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

David Kastrup <dak@gnu.org> writes:

> Well, it does not seem just as easy as that.  I now can't start
> RefTeX anymore without getting the error:
>
> Debugger entered--Lisp error: (wrong-type-argument sequencep mouse-2)
>   key-binding([(shift mouse-2)])

Should be fixed now.

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11  8:52             ` Kim F. Storm
@ 2006-09-11  9:48               ` David Kastrup
  2006-09-11 10:11                 ` David Kastrup
  2006-09-11 19:58                 ` Richard Stallman
  0 siblings, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-11  9:48 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Well, it does not seem just as easy as that.  I now can't start
>> RefTeX anymore without getting the error:
>>
>> Debugger entered--Lisp error: (wrong-type-argument sequencep mouse-2)
>>   key-binding([(shift mouse-2)])
>
> Should be fixed now.

Hm, looks like it works now.  I have removed string-key-binding from
help.el (it is no longer necessary) and the results appear fine to me.

Since we want to get good testing for the key-binding stuff, I'll
check this change to help.el in.  I am still not convinced that the
overriding keymap thingies are properly dealt with, but I'll have to
come up with a test case involving them before complaining.

key-binding in its new incantation should also be used in mouse.el to
get the bindings for `follow-link' and similar.  I had posted an API
proposal for `key-binding' previously, but it suffered from not
realizing the difference between events, and keysequences made from
events (`key-binding' can't be used to look up events, and I am less
than certain that it should).

So we probably should either

a) add an additional optional parameter BASE-KEY which, when non-NIL,
will replace KEY for all purposes of lookup except determining the
keymaps to use when KEY is a key-sequence based on an event.

b) have a function `event-replace-key' or something with a better name
which will take a key sequence based on an event and swap out the
basic event (but maybe _not_ its modifiers since they can play a part
in which keymap to use) and swap out the `down-mouse-2' or similar
with `follow-link'.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11  9:48               ` David Kastrup
@ 2006-09-11 10:11                 ` David Kastrup
  2006-09-11 12:53                   ` Kim F. Storm
  2006-09-11 13:07                   ` Chong Yidong
  2006-09-11 19:58                 ` Richard Stallman
  1 sibling, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-11 10:11 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

David Kastrup <dak@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
>
>> David Kastrup <dak@gnu.org> writes:
>>
>>> Well, it does not seem just as easy as that.  I now can't start
>>> RefTeX anymore without getting the error:
>>>
>>> Debugger entered--Lisp error: (wrong-type-argument sequencep mouse-2)
>>>   key-binding([(shift mouse-2)])
>>
>> Should be fixed now.
>
> Hm, looks like it works now.  I have removed string-key-binding from
> help.el (it is no longer necessary) and the results appear fine to me.
>
> Since we want to get good testing for the key-binding stuff, I'll
> check this change to help.el in.

Ok, here I triggered a problem: I used C-h k (with the new help.el)
followed by a mouse-2 click on an overlay's 'before-string which has
its own 'keymap and 'display properties (the string, not the overlay).

I get

Debugger entered--Lisp error: (args-out-of-range 806 806)
  key-binding([(mouse-2 (#<window 41 on ang.tex> 806 ... -61397984 ... 806 ... ... ... ...))] t)
  describe-key([(mouse-2 (#<window 41 on ang.tex> 806 ... -61397984 ... 806 ... ... ... ...))] 1 nil)
  call-interactively(describe-key)

It would appear that 806 is the buffer position, and I would guess
that this is erroneously used as in index into the before-string
property of the overlay.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11 10:11                 ` David Kastrup
@ 2006-09-11 12:53                   ` Kim F. Storm
  2006-09-11 13:04                     ` David Kastrup
  2006-09-11 13:07                   ` Chong Yidong
  1 sibling, 1 reply; 55+ messages in thread
From: Kim F. Storm @ 2006-09-11 12:53 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

David Kastrup <dak@gnu.org> writes:

> Ok, here I triggered a problem: I used C-h k (with the new help.el)
> followed by a mouse-2 click on an overlay's 'before-string which has
> its own 'keymap and 'display properties (the string, not the overlay).
>
> I get
>
> Debugger entered--Lisp error: (args-out-of-range 806 806)
>   key-binding([(mouse-2 (#<window 41 on ang.tex> 806 ... -61397984 ... 806 ... ... ... ...))] t)
>   describe-key([(mouse-2 (#<window 41 on ang.tex> 806 ... -61397984 ... 806 ... ... ... ...))] 1 nil)
>   call-interactively(describe-key)
>
> It would appear that 806 is the buffer position, and I would guess
> that this is erroneously used as in index into the before-string
> property of the overlay.

I need a self-contained test-case to work on that.
And I need to know the full contents of the failing event.

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11 12:53                   ` Kim F. Storm
@ 2006-09-11 13:04                     ` David Kastrup
  0 siblings, 0 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-11 13:04 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Ok, here I triggered a problem: I used C-h k (with the new help.el)
>> followed by a mouse-2 click on an overlay's 'before-string which has
>> its own 'keymap and 'display properties (the string, not the overlay).
>>
>> I get
>>
>> Debugger entered--Lisp error: (args-out-of-range 806 806)
>>   key-binding([(mouse-2 (#<window 41 on ang.tex> 806 ... -61397984 ... 806 ... ... ... ...))] t)
>>   describe-key([(mouse-2 (#<window 41 on ang.tex> 806 ... -61397984 ... 806 ... ... ... ...))] 1 nil)
>>   call-interactively(describe-key)
>>
>> It would appear that 806 is the buffer position, and I would guess
>> that this is erroneously used as in index into the before-string
>> property of the overlay.
>
> I need a self-contained test-case to work on that.
> And I need to know the full contents of the failing event.

I found the problem already.  However, contrary to Chong Yidong's
assertion, a proper lookup dealing with keymap property, local-map
property (which Chong does not check at all), correct use of
overriding-local-map and overriding-terminal-local-map (which have to
be consulted before the keymaps of the mouse event, but based on the
buffer that the event happened in) is not as trivial as the current
code would suggest.

Also, the C-h k test masks some problems because it manually switches
to the buffer where the event occured before looking it up, so we
don't see the case where the event is not from the current
window/buffer/terminal.

I am currently trying to implement this correctly, based on
(info "(elisp) Active Keymaps")
and the code for read-key-sequence.

I'll hopefully be through with this in a few hours.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11 10:11                 ` David Kastrup
  2006-09-11 12:53                   ` Kim F. Storm
@ 2006-09-11 13:07                   ` Chong Yidong
  1 sibling, 0 replies; 55+ messages in thread
From: Chong Yidong @ 2006-09-11 13:07 UTC (permalink / raw)
  Cc: emacs-devel, Kim F. Storm

David Kastrup <dak@gnu.org> writes:

> Ok, here I triggered a problem: I used C-h k (with the new help.el)
> followed by a mouse-2 click on an overlay's 'before-string which has
> its own 'keymap and 'display properties (the string, not the overlay).
>
> It would appear that 806 is the buffer position, and I would guess
> that this is erroneously used as in index into the before-string
> property of the overlay.

I've checked in a fix.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11  9:48               ` David Kastrup
  2006-09-11 10:11                 ` David Kastrup
@ 2006-09-11 19:58                 ` Richard Stallman
  2006-09-12 10:04                   ` David Kastrup
  1 sibling, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-11 19:58 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

     (`key-binding' can't be used to look up events, and I am less
    than certain that it should).

Of course key-binding can be used to look up events.  If you want to
look them up in the current active keymaps, key-binding is the natural
way.  The recent change affects the lookup of events that specify
positions.  It has no effect on what happens if you pass a list of
event-types without positions.

Perhaps there is a misunderstanding here about what it means
to "look up events".

    a) add an additional optional parameter BASE-KEY which, when non-NIL,
    will replace KEY for all purposes of lookup except determining the
    keymaps to use when KEY is a key-sequence based on an event.

I don't follow this at all, sorry.

    b) have a function `event-replace-key' or something with a better name
    which will take a key sequence based on an event and swap out the
    basic event (but maybe _not_ its modifiers since they can play a part
    in which keymap to use) and swap out the `down-mouse-2' or similar
    with `follow-link'.

What does "swap out" mean here?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-11 19:58                 ` Richard Stallman
@ 2006-09-12 10:04                   ` David Kastrup
  2006-09-12 15:21                     ` David Kastrup
  0 siblings, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-12 10:04 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

Richard Stallman <rms@gnu.org> writes:

>      (`key-binding' can't be used to look up events, and I am less
>     than certain that it should).
>
> Of course key-binding can be used to look up events.  If you want to
> look them up in the current active keymaps, key-binding is the
> natural way.

I think we are talking about different things here.  Call
M-: (eventp (read-key-sequence nil)) RET
and click with the mouse somewhere.  You will get "nil" as a result
since read-key-sequence does not return an event, but rather an array
with the 0th element being an event.

key-binding in its current code by Chong can only deal with such an
event _sequence_ (which is not an event according to eventp).

If you thing it reasonable that key-binding should also react to
bona-fide events _not_ wrapped in an array, I can do this easily
enough in the change I am working on.

> The recent change affects the lookup of events that specify
> positions.  It has no effect on what happens if you pass a list of
> event-types without positions.
>
> Perhaps there is a misunderstanding here about what it means
> to "look up events".
>
>     a) add an additional optional parameter BASE-KEY which, when non-NIL,
>     will replace KEY for all purposes of lookup except determining the
>     keymaps to use when KEY is a key-sequence based on an event.
>
> I don't follow this at all, sorry.

We have the situation that we want to look up an artificial
"follow-link" keymap entry depending on the keymaps where a click was
made.  It would be nice to do this with

(key-binding click-event nil nil 'follow-link)

>     b) have a function `event-replace-key' or something with a
>     better name which will take a key sequence based on an event and
>     swap out the basic event (but maybe _not_ its modifiers since
>     they can play a part in which keymap to use) and swap out the
>     `down-mouse-2' or similar with `follow-link'.
>
> What does "swap out" mean here?

Replace it (actually, make a copy and replace the `down-mouse-1'
symbol in the copy with `follow-link').


-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-12 10:04                   ` David Kastrup
@ 2006-09-12 15:21                     ` David Kastrup
  2006-09-12 15:39                       ` PCL-CVS's diff and marks (was: local keymap patch for key-binding) Stefan Monnier
                                         ` (2 more replies)
  0 siblings, 3 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-12 15:21 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

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

David Kastrup <dak@gnu.org> writes:

> If you thing it reasonable that key-binding should also react to
> bona-fide events _not_ wrapped in an array, I can do this easily
> enough in the change I am working on.

Ok, here is my current patch (incidentally: why doesn't it work to use
PCL-CVS to do a multi-file diff? It ignores the marked files, just
working on the current line).  I have added an additional argument to
key-binding and documented it (that required changes in keymap.h and
callers of key-binding from C in keyboard.c).

As far as I can tell, this works with all key sequences.  If people
agree that this is reasonable, I would install it.  I'd be glad if
someone looked it over with regard to the SPECDCL and garbage
collection protection: I don't feel comfortable about those.  I also
copied code from keyboard.c that aborts if the current frame is not
live and can't be returned to.  No idea whether this is required (I
assume it was put there to avoid a busy lockup when Emacs got its
frame killed or something) or even a good idea.

Anybody have an opinion about that?

If people think this is the right way to go, I can install it.  I have
verified it with quite a few tries.  However, I have not yet tested
the LOCATION argument.  It should come in handy for mouse.el and
follow-link.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 12305 bytes --]

Index: src/keyboard.c
===================================================================
RCS file: /sources/emacs/emacs/src/keyboard.c,v
retrieving revision 1.875
diff -c -r1.875 keyboard.c
*** src/keyboard.c	10 Sep 2006 21:10:50 -0000	1.875
--- src/keyboard.c	12 Sep 2006 15:08:14 -0000
***************
*** 7516,7522 ****
        Lisp_Object prefix;
  
        if (!NILP (tem))
! 	tem = Fkey_binding (tem, Qnil, Qnil);
  
        prefix = AREF (item_properties, ITEM_PROPERTY_KEYEQ);
        if (CONSP (prefix))
--- 7516,7522 ----
        Lisp_Object prefix;
  
        if (!NILP (tem))
! 	tem = Fkey_binding (tem, Qnil, Qnil, Qnil);
  
        prefix = AREF (item_properties, ITEM_PROPERTY_KEYEQ);
        if (CONSP (prefix))
***************
*** 9125,9140 ****
  			  if (!EQ (map_here, orig_local_map))
  			    {
  			      orig_local_map = map_here;
! 			      keybuf[t] = key;
! 			      mock_input = t + 1;
! 
! 			      goto replay_sequence;
  			    }
  			  map_here = get_local_map (XINT (pos),
  						     current_buffer, Qkeymap);
  			  if (!EQ (map_here, orig_keymap))
  			    {
  			      orig_keymap = map_here;
  			      keybuf[t] = key;
  			      mock_input = t + 1;
  
--- 9125,9143 ----
  			  if (!EQ (map_here, orig_local_map))
  			    {
  			      orig_local_map = map_here;
! 			      ++localized_local_map;
  			    }
+ 
  			  map_here = get_local_map (XINT (pos),
  						     current_buffer, Qkeymap);
  			  if (!EQ (map_here, orig_keymap))
  			    {
  			      orig_keymap = map_here;
+ 			      ++localized_local_map;
+ 			    }
+ 
+ 			  if (localized_local_map > 1)
+ 			    {
  			      keybuf[t] = key;
  			      mock_input = t + 1;
  
Index: src/keymap.c
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.c,v
retrieving revision 1.333
diff -c -r1.333 keymap.c
*** src/keymap.c	11 Sep 2006 13:03:40 -0000	1.333
--- src/keymap.c	12 Sep 2006 15:08:18 -0000
***************
*** 28,33 ****
--- 28,35 ----
  #include "buffer.h"
  #include "charset.h"
  #include "keyboard.h"
+ #include "frame.h"
+ #include "window.h"
  #include "termhooks.h"
  #include "blockinput.h"
  #include "puresize.h"
***************
*** 1226,1232 ****
      return Qnil;
  
    ASET (command_remapping_vector, 1, command);
!   return Fkey_binding (command_remapping_vector, Qnil, Qt);
  }
  
  /* Value is number if KEY is too long; nil if valid but has no definition. */
--- 1228,1234 ----
      return Qnil;
  
    ASET (command_remapping_vector, 1, command);
!   return Fkey_binding (command_remapping_vector, Qnil, Qt, Qnil);
  }
  
  /* Value is number if KEY is too long; nil if valid but has no definition. */
***************
*** 1552,1558 ****
  
  /* GC is possible in this function if it autoloads a keymap.  */
  
! DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 3, 0,
         doc: /* Return the binding for command KEY in current keymaps.
  KEY is a string or vector, a sequence of keystrokes.
  The binding is probably a symbol with a function definition.
--- 1554,1560 ----
  
  /* GC is possible in this function if it autoloads a keymap.  */
  
! DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 4, 0,
         doc: /* Return the binding for command KEY in current keymaps.
  KEY is a string or vector, a sequence of keystrokes.
  The binding is probably a symbol with a function definition.
***************
*** 1566,1620 ****
  Like the normal command loop, `key-binding' will remap the command
  resulting from looking up KEY by looking up the command in the
  current keymaps.  However, if the optional third argument NO-REMAP
! is non-nil, `key-binding' returns the unmapped command.  */)
!      (key, accept_default, no_remap)
!      Lisp_Object key, accept_default, no_remap;
  {
    Lisp_Object *maps, value;
    int nmaps, i;
!   struct gcpro gcpro1;
  
!   GCPRO1 (key);
  
! #ifdef HAVE_MOUSE
!   if (VECTORP (key) && ASIZE (key) > 0)
      {
!       Lisp_Object ev, pos;
!       if ((ev = AREF (key, 0), CONSP (ev))
! 	  && SYMBOLP (XCAR (ev))
! 	  && CONSP (XCDR (ev))
! 	  && (pos = XCAR (XCDR (ev)), CONSP (pos))
! 	  && XINT (Flength (pos)) == 10
! 	  && INTEGERP (XCAR (XCDR (pos))))
! 	{
! 	  Lisp_Object map, object;
  
! 	  object = Fnth (make_number(4), pos);
  
! 	  if (CONSP (object))
! 	    map = Fget_char_property (XCDR (object), Qkeymap, XCAR (object));
! 	  else
! 	    map = Fget_char_property (XCAR (XCDR (pos)), Qkeymap,
! 				      Fwindow_buffer (XCAR (pos)));
  
! 	  if (!NILP (Fkeymapp (map)))
  	    {
! 	      value = Flookup_key (map, key, accept_default);
! 	      if (! NILP (value) && !INTEGERP (value))
! 		goto done;
  	    }
  	}
      }
! #endif /* HAVE_MOUSE  */
! 
!   if (!NILP (current_kboard->Voverriding_terminal_local_map))
      {
        value = Flookup_key (current_kboard->Voverriding_terminal_local_map,
  			   key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
  	goto done;
      }
!   else if (!NILP (Voverriding_local_map))
      {
        value = Flookup_key (Voverriding_local_map, key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
--- 1568,1660 ----
  Like the normal command loop, `key-binding' will remap the command
  resulting from looking up KEY by looking up the command in the
  current keymaps.  However, if the optional third argument NO-REMAP
! is non-nil, `key-binding' returns the unmapped command.
! 
! If KEY is a key sequence initiated with the mouse, the used keymaps
! will depend on the clicked mouse position with regard to the buffer
! and possible local keymaps on strings.  If LOCATION is non-nil, it
! will used in place of KEY for determining mouse-dependent keymaps, but
! it will still be KEY that is looked up.
!   */)
! (key, accept_default, no_remap, location)
!   Lisp_Object key, accept_default, no_remap, location;
  {
    Lisp_Object *maps, value;
    int nmaps, i;
!   struct gcpro gcpro1, gcpro2;
!   int count = SPECPDL_INDEX ();
! 
!   GCPRO2 (key, location);
  
!   if (NILP (location))
!     location = key;
  
!   if (VECTORP(location) && ASIZE(location) > 0)
      {
!       if (SYMBOLP (AREF(location,0)) && ASIZE(location) > 1)
! 	location = AREF(location,1);
!       else
! 	location = AREF(location,0);
!     }
  
!   /* We are not interested in locations without event data */
  
!   if (!EVENT_HAS_PARAMETERS (location))
!     location = Qnil;
  
!   /* Key sequences beginning with mouse clicks
!      are read using the keymaps of the buffer clicked on, not
!      the current buffer.  So we may have to switch the buffer
!      here. */
! 
!   if (! NILP (location))
!     {
!       Lisp_Object kind;
!       Lisp_Object string;
!       
!       kind = EVENT_HEAD_KIND (EVENT_HEAD (location));
!       if (EQ (kind, Qmouse_click))
! 	{
! 	  Lisp_Object window, posn;
! 	  
! 	  window = POSN_WINDOW (EVENT_START (location));
! 	  posn   = POSN_POSN (EVENT_START (location));
! 	  
! 	  /* Key sequences beginning with mouse clicks are
! 	     read using the keymaps in the buffer clicked on,
! 	     not the current buffer.  If we're at the
! 	     beginning of a key sequence, switch buffers.  */
! 	  if (WINDOWP (window)
! 	      && BUFFERP (XWINDOW (window)->buffer)
! 	      && XBUFFER (XWINDOW (window)->buffer) != current_buffer)
  	    {
! 	      /* Arrange to go back to the original buffer once we're
! 		 done reading the key sequence.  Note that we can't
! 		 use save_excursion_{save,restore} here, because they
! 		 save point as well as the current buffer; we don't
! 		 want to save point, because redisplay may change it,
! 		 to accommodate a Fset_window_start or something.
! 	      */
! 	      
! 	      record_unwind_protect (Fset_buffer, Fcurrent_buffer ());
! 	      
! 	      if (! FRAME_LIVE_P (XFRAME (selected_frame)))
! 		Fkill_emacs (Qnil);
! 	      
! 	      set_buffer_internal (XBUFFER (XWINDOW (window)->buffer));
! 	      
  	    }
  	}
      }
!   
!   if (! NILP (current_kboard->Voverriding_terminal_local_map))
      {
        value = Flookup_key (current_kboard->Voverriding_terminal_local_map,
  			   key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
  	goto done;
      }
!   else if (! NILP (Voverriding_local_map))
      {
        value = Flookup_key (Voverriding_local_map, key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
***************
*** 1622,1633 ****
      }
    else
      {
!       Lisp_Object local;
  
!       local = get_local_map (PT, current_buffer, Qkeymap);
!       if (! NILP (local))
  	{
! 	  value = Flookup_key (local, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
--- 1662,1741 ----
      }
    else
      {
!       Lisp_Object keymap, local_map;
  
!       local_map = get_local_map (PT, current_buffer, Qlocal_map); 
!       keymap = get_local_map (PT, current_buffer, Qkeymap); 
! 
!       if (! NILP(location))
  	{
! 	  Lisp_Object kind;
! 	  Lisp_Object string;
! 
! 	  kind = EVENT_HEAD_KIND (EVENT_HEAD (location));
! 	  if (EQ (kind, Qmouse_click))
! 	    {
! 	      Lisp_Object window, posn;
! 
! 	      window = POSN_WINDOW      (EVENT_START (location));
! 	      posn   = POSN_POSN (EVENT_START (location));
! 
! 	      /* For a mouse click, get the local text-property keymap
! 		 of the place clicked on, rather than point.  */
! 
! 	      if (CONSP (XCDR (location)))
! 		{
! 		  Lisp_Object start, pos;
! 
! 		  start = EVENT_START (location);
! 
! 		  if (CONSP (start) && POSN_INBUFFER_P (start))
! 		    {
! 		      pos = POSN_BUFFER_POSN (start);
! 		      if (INTEGERP (pos)
! 			  && XINT (pos) >= BEG && XINT (pos) <= Z)
! 			{
! 			  local_map =
! 			    get_local_map (XINT (pos),
! 					   current_buffer, Qlocal_map);
! 
! 			  keymap = get_local_map (XINT (pos),
! 						  current_buffer, Qkeymap);
! 			}
! 		    }
! 		}
! 
! 	      /* If on a mode line string with a local keymap,
! 		 or for a click on a string, i.e. overlay string or a
! 		 string displayed via the `display' property,
! 		 consider `local-map' and `keymap' properties of
! 		 that string.  */
! 
! 	      if (string = POSN_STRING (EVENT_START (location)),
! 		  (CONSP (string) && STRINGP (XCAR (string))))
! 		{
! 		  Lisp_Object pos, map;
! 		  
! 		  pos = XCDR (string);
! 		  string = XCAR (string);
! 		  if (XINT (pos) >= 0
! 		      && XINT (pos) < SCHARS (string))
! 		    {
! 		      map = Fget_text_property (pos, Qlocal_map, string);
! 		      if (!NILP (map))
! 			local_map = map;
! 		      map = Fget_text_property (pos, Qkeymap, string);
! 		      if (!NILP (map))
! 			keymap = map;
! 		    }
! 		}
! 	      
! 	    }
! 	}
! 
!       if (! NILP (keymap))
! 	{
! 	  value = Flookup_key (keymap, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
***************
*** 1644,1653 ****
  	      goto done;
  	  }
  
!       local = get_local_map (PT, current_buffer, Qlocal_map);
!       if (! NILP (local))
  	{
! 	  value = Flookup_key (local, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
--- 1752,1760 ----
  	      goto done;
  	  }
  
!       if (! NILP (local_map))
  	{
! 	  value = Flookup_key (local_map, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
***************
*** 1656,1661 ****
--- 1763,1770 ----
    value = Flookup_key (current_global_map, key, accept_default);
  
   done:
+   unbind_to (count, Qnil);
+ 
    UNGCPRO;
    if (NILP (value) || INTEGERP (value))
      return Qnil;
Index: src/keymap.h
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.h,v
retrieving revision 1.14
diff -c -r1.14 keymap.h
*** src/keymap.h	6 Feb 2006 15:23:21 -0000	1.14
--- src/keymap.h	12 Sep 2006 15:08:18 -0000
***************
*** 30,36 ****
  EXFUN (Fdefine_key, 3);
  EXFUN (Flookup_key, 3);
  EXFUN (Fcommand_remapping, 1);
! EXFUN (Fkey_binding, 3);
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);
--- 30,36 ----
  EXFUN (Fdefine_key, 3);
  EXFUN (Flookup_key, 3);
  EXFUN (Fcommand_remapping, 1);
! EXFUN (Fkey_binding, 4);
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* PCL-CVS's diff and marks (was: local keymap patch for key-binding)
  2006-09-12 15:21                     ` David Kastrup
@ 2006-09-12 15:39                       ` Stefan Monnier
  2006-09-12 15:42                         ` PCL-CVS's diff and marks David Kastrup
  2006-09-12 21:45                       ` local keymap patch for key-binding Richard Stallman
  2006-09-13 15:10                       ` Richard Stallman
  2 siblings, 1 reply; 55+ messages in thread
From: Stefan Monnier @ 2006-09-12 15:39 UTC (permalink / raw)
  Cc: cyd, emacs-devel, rms, storm

> (incidentally: why doesn't it work to use
> PCL-CVS to do a multi-file diff? It ignores the marked files, just
> working on the current line).

C-h v cvs-diff-ignore-marks RET


        Stefan

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: PCL-CVS's diff and marks
  2006-09-12 15:39                       ` PCL-CVS's diff and marks (was: local keymap patch for key-binding) Stefan Monnier
@ 2006-09-12 15:42                         ` David Kastrup
  2006-09-12 15:54                           ` Stefan Monnier
  0 siblings, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-12 15:42 UTC (permalink / raw)
  Cc: emacs-devel

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

>> (incidentally: why doesn't it work to use
>> PCL-CVS to do a multi-file diff? It ignores the marked files, just
>> working on the current line).
>
> C-h v cvs-diff-ignore-marks RET

Which is obsolete and talks about an inversion of meaning.  However,
even if I precede the command with ! (which is supposed to override
the non-markiness) will the marks get ignored.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: PCL-CVS's diff and marks
  2006-09-12 15:42                         ` PCL-CVS's diff and marks David Kastrup
@ 2006-09-12 15:54                           ` Stefan Monnier
  0 siblings, 0 replies; 55+ messages in thread
From: Stefan Monnier @ 2006-09-12 15:54 UTC (permalink / raw)
  Cc: emacs-devel

>>> (incidentally: why doesn't it work to use
>>> PCL-CVS to do a multi-file diff? It ignores the marked files, just
>>> working on the current line).
>> 
>> C-h v cvs-diff-ignore-marks RET

> Which is obsolete and talks about an inversion of meaning.  However,
> even if I precede the command with ! (which is supposed to override
> the non-markiness) will the marks get ignored.

No, ! only ignores the state (e.g. allows to apply commit to an up-to-date
file).  To change the behavior wrt marks, you want to use T.


        Stefan

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-12 15:21                     ` David Kastrup
  2006-09-12 15:39                       ` PCL-CVS's diff and marks (was: local keymap patch for key-binding) Stefan Monnier
@ 2006-09-12 21:45                       ` Richard Stallman
  2006-09-12 22:23                         ` David Kastrup
  2006-09-13 15:10                       ` Richard Stallman
  2 siblings, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-12 21:45 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

    > If you thing it reasonable that key-binding should also react to
    > bona-fide events _not_ wrapped in an array, I can do this easily
    > enough in the change I am working on.

Please don't make that change.  key-binding is supposed to look up a
sequence of events; it expects that sequence to be in a vector or
string.  There is nothing wrong with that, and no need to change it.

I think we simply had a misunderstanding about what it means to say
that key-binding can "look up events".  In my way of looking at
things, you give key-binding a vector of events, and what it does is
"look up events."

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-12 21:45                       ` local keymap patch for key-binding Richard Stallman
@ 2006-09-12 22:23                         ` David Kastrup
  2006-09-12 23:44                           ` David Kastrup
  2006-09-13 19:24                           ` Richard Stallman
  0 siblings, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-12 22:23 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

Richard Stallman <rms@gnu.org> writes:

>     > If you thing it reasonable that key-binding should also react to
>     > bona-fide events _not_ wrapped in an array, I can do this easily
>     > enough in the change I am working on.
>
> Please don't make that change.  key-binding is supposed to look up a
> sequence of events; it expects that sequence to be in a vector or
> string.  There is nothing wrong with that, and no need to change it.
>
> I think we simply had a misunderstanding about what it means to say
> that key-binding can "look up events".  In my way of looking at
> things, you give key-binding a vector of events, and what it does is
> "look up events."

Ok, I'll rework my patch.

The change I proposed concerning read-key-sequence was for the sake of
letting both keymap and local-map properties (if they are specified in
a string) be heeded.  The current code only heeds the first of two.

That puts the behavior out of line with the documentation without good
reason.  While this case will probably occur rarely, I see no reason
not to do the correct thing as documented.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-12 22:23                         ` David Kastrup
@ 2006-09-12 23:44                           ` David Kastrup
  2006-09-13  7:45                             ` Kim F. Storm
  2006-09-13 19:24                           ` Richard Stallman
  1 sibling, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-12 23:44 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

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

David Kastrup <dak@gnu.org> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>>     > If you thing it reasonable that key-binding should also react to
>>     > bona-fide events _not_ wrapped in an array, I can do this easily
>>     > enough in the change I am working on.
>>
>> Please don't make that change.  key-binding is supposed to look up a
>> sequence of events; it expects that sequence to be in a vector or
>> string.  There is nothing wrong with that, and no need to change it.
>>
>> I think we simply had a misunderstanding about what it means to say
>> that key-binding can "look up events".  In my way of looking at
>> things, you give key-binding a vector of events, and what it does is
>> "look up events."
>
> Ok, I'll rework my patch.

Ok, here is the complete patch.  I also added an optional argument to
`command-remapping' (this was required to let `key-binding' do the
right thing in the case of mouse events).

I removed the `kill-emacs' call if the current frame not live: I can't
think of it making much sense in `key-binding'.

I also added a changelog entry and tested this with a number of
events.  Looks clean to me.

I would like to check this in soon so that one can make the mouse
remapping stuff use it.  That should make the `follow-link' property
work the same as any remapping.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 9356 bytes --]

Index: keymap.c
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.c,v
retrieving revision 1.333
diff -u -r1.333 keymap.c
--- keymap.c	11 Sep 2006 13:03:40 -0000	1.333
+++ keymap.c	12 Sep 2006 23:19:16 -0000
@@ -28,6 +28,7 @@
 #include "buffer.h"
 #include "charset.h"
 #include "keyboard.h"
+#include "window.h"
 #include "termhooks.h"
 #include "blockinput.h"
 #include "puresize.h"
@@ -1216,17 +1217,20 @@
 
 /* This function may GC (it calls Fkey_binding).  */
 
-DEFUN ("command-remapping", Fcommand_remapping, Scommand_remapping, 1, 1, 0,
+DEFUN ("command-remapping", Fcommand_remapping, Scommand_remapping, 1, 2, 0,
        doc: /* Return the remapping for command COMMAND in current keymaps.
-Returns nil if COMMAND is not remapped (or not a symbol).  */)
-     (command)
-     Lisp_Object command;
+Returns nil if COMMAND is not remapped (or not a symbol).
+
+If optional argument LOCATION is a mouse event based key sequence,
+the remapping occurs in the keymaps corresponding to the click. */)
+(command, location)
+Lisp_Object command, location;
 {
   if (!SYMBOLP (command))
     return Qnil;
 
   ASET (command_remapping_vector, 1, command);
-  return Fkey_binding (command_remapping_vector, Qnil, Qt);
+  return Fkey_binding (command_remapping_vector, Qnil, Qt, location);
 }
 
 /* Value is number if KEY is too long; nil if valid but has no definition. */
@@ -1552,7 +1556,7 @@
 
 /* GC is possible in this function if it autoloads a keymap.  */
 
-DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 3, 0,
+DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 4, 0,
        doc: /* Return the binding for command KEY in current keymaps.
 KEY is a string or vector, a sequence of keystrokes.
 The binding is probably a symbol with a function definition.
@@ -1566,55 +1570,94 @@
 Like the normal command loop, `key-binding' will remap the command
 resulting from looking up KEY by looking up the command in the
 current keymaps.  However, if the optional third argument NO-REMAP
-is non-nil, `key-binding' returns the unmapped command.  */)
-     (key, accept_default, no_remap)
-     Lisp_Object key, accept_default, no_remap;
+is non-nil, `key-binding' returns the unmapped command.
+
+If KEY is a key sequence initiated with the mouse, the used keymaps
+will depend on the clicked mouse position with regard to the buffer
+and possible local keymaps on strings.  If LOCATION is non-nil, it
+will used in place of KEY for determining mouse-dependent keymaps, but
+it will still be KEY that is looked up.
+  */)
+(key, accept_default, no_remap, location)
+  Lisp_Object key, accept_default, no_remap, location;
 {
-  Lisp_Object *maps, value;
+  Lisp_Object *maps, value, event;
   int nmaps, i;
-  struct gcpro gcpro1;
+  struct gcpro gcpro1, gcpro2;
+  int count = SPECPDL_INDEX ();
+
+  GCPRO2 (key, location);
 
-  GCPRO1 (key);
+  if (NILP (location))
+    location = key;
 
-#ifdef HAVE_MOUSE
-  if (VECTORP (key) && ASIZE (key) > 0)
+  if (VECTORP(location) && ASIZE(location) > 0)
     {
-      Lisp_Object ev, pos;
-      if ((ev = AREF (key, 0), CONSP (ev))
-	  && SYMBOLP (XCAR (ev))
-	  && CONSP (XCDR (ev))
-	  && (pos = XCAR (XCDR (ev)), CONSP (pos))
-	  && XINT (Flength (pos)) == 10
-	  && INTEGERP (XCAR (XCDR (pos))))
-	{
-	  Lisp_Object map, object;
+      /* mouse events may have a symbolic prefix indicating the
+	 scrollbar or mode line */
+      if (SYMBOLP (AREF(location,0)) && ASIZE(location) > 1)
+	event = AREF(location,1);
+      else
+	event = AREF(location,0);
 
-	  object = Fnth (make_number(4), pos);
+      /* We are not interested in locations without event data */
 
-	  if (CONSP (object))
-	    map = Fget_char_property (XCDR (object), Qkeymap, XCAR (object));
-	  else
-	    map = Fget_char_property (XCAR (XCDR (pos)), Qkeymap,
-				      Fwindow_buffer (XCAR (pos)));
+      if (! EVENT_HAS_PARAMETERS (event))
+	event = Qnil;
+    }
+  else
+    event = Qnil;
 
-	  if (!NILP (Fkeymapp (map)))
+  /* Key sequences beginning with mouse clicks
+     are read using the keymaps of the buffer clicked on, not
+     the current buffer.  So we may have to switch the buffer
+     here. */
+
+  if (! NILP (event))
+    {
+      Lisp_Object kind;
+      Lisp_Object string;
+      
+      kind = EVENT_HEAD_KIND (EVENT_HEAD (event));
+      if (EQ (kind, Qmouse_click))
+	{
+	  Lisp_Object window, posn;
+	  
+	  window = POSN_WINDOW (EVENT_START (event));
+	  posn   = POSN_POSN (EVENT_START (event));
+	  
+	  /* Key sequences beginning with mouse clicks are
+	     read using the keymaps in the buffer clicked on,
+	     not the current buffer.  If we're at the
+	     beginning of a key sequence, switch buffers.  */
+	  if (WINDOWP (window)
+	      && BUFFERP (XWINDOW (window)->buffer)
+	      && XBUFFER (XWINDOW (window)->buffer) != current_buffer)
 	    {
-	      value = Flookup_key (map, key, accept_default);
-	      if (! NILP (value) && !INTEGERP (value))
-		goto done;
+	      /* Arrange to go back to the original buffer once we're
+		 done reading the key sequence.  Note that we can't
+		 use save_excursion_{save,restore} here, because they
+		 save point as well as the current buffer; we don't
+		 want to save point, because redisplay may change it,
+		 to accommodate a Fset_window_start or something.
+	      */
+	      
+	      record_unwind_protect (Fset_buffer, Fcurrent_buffer ());
+
+	      set_buffer_internal (XBUFFER (XWINDOW (window)->buffer));
+	      
 	    }
 	}
     }
-#endif /* HAVE_MOUSE  */
-
-  if (!NILP (current_kboard->Voverriding_terminal_local_map))
+  
+  if (! NILP (current_kboard->Voverriding_terminal_local_map))
     {
       value = Flookup_key (current_kboard->Voverriding_terminal_local_map,
 			   key, accept_default);
       if (! NILP (value) && !INTEGERP (value))
 	goto done;
     }
-  else if (!NILP (Voverriding_local_map))
+  else if (! NILP (Voverriding_local_map))
     {
       value = Flookup_key (Voverriding_local_map, key, accept_default);
       if (! NILP (value) && !INTEGERP (value))
@@ -1622,12 +1665,80 @@
     }
   else
     {
-      Lisp_Object local;
+      Lisp_Object keymap, local_map;
 
-      local = get_local_map (PT, current_buffer, Qkeymap);
-      if (! NILP (local))
+      local_map = get_local_map (PT, current_buffer, Qlocal_map); 
+      keymap = get_local_map (PT, current_buffer, Qkeymap); 
+
+      if (! NILP(event))
+	{
+	  Lisp_Object kind;
+	  Lisp_Object string;
+
+	  kind = EVENT_HEAD_KIND (EVENT_HEAD (event));
+	  if (EQ (kind, Qmouse_click))
+	    {
+	      Lisp_Object window, posn;
+
+	      window = POSN_WINDOW      (EVENT_START (event));
+	      posn   = POSN_POSN (EVENT_START (event));
+
+	      /* For a mouse click, get the local text-property keymap
+		 of the place clicked on, rather than point.  */
+
+	      if (CONSP (XCDR (event)))
+		{
+		  Lisp_Object start, pos;
+
+		  start = EVENT_START (event);
+
+		  if (CONSP (start) && POSN_INBUFFER_P (start))
+		    {
+		      pos = POSN_BUFFER_POSN (start);
+		      if (INTEGERP (pos)
+			  && XINT (pos) >= BEG && XINT (pos) <= Z)
+			{
+			  local_map =
+			    get_local_map (XINT (pos),
+					   current_buffer, Qlocal_map);
+
+			  keymap = get_local_map (XINT (pos),
+						  current_buffer, Qkeymap);
+			}
+		    }
+		}
+
+	      /* If on a mode line string with a local keymap,
+		 or for a click on a string, i.e. overlay string or a
+		 string displayed via the `display' property,
+		 consider `local-map' and `keymap' properties of
+		 that string.  */
+
+	      if (string = POSN_STRING (EVENT_START (event)),
+		  (CONSP (string) && STRINGP (XCAR (string))))
+		{
+		  Lisp_Object pos, map;
+		  
+		  pos = XCDR (string);
+		  string = XCAR (string);
+		  if (XINT (pos) >= 0
+		      && XINT (pos) < SCHARS (string))
+		    {
+		      map = Fget_text_property (pos, Qlocal_map, string);
+		      if (!NILP (map))
+			local_map = map;
+		      map = Fget_text_property (pos, Qkeymap, string);
+		      if (!NILP (map))
+			keymap = map;
+		    }
+		}
+	      
+	    }
+	}
+
+      if (! NILP (keymap))
 	{
-	  value = Flookup_key (local, key, accept_default);
+	  value = Flookup_key (keymap, key, accept_default);
 	  if (! NILP (value) && !INTEGERP (value))
 	    goto done;
 	}
@@ -1644,10 +1755,9 @@
 	      goto done;
 	  }
 
-      local = get_local_map (PT, current_buffer, Qlocal_map);
-      if (! NILP (local))
+      if (! NILP (local_map))
 	{
-	  value = Flookup_key (local, key, accept_default);
+	  value = Flookup_key (local_map, key, accept_default);
 	  if (! NILP (value) && !INTEGERP (value))
 	    goto done;
 	}
@@ -1656,6 +1766,8 @@
   value = Flookup_key (current_global_map, key, accept_default);
 
  done:
+  unbind_to (count, Qnil);
+
   UNGCPRO;
   if (NILP (value) || INTEGERP (value))
     return Qnil;
@@ -1666,7 +1778,7 @@
   if (NILP (no_remap) && SYMBOLP (value))
     {
       Lisp_Object value1;
-      if (value1 = Fcommand_remapping (value), !NILP (value1))
+      if (value1 = Fcommand_remapping (value, location), !NILP (value1))
 	value = value1;
     }
 
@@ -2467,7 +2579,7 @@
   if (NILP (no_remap) && SYMBOLP (definition))
     {
       Lisp_Object tem;
-      if (tem = Fcommand_remapping (definition), !NILP (tem))
+      if (tem = Fcommand_remapping (definition, Qnil), !NILP (tem))
 	return Qnil;
     }
 

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-12 23:44                           ` David Kastrup
@ 2006-09-13  7:45                             ` Kim F. Storm
  2006-09-13  8:11                               ` David Kastrup
  0 siblings, 1 reply; 55+ messages in thread
From: Kim F. Storm @ 2006-09-13  7:45 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> Ok, here is the complete patch.  

Well, you forgot to include patches for keyboard.c, lisp.h and the
ChangeLog.

>                                  I also added an optional argument to
> `command-remapping' (this was required to let `key-binding' do the
> right thing in the case of mouse events).

Good.

> I also added a changelog entry and tested this with a number of
> events.  Looks clean to me.

Looks clean to me too.

> I would like to check this in soon so that one can make the mouse
> remapping stuff use it.  That should make the `follow-link' property
> work the same as any remapping.

What changes are needed to do that (patches) ?

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13  7:45                             ` Kim F. Storm
@ 2006-09-13  8:11                               ` David Kastrup
  2006-09-13 11:05                                 ` Kim F. Storm
  2006-09-13 19:25                                 ` Richard Stallman
  0 siblings, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-13  8:11 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

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

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Ok, here is the complete patch.  
>
> Well, you forgot to include patches for keyboard.c, lisp.h and the
> ChangeLog.

I don't get it.  I used Stefan's instructions, and I _checked_ that
the diff covered all files.  Sigh.  But I can't deny that this info is
missing in my mail.

[Checking, rechecking, retrying, previewing the mail]

Oh wow.  I included "*vc-diff*" instead of "*cvs-diff*".  Who would've
thunk?

Sorry for the confusion.  I suppose that the included diff is the
relevant version, since I likely used it to cook up the ChangeLog
entry.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 15624 bytes --]

Index: src/keymap.h
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.h,v
retrieving revision 1.14
diff -u -r1.14 keymap.h
*** src/keymap.h	6 Feb 2006 15:23:21 -0000	1.14
--- src/keymap.h	13 Sep 2006 08:02:42 -0000
***************
*** 29,36 ****
  EXFUN (Fkeymap_prompt, 1);
  EXFUN (Fdefine_key, 3);
  EXFUN (Flookup_key, 3);
! EXFUN (Fcommand_remapping, 1);
! EXFUN (Fkey_binding, 3);
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);
--- 29,36 ----
  EXFUN (Fkeymap_prompt, 1);
  EXFUN (Fdefine_key, 3);
  EXFUN (Flookup_key, 3);
! EXFUN (Fcommand_remapping, 2);
! EXFUN (Fkey_binding, 4);
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);
Index: src/keymap.c
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.c,v
retrieving revision 1.333
diff -u -r1.333 keymap.c
*** src/keymap.c	11 Sep 2006 13:03:40 -0000	1.333
--- src/keymap.c	13 Sep 2006 08:02:44 -0000
***************
*** 28,33 ****
--- 28,34 ----
  #include "buffer.h"
  #include "charset.h"
  #include "keyboard.h"
+ #include "window.h"
  #include "termhooks.h"
  #include "blockinput.h"
  #include "puresize.h"
***************
*** 1216,1232 ****
  
  /* This function may GC (it calls Fkey_binding).  */
  
! DEFUN ("command-remapping", Fcommand_remapping, Scommand_remapping, 1, 1, 0,
         doc: /* Return the remapping for command COMMAND in current keymaps.
! Returns nil if COMMAND is not remapped (or not a symbol).  */)
!      (command)
!      Lisp_Object command;
  {
    if (!SYMBOLP (command))
      return Qnil;
  
    ASET (command_remapping_vector, 1, command);
!   return Fkey_binding (command_remapping_vector, Qnil, Qt);
  }
  
  /* Value is number if KEY is too long; nil if valid but has no definition. */
--- 1217,1236 ----
  
  /* This function may GC (it calls Fkey_binding).  */
  
! DEFUN ("command-remapping", Fcommand_remapping, Scommand_remapping, 1, 2, 0,
         doc: /* Return the remapping for command COMMAND in current keymaps.
! Returns nil if COMMAND is not remapped (or not a symbol).
! 
! If optional argument LOCATION is a mouse event based key sequence,
! the remapping occurs in the keymaps corresponding to the click. */)
!      (command, location)
!      Lisp_Object command, location;
  {
    if (!SYMBOLP (command))
      return Qnil;
  
    ASET (command_remapping_vector, 1, command);
!   return Fkey_binding (command_remapping_vector, Qnil, Qt, location);
  }
  
  /* Value is number if KEY is too long; nil if valid but has no definition. */
***************
*** 1552,1558 ****
  
  /* GC is possible in this function if it autoloads a keymap.  */
  
! DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 3, 0,
         doc: /* Return the binding for command KEY in current keymaps.
  KEY is a string or vector, a sequence of keystrokes.
  The binding is probably a symbol with a function definition.
--- 1556,1562 ----
  
  /* GC is possible in this function if it autoloads a keymap.  */
  
! DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 4, 0,
         doc: /* Return the binding for command KEY in current keymaps.
  KEY is a string or vector, a sequence of keystrokes.
  The binding is probably a symbol with a function definition.
***************
*** 1566,1620 ****
  Like the normal command loop, `key-binding' will remap the command
  resulting from looking up KEY by looking up the command in the
  current keymaps.  However, if the optional third argument NO-REMAP
! is non-nil, `key-binding' returns the unmapped command.  */)
!      (key, accept_default, no_remap)
!      Lisp_Object key, accept_default, no_remap;
  {
!   Lisp_Object *maps, value;
    int nmaps, i;
!   struct gcpro gcpro1;
  
!   GCPRO1 (key);
  
! #ifdef HAVE_MOUSE
!   if (VECTORP (key) && ASIZE (key) > 0)
      {
!       Lisp_Object ev, pos;
!       if ((ev = AREF (key, 0), CONSP (ev))
! 	  && SYMBOLP (XCAR (ev))
! 	  && CONSP (XCDR (ev))
! 	  && (pos = XCAR (XCDR (ev)), CONSP (pos))
! 	  && XINT (Flength (pos)) == 10
! 	  && INTEGERP (XCAR (XCDR (pos))))
! 	{
! 	  Lisp_Object map, object;
  
! 	  object = Fnth (make_number(4), pos);
  
! 	  if (CONSP (object))
! 	    map = Fget_char_property (XCDR (object), Qkeymap, XCAR (object));
! 	  else
! 	    map = Fget_char_property (XCAR (XCDR (pos)), Qkeymap,
! 				      Fwindow_buffer (XCAR (pos)));
  
! 	  if (!NILP (Fkeymapp (map)))
  	    {
! 	      value = Flookup_key (map, key, accept_default);
! 	      if (! NILP (value) && !INTEGERP (value))
! 		goto done;
  	    }
  	}
      }
! #endif /* HAVE_MOUSE  */
! 
!   if (!NILP (current_kboard->Voverriding_terminal_local_map))
      {
        value = Flookup_key (current_kboard->Voverriding_terminal_local_map,
  			   key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
  	goto done;
      }
!   else if (!NILP (Voverriding_local_map))
      {
        value = Flookup_key (Voverriding_local_map, key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
--- 1570,1663 ----
  Like the normal command loop, `key-binding' will remap the command
  resulting from looking up KEY by looking up the command in the
  current keymaps.  However, if the optional third argument NO-REMAP
! is non-nil, `key-binding' returns the unmapped command.
! 
! If KEY is a key sequence initiated with the mouse, the used keymaps
! will depend on the clicked mouse position with regard to the buffer
! and possible local keymaps on strings.  If LOCATION is non-nil, it
! will used in place of KEY for determining mouse-dependent keymaps, but
! it will still be KEY that is looked up.
!   */)
! (key, accept_default, no_remap, location)
!   Lisp_Object key, accept_default, no_remap, location;
  {
!   Lisp_Object *maps, value, event;
    int nmaps, i;
!   struct gcpro gcpro1, gcpro2;
!   int count = SPECPDL_INDEX ();
! 
!   GCPRO2 (key, location);
  
!   if (NILP (location))
!     location = key;
  
!   if (VECTORP(location) && ASIZE(location) > 0)
      {
!       /* mouse events may have a symbolic prefix indicating the
! 	 scrollbar or mode line */
!       if (SYMBOLP (AREF(location,0)) && ASIZE(location) > 1)
! 	event = AREF(location,1);
!       else
! 	event = AREF(location,0);
  
!       /* We are not interested in locations without event data */
  
!       if (! EVENT_HAS_PARAMETERS (event))
! 	event = Qnil;
!     }
!   else
!     event = Qnil;
  
!   /* Key sequences beginning with mouse clicks
!      are read using the keymaps of the buffer clicked on, not
!      the current buffer.  So we may have to switch the buffer
!      here. */
! 
!   if (! NILP (event))
!     {
!       Lisp_Object kind;
!       Lisp_Object string;
!       
!       kind = EVENT_HEAD_KIND (EVENT_HEAD (event));
!       if (EQ (kind, Qmouse_click))
! 	{
! 	  Lisp_Object window, posn;
! 	  
! 	  window = POSN_WINDOW (EVENT_START (event));
! 	  posn   = POSN_POSN (EVENT_START (event));
! 	  
! 	  /* Key sequences beginning with mouse clicks are
! 	     read using the keymaps in the buffer clicked on,
! 	     not the current buffer.  If we're at the
! 	     beginning of a key sequence, switch buffers.  */
! 	  if (WINDOWP (window)
! 	      && BUFFERP (XWINDOW (window)->buffer)
! 	      && XBUFFER (XWINDOW (window)->buffer) != current_buffer)
  	    {
! 	      /* Arrange to go back to the original buffer once we're
! 		 done reading the key sequence.  Note that we can't
! 		 use save_excursion_{save,restore} here, because they
! 		 save point as well as the current buffer; we don't
! 		 want to save point, because redisplay may change it,
! 		 to accommodate a Fset_window_start or something.
! 	      */
! 	      
! 	      record_unwind_protect (Fset_buffer, Fcurrent_buffer ());
! 
! 	      set_buffer_internal (XBUFFER (XWINDOW (window)->buffer));
! 	      
  	    }
  	}
      }
!   
!   if (! NILP (current_kboard->Voverriding_terminal_local_map))
      {
        value = Flookup_key (current_kboard->Voverriding_terminal_local_map,
  			   key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
  	goto done;
      }
!   else if (! NILP (Voverriding_local_map))
      {
        value = Flookup_key (Voverriding_local_map, key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
***************
*** 1622,1633 ****
      }
    else
      {
!       Lisp_Object local;
  
!       local = get_local_map (PT, current_buffer, Qkeymap);
!       if (! NILP (local))
  	{
! 	  value = Flookup_key (local, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
--- 1665,1744 ----
      }
    else
      {
!       Lisp_Object keymap, local_map;
  
!       local_map = get_local_map (PT, current_buffer, Qlocal_map); 
!       keymap = get_local_map (PT, current_buffer, Qkeymap); 
! 
!       if (! NILP(event))
! 	{
! 	  Lisp_Object kind;
! 	  Lisp_Object string;
! 
! 	  kind = EVENT_HEAD_KIND (EVENT_HEAD (event));
! 	  if (EQ (kind, Qmouse_click))
! 	    {
! 	      Lisp_Object window, posn;
! 
! 	      window = POSN_WINDOW      (EVENT_START (event));
! 	      posn   = POSN_POSN (EVENT_START (event));
! 
! 	      /* For a mouse click, get the local text-property keymap
! 		 of the place clicked on, rather than point.  */
! 
! 	      if (CONSP (XCDR (event)))
! 		{
! 		  Lisp_Object start, pos;
! 
! 		  start = EVENT_START (event);
! 
! 		  if (CONSP (start) && POSN_INBUFFER_P (start))
! 		    {
! 		      pos = POSN_BUFFER_POSN (start);
! 		      if (INTEGERP (pos)
! 			  && XINT (pos) >= BEG && XINT (pos) <= Z)
! 			{
! 			  local_map =
! 			    get_local_map (XINT (pos),
! 					   current_buffer, Qlocal_map);
! 
! 			  keymap = get_local_map (XINT (pos),
! 						  current_buffer, Qkeymap);
! 			}
! 		    }
! 		}
! 
! 	      /* If on a mode line string with a local keymap,
! 		 or for a click on a string, i.e. overlay string or a
! 		 string displayed via the `display' property,
! 		 consider `local-map' and `keymap' properties of
! 		 that string.  */
! 
! 	      if (string = POSN_STRING (EVENT_START (event)),
! 		  (CONSP (string) && STRINGP (XCAR (string))))
! 		{
! 		  Lisp_Object pos, map;
! 		  
! 		  pos = XCDR (string);
! 		  string = XCAR (string);
! 		  if (XINT (pos) >= 0
! 		      && XINT (pos) < SCHARS (string))
! 		    {
! 		      map = Fget_text_property (pos, Qlocal_map, string);
! 		      if (!NILP (map))
! 			local_map = map;
! 		      map = Fget_text_property (pos, Qkeymap, string);
! 		      if (!NILP (map))
! 			keymap = map;
! 		    }
! 		}
! 	      
! 	    }
! 	}
! 
!       if (! NILP (keymap))
  	{
! 	  value = Flookup_key (keymap, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
***************
*** 1644,1653 ****
  	      goto done;
  	  }
  
!       local = get_local_map (PT, current_buffer, Qlocal_map);
!       if (! NILP (local))
  	{
! 	  value = Flookup_key (local, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
--- 1755,1763 ----
  	      goto done;
  	  }
  
!       if (! NILP (local_map))
  	{
! 	  value = Flookup_key (local_map, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
***************
*** 1656,1661 ****
--- 1766,1773 ----
    value = Flookup_key (current_global_map, key, accept_default);
  
   done:
+   unbind_to (count, Qnil);
+ 
    UNGCPRO;
    if (NILP (value) || INTEGERP (value))
      return Qnil;
***************
*** 1666,1672 ****
    if (NILP (no_remap) && SYMBOLP (value))
      {
        Lisp_Object value1;
!       if (value1 = Fcommand_remapping (value), !NILP (value1))
  	value = value1;
      }
  
--- 1778,1784 ----
    if (NILP (no_remap) && SYMBOLP (value))
      {
        Lisp_Object value1;
!       if (value1 = Fcommand_remapping (value, location), !NILP (value1))
  	value = value1;
      }
  
***************
*** 2467,2473 ****
    if (NILP (no_remap) && SYMBOLP (definition))
      {
        Lisp_Object tem;
!       if (tem = Fcommand_remapping (definition), !NILP (tem))
  	return Qnil;
      }
  
--- 2579,2585 ----
    if (NILP (no_remap) && SYMBOLP (definition))
      {
        Lisp_Object tem;
!       if (tem = Fcommand_remapping (definition, Qnil), !NILP (tem))
  	return Qnil;
      }
  
Index: src/keyboard.c
===================================================================
RCS file: /sources/emacs/emacs/src/keyboard.c,v
retrieving revision 1.875
diff -u -r1.875 keyboard.c
*** src/keyboard.c	10 Sep 2006 21:10:50 -0000	1.875
--- src/keyboard.c	13 Sep 2006 08:02:56 -0000
***************
*** 1674,1680 ****
        if (SYMBOLP (cmd))
  	{
  	  Lisp_Object cmd1;
! 	  if (cmd1 = Fcommand_remapping (cmd), !NILP (cmd1))
  	    cmd = cmd1;
  	}
  
--- 1674,1680 ----
        if (SYMBOLP (cmd))
  	{
  	  Lisp_Object cmd1;
! 	  if (cmd1 = Fcommand_remapping (cmd, Qnil), !NILP (cmd1))
  	    cmd = cmd1;
  	}
  
***************
*** 7516,7522 ****
        Lisp_Object prefix;
  
        if (!NILP (tem))
! 	tem = Fkey_binding (tem, Qnil, Qnil);
  
        prefix = AREF (item_properties, ITEM_PROPERTY_KEYEQ);
        if (CONSP (prefix))
--- 7516,7522 ----
        Lisp_Object prefix;
  
        if (!NILP (tem))
! 	tem = Fkey_binding (tem, Qnil, Qnil, Qnil);
  
        prefix = AREF (item_properties, ITEM_PROPERTY_KEYEQ);
        if (CONSP (prefix))
***************
*** 9125,9140 ****
  			  if (!EQ (map_here, orig_local_map))
  			    {
  			      orig_local_map = map_here;
! 			      keybuf[t] = key;
! 			      mock_input = t + 1;
! 
! 			      goto replay_sequence;
  			    }
  			  map_here = get_local_map (XINT (pos),
  						     current_buffer, Qkeymap);
  			  if (!EQ (map_here, orig_keymap))
  			    {
  			      orig_keymap = map_here;
  			      keybuf[t] = key;
  			      mock_input = t + 1;
  
--- 9125,9143 ----
  			  if (!EQ (map_here, orig_local_map))
  			    {
  			      orig_local_map = map_here;
! 			      ++localized_local_map;
  			    }
+ 
  			  map_here = get_local_map (XINT (pos),
  						     current_buffer, Qkeymap);
  			  if (!EQ (map_here, orig_keymap))
  			    {
  			      orig_keymap = map_here;
+ 			      ++localized_local_map;
+ 			    }
+ 
+ 			  if (localized_local_map > 1)
+ 			    {
  			      keybuf[t] = key;
  			      mock_input = t + 1;
  
Index: src/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/src/ChangeLog,v
retrieving revision 1.5293
diff -u -r1.5293 ChangeLog
*** src/ChangeLog	12 Sep 2006 16:47:50 -0000	1.5293
--- src/ChangeLog	13 Sep 2006 08:03:25 -0000
***************
*** 1,3 ****
--- 1,22 ----
+ 2006-09-13  David Kastrup  <dak@gnu.org>
+ 
+ 	* keymap.c: include "window.h".
+ 	(Fcommand_remapping): New optional LOCATION argument.
+ 	(Fkey_binding): New optional LOCATION argument.  Completely rework
+ 	handling of mouse clicks to get the same order of keymaps as
+ 	`read-key-sequence' and heed LOCATION.  Also temporarily switch
+ 	buffers to location of mouse click and back.
+ 
+ 	* keyboard.c (command_loop_1): Adjust call of `Fcommand_remapping'
+ 	for additional argument.
+ 	(parse_menu_item): Adjust call of `Fkey_binding' for additional
+ 	argument.
+ 	(read_key_sequence): If there are both `local-map' and `keymap'
+ 	text properties at some location, heed both.
+ 
+ 	* keymap.h: Declare additional optional arguments of
+ 	`Fcommand_remapping and `Fkey_binding'.
+ 
  2006-09-12  Stefan Monnier  <monnier@iro.umontreal.ca>
  
  	* textprop.c (Fnext_property_change, Fnext_single_property_change)

[-- Attachment #3: Type: text/plain, Size: 166 bytes --]


By the way: don't ask me why the diff is in reverse order.  Seems like
that is what T = does in a PCL-CVS buffer.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13  8:11                               ` David Kastrup
@ 2006-09-13 11:05                                 ` Kim F. Storm
  2006-09-13 11:38                                   ` David Kastrup
  2006-09-13 19:25                                 ` Richard Stallman
  1 sibling, 1 reply; 55+ messages in thread
From: Kim F. Storm @ 2006-09-13 11:05 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel


Your patch still looks good to me.

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 11:05                                 ` Kim F. Storm
@ 2006-09-13 11:38                                   ` David Kastrup
  2006-09-13 12:23                                     ` Kim F. Storm
  2006-09-13 19:25                                     ` Richard Stallman
  0 siblings, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-13 11:38 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

storm@cua.dk (Kim F. Storm) writes:

> Your patch still looks good to me.

Well, I could not witness any adverse effect after some testing, it
tracks the behavior of read-key-sequence closely.

There is one concern that I have: the type of the "location" argument.
This currently is a key sequence.  But it might make more sense to
turn this into the "location" data structure that `event-start' and/or
`event-end' return.  This would make it much easier to feed lookup-key
with data produced from, say, `posn-at-x-y'.

In order to get this data easier, it might make sense to define a
convenience function.

(defun key-event (key)
   "Return event from moused-base key sequence KEY."
   (and (vectorp key)
        (if (consp (aref key 0))
            (aref key 0)
          (and
            (symbolp (aref key 0))
            (> (length key) 1)
            (consp (aref key 1))
            (aref key 1)))))

Maybe even in C, as this function can be executed without consing.

I found that the current usage in the help echo fixup stuff does not
even have a key sequence to start from, just a mouse position.

So it would likely make more sense to rework the location argument
type to just be a mouse position.

That seems like the most basic unit for this argument that still makes
sense.

WDYT?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 11:38                                   ` David Kastrup
@ 2006-09-13 12:23                                     ` Kim F. Storm
  2006-09-13 12:32                                       ` David Kastrup
  2006-09-13 19:25                                     ` Richard Stallman
  1 sibling, 1 reply; 55+ messages in thread
From: Kim F. Storm @ 2006-09-13 12:23 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
>
>> Your patch still looks good to me.
>
> Well, I could not witness any adverse effect after some testing, it
> tracks the behavior of read-key-sequence closely.
>
> There is one concern that I have: the type of the "location" argument.
> This currently is a key sequence.  But it might make more sense to
> turn this into the "location" data structure that `event-start' and/or
> `event-end' return.  This would make it much easier to feed lookup-key
> with data produced from, say, `posn-at-x-y'.

I overlooked that part -- I actually thought it was such a location.

> In order to get this data easier, it might make sense to define a
> convenience function.
>
> (defun key-event (key)
>    "Return event from moused-base key sequence KEY."
>    (and (vectorp key)
>         (if (consp (aref key 0))
>             (aref key 0)
>           (and
>             (symbolp (aref key 0))
>             (> (length key) 1)
>             (consp (aref key 1))
>             (aref key 1)))))
>
> Maybe even in C, as this function can be executed without consing.

That is usually not the normal way to decide what should be in C...

> I found that the current usage in the help echo fixup stuff does not
> even have a key sequence to start from, just a mouse position.
>
> So it would likely make more sense to rework the location argument
> type to just be a mouse position.

in pixels or chars?

> That seems like the most basic unit for this argument that still makes
> sense.

True, but shouldn't it accept both types ... 

if you already have the full event, it seems wasteful to first convert
it to just a mouse position, pass that to key-binding and then use that
to derive what the original event was (eg. using posn-at-x-y).

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 12:23                                     ` Kim F. Storm
@ 2006-09-13 12:32                                       ` David Kastrup
  2006-09-13 13:45                                         ` Kim F. Storm
  0 siblings, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-13 12:32 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:
>
>> storm@cua.dk (Kim F. Storm) writes:
>>
>>> Your patch still looks good to me.
>>
>> Well, I could not witness any adverse effect after some testing, it
>> tracks the behavior of read-key-sequence closely.
>>
>> There is one concern that I have: the type of the "location" argument.
>> This currently is a key sequence.  But it might make more sense to
>> turn this into the "location" data structure that `event-start' and/or
>> `event-end' return.  This would make it much easier to feed lookup-key
>> with data produced from, say, `posn-at-x-y'.
>
> I overlooked that part -- I actually thought it was such a location.
>
>> In order to get this data easier, it might make sense to define a
>> convenience function.
>>
>> (defun key-event (key)
>>    "Return event from moused-base key sequence KEY."
>>    (and (vectorp key)
>>         (if (consp (aref key 0))
>>             (aref key 0)
>>           (and
>>             (symbolp (aref key 0))
>>             (> (length key) 1)
>>             (consp (aref key 1))
>>             (aref key 1)))))
>>
>> Maybe even in C, as this function can be executed without consing.
>
> That is usually not the normal way to decide what should be in C...
>
>> I found that the current usage in the help echo fixup stuff does
>> not even have a key sequence to start from, just a mouse position.
>>
>> So it would likely make more sense to rework the location argument
>> type to just be a mouse position.
>
> in pixels or chars?

In the form returned by `event-start' or `posn-at-x-y'.  One does not
want to look up the object relations in the display matrix again,
since those may have changed since the event.

>> That seems like the most basic unit for this argument that still
>> makes sense.
>
> True, but shouldn't it accept both types ...

I don't see that this will buy much over using

(let ((event (key-event key)))
  (key-binding whatever nil nil (and event (event-start event))))

once the convenience function key-event (or key-mouse-event or what
ever you want to call it) is present (note that (event-start nil) does
not return nil).

It also has the advantage that you can, if you want to, do a lookup in
the keymaps at the _end_ of a drag event.

> if you already have the full event, it seems wasteful to first
> convert it to just a mouse position, pass that to key-binding and
> then use that to derive what the original event was (eg. using
> posn-at-x-y).

No, I am afraid you misunderstood.  I mentioned `posn-at-x-y' only for
the case where indeed nothing but pixel coordinates exist.

I wanted to use a `location' argument as returned by `event-start'.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 12:32                                       ` David Kastrup
@ 2006-09-13 13:45                                         ` Kim F. Storm
  0 siblings, 0 replies; 55+ messages in thread
From: Kim F. Storm @ 2006-09-13 13:45 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

>>> So it would likely make more sense to rework the location argument
>>> type to just be a mouse position.
>>
>> in pixels or chars?
>
> In the form returned by `event-start' or `posn-at-x-y'.  One does not
> want to look up the object relations in the display matrix again,
> since those may have changed since the event.

Ok.  I tought you meant the format returned by `mouse-position' or
`mouse-pixel-position'.

> I wanted to use a `location' argument as returned by `event-start'.

Ok with me.

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-12 15:21                     ` David Kastrup
  2006-09-12 15:39                       ` PCL-CVS's diff and marks (was: local keymap patch for key-binding) Stefan Monnier
  2006-09-12 21:45                       ` local keymap patch for key-binding Richard Stallman
@ 2006-09-13 15:10                       ` Richard Stallman
  2006-09-13 15:25                         ` David Kastrup
  2 siblings, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-13 15:10 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

It looks like your patch does three things:

1. Makes Fkey_binding handle all the maps properly.
2. Implements a new arg LOCATION.
3. Changes something else in keyboard.c for motives that were
never explained.

Is there some reason why we need to add the LOCATION arg feature now?
I think we should not add it now if we don't really need to.

I already asked for an explanation of the purpose of the other change
in keyboard.c.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 15:10                       ` Richard Stallman
@ 2006-09-13 15:25                         ` David Kastrup
  2006-09-14  2:34                           ` Richard Stallman
  0 siblings, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-13 15:25 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> It looks like your patch does three things:
>
> 1. Makes Fkey_binding handle all the maps properly.
> 2. Implements a new arg LOCATION.
> 3. Changes something else in keyboard.c for motives that were
> never explained.
>
> Is there some reason why we need to add the LOCATION arg feature
> now?

Things like the follow-link property don't work consistently.  The
keymaps where follow-link entries have an effect are not the same than
the keymaps where the mouse events are stored.

This inconsistency is annoying and unpredictable to application
programmers.

> I think we should not add it now if we don't really need to.

One actually needs something like the location argument (I reworked
this into a POSITION argument in the next patch, with position being
something like `event-start' returns) in order to have
`command-remapping' work on the same maps when it is called by
`key-binding'.

> I already asked for an explanation of the purpose of the other
> change in keyboard.c.

Probably a miscommunication, or crossing mails.  I think I explained
it a few times already.  But again: if we have both a `keymap' and
`local-map' text property in the buffer, the resulting behavior is
inconsistent with the documentation of keymap lookup: one of those
properties then gets ignored.  The cost of making it consistent with
the documentation is negligible both in amounts of code (maybe 4
lines) as well as performance (probably sub-microsecond).  So I
consider it a bugfix worth doing, even though I don't know of anybody
who has yet been bitten by this bug.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-12 22:23                         ` David Kastrup
  2006-09-12 23:44                           ` David Kastrup
@ 2006-09-13 19:24                           ` Richard Stallman
  1 sibling, 0 replies; 55+ messages in thread
From: Richard Stallman @ 2006-09-13 19:24 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

    The change I proposed concerning read-key-sequence was for the sake of
    letting both keymap and local-map properties (if they are specified in
    a string) be heeded.  The current code only heeds the first of two.

That sounds like something that should be done.  Can you please add
comments to explain the desired effect of your change, and explain
how it achieves that effect?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13  8:11                               ` David Kastrup
  2006-09-13 11:05                                 ` Kim F. Storm
@ 2006-09-13 19:25                                 ` Richard Stallman
  2006-09-13 20:02                                   ` David Kastrup
  1 sibling, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-13 19:25 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

Why add the argument LOCATION?  Is it just for the sake of
command-rebinding?

Why the need to switch buffers?

Aside from that, some little details:

    + 	* keymap.c: include "window.h".

You need to add a dependency in Makefile.in for that.

    !       if (SYMBOLP (AREF(location,0)) && ASIZE(location) > 1)
    ! 	event = AREF(location,1);
    !       else
    ! 	event = AREF(location,0);

To follow our coding style you need additional spaces here:

    !       if (SYMBOLP (AREF (location,0)) && ASIZE (location) > 1)
    ! 	event = AREF (location, 1);
    !       else
    ! 	event = AREF (location, 0);

A comma should always be followed by a space.
Outside of a parenthesis there must be a space,
unless the same spot is inside another parenthesis
or followed by a semicolon.

There needs to be documentation for LOCATION in etc/NEWS and the Lisp
Manual.

    ! 	  /* Key sequences beginning with mouse clicks are
    ! 	     read using the keymaps in the buffer clicked on,
    ! 	     not the current buffer.  If we're at the
    ! 	     beginning of a key sequence, switch buffers.  */

I can't understand the second sentence, "If we're at the beginning of
a key sequence".  Since key-binding always operates on a key sequence,
and since we're not in the middle of a loop which scans that key
sequence, what does it mean to say that we are, or are not, "at the
beginning" of it?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 11:38                                   ` David Kastrup
  2006-09-13 12:23                                     ` Kim F. Storm
@ 2006-09-13 19:25                                     ` Richard Stallman
  2006-09-13 19:49                                       ` David Kastrup
  1 sibling, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-13 19:25 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

    There is one concern that I have: the type of the "location" argument.
    This currently is a key sequence.  But it might make more sense to
    turn this into the "location" data structure that `event-start' and/or
    `event-end' return.  This would make it much easier to feed lookup-key
    with data produced from, say, `posn-at-x-y'.

That seems like a good idea.  (We call that a "position".)

    In order to get this data easier, it might make sense to define a
    convenience function.

    (defun key-event (key)
       "Return event from moused-base key sequence KEY."
       (and (vectorp key)
	    (if (consp (aref key 0))
		(aref key 0)
	      (and
		(symbolp (aref key 0))
		(> (length key) 1)
		(consp (aref key 1))
		(aref key 1)))))

I have nothing against it, but do we really need it?
Is there a place that needs to call this, outside of the code
of Fkey_binding itself?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 19:25                                     ` Richard Stallman
@ 2006-09-13 19:49                                       ` David Kastrup
  0 siblings, 0 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-13 19:49 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

Richard Stallman <rms@gnu.org> writes:

>     There is one concern that I have: the type of the "location"
>     argument.  This currently is a key sequence.  But it might make
>     more sense to turn this into the "location" data structure that
>     `event-start' and/or `event-end' return.  This would make it
>     much easier to feed lookup-key with data produced from, say,
>     `posn-at-x-y'.
>
> That seems like a good idea.  (We call that a "position".)

Yes, I already noticed and changed this accordingly.  It actually
shrinked the size of the code.

>     In order to get this data easier, it might make sense to define a
>     convenience function.
>
>     (defun key-event (key)
>        "Return event from moused-base key sequence KEY."
>        (and (vectorp key)
> 	    (if (consp (aref key 0))
> 		(aref key 0)
> 	      (and
> 		(symbolp (aref key 0))
> 		(> (length key) 1)
> 		(consp (aref key 1))
> 		(aref key 1)))))
>
> I have nothing against it, but do we really need it?
> Is there a place that needs to call this, outside of the code
> of Fkey_binding itself?

There will be several places in help.el and possibly mouse.el which
will need to do this operation after I finished adapting them to do
the right thing in all cases.

This can be decided when I am preparing the patches for those files.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 19:25                                 ` Richard Stallman
@ 2006-09-13 20:02                                   ` David Kastrup
  0 siblings, 0 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-13 20:02 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

Richard Stallman <rms@gnu.org> writes:

> Why add the argument LOCATION?  Is it just for the sake of
> command-rebinding?

See the more elaborate explanation in a recent mail.

> Why the need to switch buffers?

Because the values of overriding-local-map and the current minor maps
might depend on it.  The buffer-local maps for the lookup of a mouse
event are taken from the buffer where the mouse event occured, not the
currently selected buffer.

> Aside from that, some little details:
>
>     + 	* keymap.c: include "window.h".
>
> You need to add a dependency in Makefile.in for that.

Ouch.  Yes.

>     !       if (SYMBOLP (AREF(location,0)) && ASIZE(location) > 1)
>     ! 	event = AREF(location,1);
>     !       else
>     ! 	event = AREF(location,0);
>
> To follow our coding style you need additional spaces here:
>
>     !       if (SYMBOLP (AREF (location,0)) && ASIZE (location) > 1)
>     ! 	event = AREF (location, 1);
>     !       else
>     ! 	event = AREF (location, 0);
>
> A comma should always be followed by a space.
> Outside of a parenthesis there must be a space,
> unless the same spot is inside another parenthesis
> or followed by a semicolon.

OK, will do.

> There needs to be documentation for LOCATION in etc/NEWS and the Lisp
> Manual.

Well, I wanted to get the code right (and use it) before tackling
those: and indeed, LOCATION has already morphed into POSITION because
the latter is more useful for mouse.el...

I would want to postpone the entries until I have finished recoding
the use cases of this functionality, and they test out well.  That
way, I don't need to rewrite stuff in case the API turns out not to do
the trick.

>     ! 	  /* Key sequences beginning with mouse clicks are
>     ! 	     read using the keymaps in the buffer clicked on,
>     ! 	     not the current buffer.  If we're at the
>     ! 	     beginning of a key sequence, switch buffers.  */
>
> I can't understand the second sentence, "If we're at the beginning
> of a key sequence".  Since key-binding always operates on a key
> sequence, and since we're not in the middle of a loop which scans
> that key sequence, what does it mean to say that we are, or are not,
> "at the beginning" of it?

Uhm, well, it means that this comment was copied and pasted from
keyboard.c (which has similar functionality) and not fully rewritten.

I'll change that.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-13 15:25                         ` David Kastrup
@ 2006-09-14  2:34                           ` Richard Stallman
  2006-09-14 11:57                             ` David Kastrup
  0 siblings, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-14  2:34 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

Ok, I agree to adding the LOCATION arg.  But it needs to be documented.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-14  2:34                           ` Richard Stallman
@ 2006-09-14 11:57                             ` David Kastrup
  2006-09-14 22:34                               ` David Kastrup
                                                 ` (2 more replies)
  0 siblings, 3 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-14 11:57 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

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

Richard Stallman <rms@gnu.org> writes:

> Ok, I agree to adding the LOCATION arg.  But it needs to be documented.

Here is the latest patch iteration.  I think it should be fit for
checking in.  It might also seem reasonable to add the optional
`position' argument to `current-active-maps', but this has not been
done.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 28586 bytes --]

Index: src/keymap.h
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.h,v
retrieving revision 1.14
diff -u -r1.14 keymap.h
*** src/keymap.h	6 Feb 2006 15:23:21 -0000	1.14
--- src/keymap.h	14 Sep 2006 11:49:06 -0000
***************
*** 29,36 ****
  EXFUN (Fkeymap_prompt, 1);
  EXFUN (Fdefine_key, 3);
  EXFUN (Flookup_key, 3);
! EXFUN (Fcommand_remapping, 1);
! EXFUN (Fkey_binding, 3);
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);
--- 29,36 ----
  EXFUN (Fkeymap_prompt, 1);
  EXFUN (Fdefine_key, 3);
  EXFUN (Flookup_key, 3);
! EXFUN (Fcommand_remapping, 2);
! EXFUN (Fkey_binding, 4);
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);
Index: src/keymap.c
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.c,v
retrieving revision 1.333
diff -u -r1.333 keymap.c
*** src/keymap.c	11 Sep 2006 13:03:40 -0000	1.333
--- src/keymap.c	14 Sep 2006 11:49:09 -0000
***************
*** 33,38 ****
--- 33,39 ----
  #include "puresize.h"
  #include "intervals.h"
  #include "keymap.h"
+ #include "window.h"
  
  /* The number of elements in keymap vectors.  */
  #define DENSE_TABLE_SIZE (0200)
***************
*** 1216,1232 ****
  
  /* This function may GC (it calls Fkey_binding).  */
  
! DEFUN ("command-remapping", Fcommand_remapping, Scommand_remapping, 1, 1, 0,
         doc: /* Return the remapping for command COMMAND in current keymaps.
! Returns nil if COMMAND is not remapped (or not a symbol).  */)
!      (command)
!      Lisp_Object command;
  {
    if (!SYMBOLP (command))
      return Qnil;
  
    ASET (command_remapping_vector, 1, command);
!   return Fkey_binding (command_remapping_vector, Qnil, Qt);
  }
  
  /* Value is number if KEY is too long; nil if valid but has no definition. */
--- 1217,1239 ----
  
  /* This function may GC (it calls Fkey_binding).  */
  
! DEFUN ("command-remapping", Fcommand_remapping, Scommand_remapping, 1, 2, 0,
         doc: /* Return the remapping for command COMMAND in current keymaps.
! Returns nil if COMMAND is not remapped (or not a symbol).
! 
! If the optional argument POSITION is non-nil, it specifies a mouse
! position as returned by `event-start' and `event-end', and the
! remapping occurs in the keymaps associated with it.  It can also be a
! number or marker, in which case the keymap properties at the specified
! buffer position instead of point are used. */)
!      (command, position)
!      Lisp_Object command, position;
  {
    if (!SYMBOLP (command))
      return Qnil;
  
    ASET (command_remapping_vector, 1, command);
!   return Fkey_binding (command_remapping_vector, Qnil, Qt, position);
  }
  
  /* Value is number if KEY is too long; nil if valid but has no definition. */
***************
*** 1552,1558 ****
  
  /* GC is possible in this function if it autoloads a keymap.  */
  
! DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 3, 0,
         doc: /* Return the binding for command KEY in current keymaps.
  KEY is a string or vector, a sequence of keystrokes.
  The binding is probably a symbol with a function definition.
--- 1559,1565 ----
  
  /* GC is possible in this function if it autoloads a keymap.  */
  
! DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 4, 0,
         doc: /* Return the binding for command KEY in current keymaps.
  KEY is a string or vector, a sequence of keystrokes.
  The binding is probably a symbol with a function definition.
***************
*** 1566,1620 ****
  Like the normal command loop, `key-binding' will remap the command
  resulting from looking up KEY by looking up the command in the
  current keymaps.  However, if the optional third argument NO-REMAP
! is non-nil, `key-binding' returns the unmapped command.  */)
!      (key, accept_default, no_remap)
!      Lisp_Object key, accept_default, no_remap;
  {
    Lisp_Object *maps, value;
    int nmaps, i;
!   struct gcpro gcpro1;
  
!   GCPRO1 (key);
  
! #ifdef HAVE_MOUSE
!   if (VECTORP (key) && ASIZE (key) > 0)
      {
!       Lisp_Object ev, pos;
!       if ((ev = AREF (key, 0), CONSP (ev))
! 	  && SYMBOLP (XCAR (ev))
! 	  && CONSP (XCDR (ev))
! 	  && (pos = XCAR (XCDR (ev)), CONSP (pos))
! 	  && XINT (Flength (pos)) == 10
! 	  && INTEGERP (XCAR (XCDR (pos))))
! 	{
! 	  Lisp_Object map, object;
  
! 	  object = Fnth (make_number(4), pos);
  
! 	  if (CONSP (object))
! 	    map = Fget_char_property (XCDR (object), Qkeymap, XCAR (object));
! 	  else
! 	    map = Fget_char_property (XCAR (XCDR (pos)), Qkeymap,
! 				      Fwindow_buffer (XCAR (pos)));
  
! 	  if (!NILP (Fkeymapp (map)))
! 	    {
! 	      value = Flookup_key (map, key, accept_default);
! 	      if (! NILP (value) && !INTEGERP (value))
! 		goto done;
! 	    }
! 	}
      }
- #endif /* HAVE_MOUSE  */
  
!   if (!NILP (current_kboard->Voverriding_terminal_local_map))
      {
        value = Flookup_key (current_kboard->Voverriding_terminal_local_map,
  			   key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
  	goto done;
      }
!   else if (!NILP (Voverriding_local_map))
      {
        value = Flookup_key (Voverriding_local_map, key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
--- 1573,1658 ----
  Like the normal command loop, `key-binding' will remap the command
  resulting from looking up KEY by looking up the command in the
  current keymaps.  However, if the optional third argument NO-REMAP
! is non-nil, `key-binding' returns the unmapped command.
! 
! If KEY is a key sequence initiated with the mouse, the used keymaps
! will depend on the clicked mouse position with regard to the buffer
! and possible local keymaps on strings.
! 
! If the optional argument POSITION is non-nil, it specifies a mouse
! position as returned by `event-start' and `event-end', and the lookup
! occurs in the keymaps associated with it instead of KEY.  It can also
! be a number or marker, in which case the keymap properties at the
! specified buffer position instead of point are used.
!   */)
!     (key, accept_default, no_remap, position)
!     Lisp_Object key, accept_default, no_remap, position;
  {
    Lisp_Object *maps, value;
    int nmaps, i;
!   struct gcpro gcpro1, gcpro2;
!   int count = SPECPDL_INDEX ();
  
!   GCPRO2 (key, position);
  
!   if (NILP (position))
      {
!       Lisp_Object event;
!       /* mouse events may have a symbolic prefix indicating the
! 	 scrollbar or mode line */
!       if (SYMBOLP (AREF (key, 0)) && ASIZE (key) > 1)
! 	event = AREF (key, 1);
!       else
! 	event = AREF (key, 0);
  
!       /* We are not interested in locations without event data */
  
!       if (EVENT_HAS_PARAMETERS (event)) {
! 	Lisp_Object kind;
  
! 	kind = EVENT_HEAD_KIND (EVENT_HEAD (event));
! 	if (EQ (kind, Qmouse_click))
! 	  position = EVENT_START (event);
!       }
      }
  
!   /* Key sequences beginning with mouse clicks
!      are read using the keymaps of the buffer clicked on, not
!      the current buffer.  So we may have to switch the buffer
!      here. */
!   
!   if (CONSP (position))
!     {
!       Lisp_Object window;
!       
!       window = POSN_WINDOW (position);
! 	  
!       if (WINDOWP (window)
! 	  && BUFFERP (XWINDOW (window)->buffer)
! 	  && XBUFFER (XWINDOW (window)->buffer) != current_buffer)
! 	{
! 	  /* Arrange to go back to the original buffer once we're done
! 	     processing the key sequence.  We don't use
! 	     save_excursion_{save,restore} here, in analogy to
! 	     `read-key-sequence' to avoid saving point.  Maybe this
! 	     would not be a problem here, but it is easier to keep
! 	     things the same.
! 	  */
! 	      
! 	  record_unwind_protect (Fset_buffer, Fcurrent_buffer ());
! 	  
! 	  set_buffer_internal (XBUFFER (XWINDOW (window)->buffer));
! 	}
!     }
!   
!   if (! NILP (current_kboard->Voverriding_terminal_local_map))
      {
        value = Flookup_key (current_kboard->Voverriding_terminal_local_map,
  			   key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
  	goto done;
      }
!   else if (! NILP (Voverriding_local_map))
      {
        value = Flookup_key (Voverriding_local_map, key, accept_default);
        if (! NILP (value) && !INTEGERP (value))
***************
*** 1622,1633 ****
      }
    else
      {
!       Lisp_Object local;
  
!       local = get_local_map (PT, current_buffer, Qkeymap);
!       if (! NILP (local))
  	{
! 	  value = Flookup_key (local, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
--- 1660,1731 ----
      }
    else
      {
!       Lisp_Object keymap, local_map;
!       EMACS_INT pt;
  
!       pt = INTEGERP (position) ? XINT (position)
! 	: MARKERP (position) ? marker_position (position)
! 	: PT;
! 
!       local_map = get_local_map (pt, current_buffer, Qlocal_map); 
!       keymap = get_local_map (pt, current_buffer, Qkeymap); 
! 
!       if (CONSP (position))
! 	{
! 	  Lisp_Object string, window;
! 
! 	  window = POSN_WINDOW (position);
! 
! 	  /* For a mouse click, get the local text-property keymap
! 	     of the place clicked on, rather than point.  */
! 	  
! 	  if (POSN_INBUFFER_P (position))
! 	    {
! 	      Lisp_Object pos;
! 
! 	      pos = POSN_BUFFER_POSN (position);
! 	      if (INTEGERP (pos)
! 		  && XINT (pos) >= BEG && XINT (pos) <= Z)
! 		{
! 		  local_map = get_local_map (XINT (pos),
! 					     current_buffer, Qlocal_map);
! 		  
! 		  keymap = get_local_map (XINT (pos),
! 					  current_buffer, Qkeymap);
! 		}
! 	    }
! 
! 	  /* If on a mode line string with a local keymap,
! 	     or for a click on a string, i.e. overlay string or a
! 	     string displayed via the `display' property,
! 	     consider `local-map' and `keymap' properties of
! 	     that string.  */
! 	  
! 	  if (string = POSN_STRING (position),
! 	      (CONSP (string) && STRINGP (XCAR (string))))
! 	    {
! 	      Lisp_Object pos, map;
! 	      
! 	      pos = XCDR (string);
! 	      string = XCAR (string);
! 	      if (XINT (pos) >= 0
! 		  && XINT (pos) < SCHARS (string))
! 		{
! 		  map = Fget_text_property (pos, Qlocal_map, string);
! 		  if (!NILP (map))
! 		    local_map = map;
! 
! 		  map = Fget_text_property (pos, Qkeymap, string);
! 		  if (!NILP (map))
! 		    keymap = map;
! 		}
! 	    }
! 	  
! 	}
! 
!       if (! NILP (keymap))
  	{
! 	  value = Flookup_key (keymap, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
***************
*** 1644,1653 ****
  	      goto done;
  	  }
  
!       local = get_local_map (PT, current_buffer, Qlocal_map);
!       if (! NILP (local))
  	{
! 	  value = Flookup_key (local, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
--- 1742,1750 ----
  	      goto done;
  	  }
  
!       if (! NILP (local_map))
  	{
! 	  value = Flookup_key (local_map, key, accept_default);
  	  if (! NILP (value) && !INTEGERP (value))
  	    goto done;
  	}
***************
*** 1656,1661 ****
--- 1753,1760 ----
    value = Flookup_key (current_global_map, key, accept_default);
  
   done:
+   unbind_to (count, Qnil);
+ 
    UNGCPRO;
    if (NILP (value) || INTEGERP (value))
      return Qnil;
***************
*** 1666,1672 ****
    if (NILP (no_remap) && SYMBOLP (value))
      {
        Lisp_Object value1;
!       if (value1 = Fcommand_remapping (value), !NILP (value1))
  	value = value1;
      }
  
--- 1765,1771 ----
    if (NILP (no_remap) && SYMBOLP (value))
      {
        Lisp_Object value1;
!       if (value1 = Fcommand_remapping (value, position), !NILP (value1))
  	value = value1;
      }
  
***************
*** 2467,2473 ****
    if (NILP (no_remap) && SYMBOLP (definition))
      {
        Lisp_Object tem;
!       if (tem = Fcommand_remapping (definition), !NILP (tem))
  	return Qnil;
      }
  
--- 2566,2572 ----
    if (NILP (no_remap) && SYMBOLP (definition))
      {
        Lisp_Object tem;
!       if (tem = Fcommand_remapping (definition, Qnil), !NILP (tem))
  	return Qnil;
      }
  
Index: src/keyboard.c
===================================================================
RCS file: /sources/emacs/emacs/src/keyboard.c,v
retrieving revision 1.876
diff -u -r1.876 keyboard.c
*** src/keyboard.c	13 Sep 2006 15:12:59 -0000	1.876
--- src/keyboard.c	14 Sep 2006 11:49:20 -0000
***************
*** 1674,1680 ****
        if (SYMBOLP (cmd))
  	{
  	  Lisp_Object cmd1;
! 	  if (cmd1 = Fcommand_remapping (cmd), !NILP (cmd1))
  	    cmd = cmd1;
  	}
  
--- 1674,1680 ----
        if (SYMBOLP (cmd))
  	{
  	  Lisp_Object cmd1;
! 	  if (cmd1 = Fcommand_remapping (cmd, Qnil), !NILP (cmd1))
  	    cmd = cmd1;
  	}
  
***************
*** 7517,7523 ****
        Lisp_Object prefix;
  
        if (!NILP (tem))
! 	tem = Fkey_binding (tem, Qnil, Qnil);
  
        prefix = AREF (item_properties, ITEM_PROPERTY_KEYEQ);
        if (CONSP (prefix))
--- 7517,7523 ----
        Lisp_Object prefix;
  
        if (!NILP (tem))
! 	tem = Fkey_binding (tem, Qnil, Qnil, Qnil);
  
        prefix = AREF (item_properties, ITEM_PROPERTY_KEYEQ);
        if (CONSP (prefix))
***************
*** 9134,9149 ****
  			  if (!EQ (map_here, orig_local_map))
  			    {
  			      orig_local_map = map_here;
! 			      keybuf[t] = key;
! 			      mock_input = t + 1;
! 
! 			      goto replay_sequence;
  			    }
  			  map_here = get_local_map (XINT (pos),
  						     current_buffer, Qkeymap);
  			  if (!EQ (map_here, orig_keymap))
  			    {
  			      orig_keymap = map_here;
  			      keybuf[t] = key;
  			      mock_input = t + 1;
  
--- 9134,9152 ----
  			  if (!EQ (map_here, orig_local_map))
  			    {
  			      orig_local_map = map_here;
! 			      ++localized_local_map;
  			    }
+ 
  			  map_here = get_local_map (XINT (pos),
  						     current_buffer, Qkeymap);
  			  if (!EQ (map_here, orig_keymap))
  			    {
  			      orig_keymap = map_here;
+ 			      ++localized_local_map;
+ 			    }
+ 
+ 			  if (localized_local_map > 1)
+ 			    {
  			      keybuf[t] = key;
  			      mock_input = t + 1;
  
Index: src/Makefile.in
===================================================================
RCS file: /sources/emacs/emacs/src/Makefile.in,v
retrieving revision 1.330
diff -u -r1.330 Makefile.in
*** src/Makefile.in	6 Sep 2006 17:53:59 -0000	1.330
--- src/Makefile.in	14 Sep 2006 11:49:21 -0000
***************
*** 1146,1152 ****
     systty.h systime.h dispextern.h syntax.h $(INTERVAL_SRC) blockinput.h \
     atimer.h xterm.h puresize.h msdos.h keymap.h w32term.h macterm.h $(config_h)
  keymap.o: keymap.c buffer.h commands.h keyboard.h termhooks.h blockinput.h \
!    atimer.h systime.h puresize.h charset.h intervals.h $(config_h)
  lastfile.o: lastfile.c  $(config_h)
  macros.o: macros.c window.h buffer.h commands.h macros.h keyboard.h \
  	dispextern.h $(config_h)
--- 1146,1153 ----
     systty.h systime.h dispextern.h syntax.h $(INTERVAL_SRC) blockinput.h \
     atimer.h xterm.h puresize.h msdos.h keymap.h w32term.h macterm.h $(config_h)
  keymap.o: keymap.c buffer.h commands.h keyboard.h termhooks.h blockinput.h \
!    atimer.h systime.h puresize.h charset.h intervals.h keymap.h window.h \
!    $(config_h)
  lastfile.o: lastfile.c  $(config_h)
  macros.o: macros.c window.h buffer.h commands.h macros.h keyboard.h \
  	dispextern.h $(config_h)
Index: src/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/src/ChangeLog,v
retrieving revision 1.5297
diff -u -r1.5297 ChangeLog
*** src/ChangeLog	14 Sep 2006 09:37:29 -0000	1.5297
--- src/ChangeLog	14 Sep 2006 11:49:51 -0000
***************
*** 1,3 ****
--- 1,25 ----
+ 2006-09-14  David Kastrup  <dak@gnu.org>
+ 
+ 	* Makefile.in (keymap.o): Add "keymap.h" and "window.h"
+ 	dependencies.
+ 
+ 	* keymap.c: include "window.h".
+ 	(Fcommand_remapping): New optional POSITION argument.
+ 	(Fkey_binding): New optional POSITION argument.  Completely rework
+ 	handling of mouse clicks to get the same order of keymaps as
+ 	`read-key-sequence' and heed POSITION.  Also temporarily switch
+ 	buffers to location of mouse click and back.
+ 
+ 	* keyboard.c (command_loop_1): Adjust call of `Fcommand_remapping'
+ 	for additional argument.
+ 	(parse_menu_item): Adjust call of `Fkey_binding' for additional
+ 	argument.
+ 	(read_key_sequence): If there are both `local-map' and `keymap'
+ 	text properties at some buffer position, heed both.
+ 
+ 	* keymap.h: Declare additional optional arguments of
+ 	`Fcommand_remapping' and `Fkey_binding'.
+ 
  2006-09-14  Kim F. Storm  <storm@cua.dk>
  
  	* xdisp.c (produce_image_glyph): Automatically crop wide images at
Index: lispref/keymaps.texi
===================================================================
RCS file: /sources/emacs/emacs/lispref/keymaps.texi,v
retrieving revision 1.86
diff -u -r1.86 keymaps.texi
*** lispref/keymaps.texi	11 Sep 2006 14:34:16 -0000	1.86
--- lispref/keymaps.texi	14 Sep 2006 11:49:54 -0000
***************
*** 583,594 ****
          (@var{find-in} overriding-terminal-local-map)
        (if overriding-local-map
            (@var{find-in} overriding-local-map)
!         (or (@var{find-in} (get-text-property (point) 'keymap))
              (@var{find-in-any} emulation-mode-map-alists)
              (@var{find-in-any} minor-mode-overriding-map-alist)
              (@var{find-in-any} minor-mode-map-alist)
!             (if (get-text-property (point) 'local-map)
!                 (@var{find-in} (get-text-property (point) 'local-map))
                (@var{find-in} (current-local-map))))))
      (@var{find-in} (current-global-map)))
  @end lisp
--- 583,594 ----
          (@var{find-in} overriding-terminal-local-map)
        (if overriding-local-map
            (@var{find-in} overriding-local-map)
!         (or (@var{find-in} (get-char-property (point) 'keymap))
              (@var{find-in-any} emulation-mode-map-alists)
              (@var{find-in-any} minor-mode-overriding-map-alist)
              (@var{find-in-any} minor-mode-map-alist)
!             (if (get-char-property (point) 'local-map)
!                 (@var{find-in} (get-char-property (point) 'local-map))
                (@var{find-in} (current-local-map))))))
      (@var{find-in} (current-global-map)))
  @end lisp
***************
*** 599,604 ****
--- 599,614 ----
  appropriate keymaps from an alist.  (Searching a single keymap for a
  binding is called @dfn{key lookup}; see @ref{Key Lookup}.)
  
+ This process is somewhat modified for mouse events: the local modes and
+ keymaps of the buffer corresponding to the mouse click position are
+ searched instead, text properties are taken from the mouse click
+ position in the buffer rather than point, and if the click happens on a
+ string embedded with a @code{display}, @code{before-string}, or
+ @code{after-string} text property (@pxref{Special Properties}) or
+ overlay property (@pxref{Overlay Properties}), any non-@code{nil} maps
+ specified with text properties of this string are searched instead of
+ those of the buffer.
+ 
    The @dfn{global keymap} holds the bindings of keys that are defined
  regardless of the current buffer, such as @kbd{C-f}.  The variable
  @code{global-map} holds this keymap, which is always active.
***************
*** 655,679 ****
  non-@code{nil} then it pays attention to them.
  @end defun
  
! @defun key-binding key &optional accept-defaults no-remap
! This function returns the binding for @var{key} according to the
! current active keymaps.  The result is @code{nil} if @var{key} is
! undefined in the keymaps.
  
  @c Emacs 19 feature
  The argument @var{accept-defaults} controls checking for default
  bindings, as in @code{lookup-key} (above).
  
- When @var{key} is a vector containing an input event, such as a mouse
- click, @code{key-binding} first looks for the binding in the keymaps
- that would be active at the position where the click was done.
- 
  When commands are remapped (@pxref{Remapping Commands}),
  @code{key-binding} normally processes command remappings so as to
  returns the remapped command that will actually be executed.  However,
  if @var{no-remap} is non-@code{nil}, @code{key-binding} ignores
  remappings and returns the binding directly specified for @var{key}.
  
  An error is signaled if @var{key} is not a string or a vector.
  
  @example
--- 665,691 ----
  non-@code{nil} then it pays attention to them.
  @end defun
  
! @defun key-binding key &optional accept-defaults no-remap position
! This function returns the binding for @var{key} according to the current
! active keymaps.  The result is @code{nil} if @var{key} is undefined in
! the keymaps.  If @var{key} is a key sequence started with the mouse, the
! consulted maps will be changed accordingly.
  
  @c Emacs 19 feature
  The argument @var{accept-defaults} controls checking for default
  bindings, as in @code{lookup-key} (above).
  
  When commands are remapped (@pxref{Remapping Commands}),
  @code{key-binding} normally processes command remappings so as to
  returns the remapped command that will actually be executed.  However,
  if @var{no-remap} is non-@code{nil}, @code{key-binding} ignores
  remappings and returns the binding directly specified for @var{key}.
  
+ If @var{position} is non-@code{nil}, it specifies either a buffer
+ position or a position like those returned from @code{event-start}.  In
+ this case, @var{position} instead of @var{key} determines the
+ click-specific maps.
+ 
  An error is signaled if @var{key} is not a string or a vector.
  
  @example
***************
*** 696,714 ****
          (@var{find-in} overriding-terminal-local-map)
        (if overriding-local-map
            (@var{find-in} overriding-local-map)
!         (or (@var{find-in} (get-text-property (point) 'keymap))
              (@var{find-in-any} emulation-mode-map-alists)
              (@var{find-in-any} minor-mode-overriding-map-alist)
              (@var{find-in-any} minor-mode-map-alist)
              (if (get-text-property (point) 'local-map)
!                 (@var{find-in} (get-text-property (point) 'local-map))
                (@var{find-in} (current-local-map))))))
      (@var{find-in} (current-global-map)))
  @end lisp
  
  @noindent
! The @var{find-in} and @var{find-in-any} are pseudo functions that
! search in one keymap and in an alist of keymaps, respectively.
  
  @enumerate
  @item
--- 708,730 ----
          (@var{find-in} overriding-terminal-local-map)
        (if overriding-local-map
            (@var{find-in} overriding-local-map)
!         (or (@var{find-in} (get-char-property (point) 'keymap))
              (@var{find-in-any} emulation-mode-map-alists)
              (@var{find-in-any} minor-mode-overriding-map-alist)
              (@var{find-in-any} minor-mode-map-alist)
              (if (get-text-property (point) 'local-map)
!                 (@var{find-in} (get-char-property (point) 'local-map))
                (@var{find-in} (current-local-map))))))
      (@var{find-in} (current-global-map)))
  @end lisp
  
  @noindent
! The @var{find-in} and @var{find-in-any} are pseudo functions that search
! in one keymap and in an alist of keymaps, respectively.  Mouse events
! will consult the maps and positions of the buffer where the event
! started, and will also consult text properties of a string displayed via
! display or overlay properties instead of those of the buffer when the
! former are non-@code{nil}.
  
  @enumerate
  @item
***************
*** 1470,1480 ****
  if an ordinary binding specifies @code{my-kill-line}, this keymap will
  remap it to @code{my-other-kill-line}.
  
! @defun command-remapping command
! This function returns the remapping for @var{command} (a symbol),
! given the current active keymaps.  If @var{command} is not remapped
! (which is the usual situation), or not a symbol, the function returns
! @code{nil}.
  @end defun
  
  @node Translation Keymaps
--- 1486,1498 ----
  if an ordinary binding specifies @code{my-kill-line}, this keymap will
  remap it to @code{my-other-kill-line}.
  
! @defun command-remapping command &optional position
! This function returns the remapping for @var{command} (a symbol), given
! the current active keymaps.  If @var{command} is not remapped (which is
! the usual situation), or not a symbol, the function returns @code{nil}.
! @code{position} can optionally specify a buffer position or a position
! like those returned from @code{event-start}: in that case, the active
! maps are changed like they are in @code{key-binding}.
  @end defun
  
  @node Translation Keymaps
Index: lispref/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/lispref/ChangeLog,v
retrieving revision 1.750
diff -u -r1.750 ChangeLog
*** lispref/ChangeLog	12 Sep 2006 01:43:17 -0000	1.750
--- lispref/ChangeLog	14 Sep 2006 11:50:00 -0000
***************
*** 1,3 ****
--- 1,12 ----
+ 2006-09-14  David Kastrup  <dak@gnu.org>
+ 
+ 	* keymaps.texi (Active Keymaps): Adapt description to use
+ 	`get-char-property' instead `get-text-property'.  Explain how
+ 	mouse events change this.  Explain the new optional argument of
+ 	`key-binding' and its mouse-dependent lookup.
+ 	(Searching Keymaps): Adapt description similarly.  Explain the new
+ 	optional argument of `command-remapping'.
+ 
  2006-09-11  Richard Stallman  <rms@gnu.org>
  
  	* display.texi (Display Table Format): Wording clarification.
***************
*** 4859,4865 ****
  	(info): Add target.
  	(installall): Target removed.
  
! 2001-10-31  Pavel Jan^[,Bm^[(Bk  <Pavel@Janik.cz>
  
  	* tips.texi (Coding Conventions): Fix typo.
  
--- 4868,4874 ----
  	(info): Add target.
  	(installall): Target removed.
  
! 2001-10-31  Pavel Jan^[,Am^[(Bk  <Pavel@Janik.cz>
  
  	* tips.texi (Coding Conventions): Fix typo.
  
Index: etc/NEWS
===================================================================
RCS file: /sources/emacs/emacs/etc/NEWS,v
retrieving revision 1.1394
diff -u -r1.1394 NEWS
*** etc/NEWS	12 Sep 2006 16:43:23 -0000	1.1394
--- etc/NEWS	14 Sep 2006 11:50:07 -0000
***************
*** 4666,4671 ****
--- 4666,4677 ----
  text properties, according to their stickiness.  This also means that it
  works with empty overlays.  The same hold for the `local-map' property.
  
+ *** `key-binding' will now look up mouse-specific bindings.  The
+ keymaps consulted by `key-binding' will get adapted if the key
+ sequence is started with a mouse event.  Instead of letting the click
+ position be determined from the key sequence itself, it is also
+ possible to specify it with an optional argument explicitly.
+ 
  *** Dense keymaps now handle inheritance correctly.
  
  Previously a dense keymap would hide all of the simple-char key
Index: etc/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/etc/ChangeLog,v
retrieving revision 1.452
diff -u -r1.452 ChangeLog
*** etc/ChangeLog	12 Sep 2006 16:43:23 -0000	1.452
--- etc/ChangeLog	14 Sep 2006 11:50:09 -0000
***************
*** 1,3 ****
--- 1,8 ----
+ 2006-09-14  David Kastrup  <dak@gnu.org>
+ 
+ 	* NEWS: explain new behavior and arguments of `key-binding' and
+ 	`command-remapping'.
+ 
  2006-09-11  Paul Eggert  <eggert@cs.ucla.edu>
  
  	* NEWS: In terminal-oriented subshells, the EMACS environment
***************
*** 102,108 ****
  
  	* PROBLEMS: Emacs now requires ws2_32.dll on Windows.
  
! 2006-07-14  K^[,Aa^[(Broly L^[,Bu^[(Brentey  <lorentey@elte.hu>
  
  	* HELLO: Update Hungarian sample.
  
--- 107,113 ----
  
  	* PROBLEMS: Emacs now requires ws2_32.dll on Windows.
  
! 2006-07-14  K^[,Aa^[(Broly L^[$,1 q^[(Brentey  <lorentey@elte.hu>
  
  	* HELLO: Update Hungarian sample.
  

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-14 11:57                             ` David Kastrup
@ 2006-09-14 22:34                               ` David Kastrup
  2006-09-14 22:36                                 ` David Kastrup
  2006-09-15  3:14                               ` Richard Stallman
  2006-09-15 23:47                               ` David Kastrup
  2 siblings, 1 reply; 55+ messages in thread
From: David Kastrup @ 2006-09-14 22:34 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

David Kastrup <dak@gnu.org> writes:

> Here is the latest patch iteration.  I think it should be fit for
> checking in.  It might also seem reasonable to add the optional
> `position' argument to `current-active-maps', but this has not been
> done.

Ok, and since there has been some quietness from Richard today, let me
feed the patch queue further:

The following are changes to help.el, mouse.el and similar.  I think
it becomes obvious that the idiom "Take a look at whether element 0 or
element 1 of a key sequence happen to be an event" is used pretty
often in those files, so maybe `key-event' would be nice to have in
subr.el.

Another addition is `mouse-posn-property', a new function: this could
be another candidate for subr.el (under the name `posn-property',
then).  It digs through a possible display string and then the buffer
for a given property.  At the current point of time, it does not sift
through a possible "image" property list in case an image is clicked:
image properties tend to be suitably dissimilar from buffer properties
that it does not seem to make much sense to treat them the same.

Note that this patch set does not currently need a NEWS entry or
similar: it more or less brings a bunch of behavior in line with what
one would expect.

Don't apply it unless the key-binding patch set has been applied
previously, or things will _really_ break.

So once Richard has been able to look over his mail again, I'd like to
apply both patch sets.  They work fine here.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-14 22:34                               ` David Kastrup
@ 2006-09-14 22:36                                 ` David Kastrup
  0 siblings, 0 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-14 22:36 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

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

David Kastrup <dak@gnu.org> writes:

> Don't apply it unless the key-binding patch set has been applied
> previously, or things will _really_ break.
>
> So once Richard has been able to look over his mail again, I'd like
> to apply both patch sets.  They work fine here.

I am trying to set a new record for not or badly included patches.

Next try:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 19740 bytes --]

Index: lisp/mouse.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mouse.el,v
retrieving revision 1.300
diff -u -r1.300 mouse.el
*** lisp/mouse.el	17 Aug 2006 03:36:17 -0000	1.300
--- lisp/mouse.el	14 Sep 2006 22:23:10 -0000
***************
*** 775,780 ****
--- 775,791 ----
        (mouse-drag-track start-event t))))
  
  
+ (defun mouse-posn-property (pos property)
+   "Look for a property at click position."
+   (if (consp pos)
+       (let ((w (posn-window pos)) (pt (posn-point pos))
+ 	    (str (posn-string pos)))
+ 	(or (and str
+ 		 (get-text-property (cdr str) property (car str)))
+ 	    (and pt
+ 		 (get-char-property pt property w))))
+     (get-char-property pos property)))
+ 
  (defun mouse-on-link-p (pos)
    "Return non-nil if POS is on a link in the current buffer.
  POS must be a buffer position in the current buffer or a mouse
***************
*** 814,837 ****
  
  - Otherwise, the mouse-1 event is translated into a mouse-2 event
  at the same position."
!   (let ((w (and (consp pos) (posn-window pos))))
!     (if (consp pos)
! 	(setq pos (and (or mouse-1-click-in-non-selected-windows
! 			   (eq (selected-window) w))
! 		       (posn-point pos))))
!     (when pos
!       (with-current-buffer (window-buffer w)
! 	(let ((action
! 	       (or (get-char-property pos 'follow-link)
! 		   (save-excursion
! 		     (goto-char pos)
! 		     (key-binding [follow-link] nil t)))))
! 	  (cond
! 	   ((eq action 'mouse-face)
! 	    (and (get-char-property pos 'mouse-face) t))
! 	   ((functionp action)
! 	    (funcall action pos))
! 	   (t action)))))))
  
  (defun mouse-fixup-help-message (msg)
    "Fix help message MSG for `mouse-1-click-follows-link'."
--- 825,842 ----
  
  - Otherwise, the mouse-1 event is translated into a mouse-2 event
  at the same position."
!   (let ((action
! 	 (and (or (not (consp pos))
! 		  mouse-1-click-in-non-selected-windows
! 		  (eq (selected-window) (posn-window pos)))
! 	      (or (mouse-posn-property pos 'follow-link)
! 		  (key-binding [follow-link] nil t pos)))))
!     (cond
!      ((eq action 'mouse-face)
!       (and (mouse-posn-property pos 'mouse-face) t))
!      ((functionp action)
!       (funcall action pos))
!      (t action))))
  
  (defun mouse-fixup-help-message (msg)
    "Fix help message MSG for `mouse-1-click-follows-link'."
***************
*** 904,910 ****
                         ;; Use start-point before the intangibility
                         ;; treatment, in case we click on a link inside an
                         ;; intangible text.
!                        (mouse-on-link-p start-point)))
  	 (click-count (1- (event-click-count start-event)))
  	 (remap-double-click (and on-link
  				  (eq mouse-1-click-follows-link 'double)
--- 909,915 ----
                         ;; Use start-point before the intangibility
                         ;; treatment, in case we click on a link inside an
                         ;; intangible text.
!                        (mouse-on-link-p start-posn)))
  	 (click-count (1- (event-click-count start-event)))
  	 (remap-double-click (and on-link
  				  (eq mouse-1-click-follows-link 'double)
Index: lisp/mouse-sel.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mouse-sel.el,v
retrieving revision 1.47
diff -u -r1.47 mouse-sel.el
*** lisp/mouse-sel.el	6 Feb 2006 14:33:34 -0000	1.47
--- lisp/mouse-sel.el	14 Sep 2006 22:23:10 -0000
***************
*** 702,708 ****
  using double-clicks."
    (and initial final mouse-1-click-follows-link
         (eq (car initial) 'down-mouse-1)
!        (mouse-on-link-p	(posn-point (event-start initial)))
         (= (posn-point (event-start initial))
  	  (posn-point (event-end final)))
         (= (event-click-count initial) 1)
--- 702,708 ----
  using double-clicks."
    (and initial final mouse-1-click-follows-link
         (eq (car initial) 'down-mouse-1)
!        (mouse-on-link-p (event-start initial))
         (= (posn-point (event-start initial))
  	  (posn-point (event-end final)))
         (= (event-click-count initial) 1)
Index: lisp/help.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/help.el,v
retrieving revision 1.315
diff -u -r1.315 help.el
*** lisp/help.el	11 Sep 2006 09:47:43 -0000	1.315
--- lisp/help.el	14 Sep 2006 22:23:12 -0000
***************
*** 567,577 ****
  	     (menu-bar-update-yank-menu "(any string)" nil))
  	   (setq key (read-key-sequence "Describe key (or click or menu item): "))
  	   ;; If KEY is a down-event, read and discard the
! 	   ;; corresponding up-event.
! 	   (if (and (vectorp key)
! 		    (eventp (elt key 0))
! 		    (memq 'down (event-modifiers (elt key 0))))
! 	       (read-event))
  	   (list
  	    key
  	    (if current-prefix-arg (prefix-numeric-value current-prefix-arg))
--- 567,582 ----
  	     (menu-bar-update-yank-menu "(any string)" nil))
  	   (setq key (read-key-sequence "Describe key (or click or menu item): "))
  	   ;; If KEY is a down-event, read and discard the
! 	   ;; corresponding up-event.  Note that there are also
! 	   ;; down-events on scroll bars and mode lines: the actual
! 	   ;; event then is in the second element of the vector.
! 	   (and (vectorp key)
! 		(or (and (eventp (aref key 0))
! 			 (memq 'down (event-modifiers (aref key 0))))
! 		    (and (> (length key) 1)
! 			 (eventp (aref key 1))
! 			 (memq 'down (event-modifiers (aref key 1)))))
! 		(read-event))
  	   (list
  	    key
  	    (if current-prefix-arg (prefix-numeric-value current-prefix-arg))
***************
*** 582,626 ****
  	 (fset 'yank-menu (cons 'keymap yank-menu))))))
    (if (numberp untranslated)
        (setq untranslated (this-single-command-raw-keys)))
!   (save-excursion
!     (let ((modifiers (event-modifiers (aref key 0)))
! 	  (standard-output (if insert (current-buffer) t))
! 	  window position)
!       ;; For a mouse button event, go to the button it applies to
!       ;; to get the right key bindings.  And go to the right place
!       ;; in case the keymap depends on where you clicked.
!       (if (or (memq 'click modifiers) (memq 'down modifiers)
! 	      (memq 'drag modifiers))
! 	  (setq window (posn-window (event-start (aref key 0)))
! 		position (posn-point (event-start (aref key 0)))))
!       (if (windowp window)
! 	  (progn
! 	    (set-buffer (window-buffer window))
! 	    (goto-char position)))
!       ;; Ok, now look up the key and name the command.
!       (let ((defn (key-binding key t))
! 	    key-desc)
! 	;; Handle the case where we faked an entry in "Select and Paste" menu.
! 	(if (and (eq defn nil)
! 		 (stringp (aref key (1- (length key))))
! 		 (eq (key-binding (substring key 0 -1)) 'yank-menu))
! 	    (setq defn 'menu-bar-select-yank))
! 	;; Don't bother user with strings from (e.g.) the select-paste menu.
! 	(if (stringp (aref key (1- (length key))))
! 	    (aset key (1- (length key)) "(any string)"))
! 	(if (and (> (length untranslated) 0)
! 		 (stringp (aref untranslated (1- (length untranslated)))))
! 	    (aset untranslated (1- (length untranslated))
! 		  "(any string)"))
! 	;; Now describe the key, perhaps as changed.
! 	(setq key-desc (help-key-description key untranslated))
! 	(if (or (null defn) (integerp defn) (equal defn 'undefined))
! 	    (princ (format "%s is undefined" key-desc))
! 	  (princ (format (if (windowp window)
! 			     "%s at that spot runs the command %s"
! 			   "%s runs the command %s")
! 			 key-desc
! 			 (if (symbolp defn) defn (prin1-to-string defn)))))))))
  
  (defun describe-key (&optional key untranslated up-event)
    "Display documentation of the function invoked by KEY.
--- 587,626 ----
  	 (fset 'yank-menu (cons 'keymap yank-menu))))))
    (if (numberp untranslated)
        (setq untranslated (this-single-command-raw-keys)))
!   (let* ((event (if (and (symbolp (aref key 0))
! 			 (> (length key) 1)
! 			 (consp (aref key 1)))
! 		    (aref key 1)
! 		  (aref key 0)))
! 	 (modifiers (event-modifiers event))
! 	 (standard-output (if insert (current-buffer) t))
! 	 (mousep
! 	  (or (memq 'click modifiers) (memq 'down modifiers)
! 	      (memq 'drag modifiers))))
!     ;; Ok, now look up the key and name the command.
!     (let ((defn (key-binding key t))
! 	  key-desc)
!       ;; Handle the case where we faked an entry in "Select and Paste" menu.
!       (if (and (eq defn nil)
! 	       (stringp (aref key (1- (length key))))
! 	       (eq (key-binding (substring key 0 -1)) 'yank-menu))
! 	  (setq defn 'menu-bar-select-yank))
!       ;; Don't bother user with strings from (e.g.) the select-paste menu.
!       (if (stringp (aref key (1- (length key))))
! 	  (aset key (1- (length key)) "(any string)"))
!       (if (and (> (length untranslated) 0)
! 	       (stringp (aref untranslated (1- (length untranslated)))))
! 	  (aset untranslated (1- (length untranslated))
! 		"(any string)"))
!       ;; Now describe the key, perhaps as changed.
!       (setq key-desc (help-key-description key untranslated))
!       (if (or (null defn) (integerp defn) (equal defn 'undefined))
! 	  (princ (format "%s is undefined" key-desc))
! 	(princ (format (if mousep
! 			   "%s at that spot runs the command %s"
! 			 "%s runs the command %s")
! 		       key-desc
! 		       (if (symbolp defn) defn (prin1-to-string defn))))))))
  
  (defun describe-key (&optional key untranslated up-event)
    "Display documentation of the function invoked by KEY.
***************
*** 652,756 ****
  	    (prefix-numeric-value current-prefix-arg)
  	    ;; If KEY is a down-event, read the corresponding up-event
  	    ;; and use it as the third argument.
! 	    (if (and (vectorp key)
! 		     (eventp (elt key 0))
! 		     (memq 'down (event-modifiers (elt key 0))))
! 		(read-event))))
         ;; Put yank-menu back as it was, if we changed it.
         (when saved-yank-menu
  	 (setq yank-menu (copy-sequence saved-yank-menu))
  	 (fset 'yank-menu (cons 'keymap yank-menu))))))
    (if (numberp untranslated)
        (setq untranslated (this-single-command-raw-keys)))
!   (save-excursion
!     (let ((modifiers (event-modifiers (aref key 0)))
! 	  window position)
!       ;; For a mouse button event, go to the button it applies to
!       ;; to get the right key bindings.  And go to the right place
!       ;; in case the keymap depends on where you clicked.
!       (if (or (memq 'click modifiers) (memq 'down modifiers)
! 	      (memq 'drag modifiers))
! 	  (setq window (posn-window (event-start (aref key 0)))
! 		position (posn-point (event-start (aref key 0)))))
!       (when (windowp window)
! 	    (set-buffer (window-buffer window))
! 	(goto-char position))
!       (let ((defn (key-binding key t)))
! 	;; Handle the case where we faked an entry in "Select and Paste" menu.
! 	(if (and (eq defn nil)
! 		 (stringp (aref key (1- (length key))))
! 		 (eq (key-binding (substring key 0 -1)) 'yank-menu))
! 	    (setq defn 'menu-bar-select-yank))
! 	(if (or (null defn) (integerp defn) (equal defn 'undefined))
! 	    (message "%s is undefined" (help-key-description key untranslated))
! 	  (help-setup-xref (list #'describe-function defn) (interactive-p))
! 	  ;; Don't bother user with strings from (e.g.) the select-paste menu.
! 	  (if (stringp (aref key (1- (length key))))
! 	      (aset key (1- (length key)) "(any string)"))
! 	  (if (and untranslated
! 		   (stringp (aref untranslated (1- (length untranslated)))))
! 	      (aset untranslated (1- (length untranslated))
! 		    "(any string)"))
! 	  (with-output-to-temp-buffer (help-buffer)
! 	    (princ (help-key-description key untranslated))
! 	    (if (windowp window)
! 		(princ " at that spot"))
! 	    (princ " runs the command ")
! 	    (prin1 defn)
! 	    (princ "\n   which is ")
! 	    (describe-function-1 defn)
! 	    (when up-event
! 	      (let ((type (event-basic-type up-event))
! 		    (hdr "\n\n-------------- up event ---------------\n\n")
! 		    defn sequence
! 		    mouse-1-tricky mouse-1-remapped)
! 		(setq sequence (vector up-event))
! 		(when (and (eq type 'mouse-1)
! 			   (windowp window)
! 			   mouse-1-click-follows-link
! 			   (not (eq mouse-1-click-follows-link 'double))
! 			   (setq mouse-1-remapped
! 				 (with-current-buffer (window-buffer window)
! 				   (mouse-on-link-p (posn-point
! 						     (event-start up-event))))))
! 		  (setq mouse-1-tricky (and (integerp mouse-1-click-follows-link)
! 					    (> mouse-1-click-follows-link 0)))
! 		  (cond ((stringp mouse-1-remapped)
! 			 (setq sequence mouse-1-remapped))
! 			((vectorp mouse-1-remapped)
! 			 (setcar up-event (elt mouse-1-remapped 0)))
! 			(t (setcar up-event 'mouse-2))))
! 		(setq defn (key-binding sequence))
! 		(unless (or (null defn) (integerp defn) (equal defn 'undefined))
! 		  (princ (if mouse-1-tricky
! 			     "\n\n----------------- up-event (short click) ----------------\n\n"
! 			   hdr))
! 		  (setq hdr nil)
! 		  (princ (symbol-name type))
! 		  (if (windowp window)
  		      (princ " at that spot"))
! 		  (if mouse-1-remapped
! 		      (princ " is remapped to <mouse-2>\n  which" ))
  		  (princ " runs the command ")
  		  (prin1 defn)
  		  (princ "\n   which is ")
! 		  (describe-function-1 defn))
! 		(when mouse-1-tricky
! 		  (setcar up-event 'mouse-1)
! 		  (setq defn (key-binding (vector up-event)))
! 		  (unless (or (null defn) (integerp defn) (eq defn 'undefined))
! 		    (princ (or hdr
! 			       "\n\n----------------- up-event (long click) ----------------\n\n"))
! 		    (princ "Pressing mouse-1")
! 		    (if (windowp window)
! 			(princ " at that spot"))
! 		    (princ (format " for longer than %d milli-seconds\n"
! 				   mouse-1-click-follows-link))
! 		    (princ " runs the command ")
! 		    (prin1 defn)
! 		    (princ "\n   which is ")
! 		    (describe-function-1 defn)))))
! 	    (print-help-return-message)))))))
  \f
  (defun describe-mode (&optional buffer)
    "Display documentation of current major mode and minor modes.
--- 652,755 ----
  	    (prefix-numeric-value current-prefix-arg)
  	    ;; If KEY is a down-event, read the corresponding up-event
  	    ;; and use it as the third argument.
! 	    (and (vectorp key)
! 		 (or (and (eventp (aref key 0))
! 			  (memq 'down (event-modifiers (aref key 0))))
! 		     (and (> (length key) 1)
! 			  (eventp (aref key 1))
! 			  (memq 'down (event-modifiers (aref key 1)))))
! 		 (read-event))))
         ;; Put yank-menu back as it was, if we changed it.
         (when saved-yank-menu
  	 (setq yank-menu (copy-sequence saved-yank-menu))
  	 (fset 'yank-menu (cons 'keymap yank-menu))))))
    (if (numberp untranslated)
        (setq untranslated (this-single-command-raw-keys)))
!   (let* ((event (if (and (symbolp (aref key 0))
! 			 (> (length key) 1)
! 			 (consp (aref key 1)))
! 		    (aref key 1)
! 		  (aref key 0)))
! 	 (modifiers (event-modifiers event))
! 	 (mousep
! 	  (or (memq 'click modifiers) (memq 'down modifiers)
! 	      (memq 'drag modifiers))))
!     ;; Ok, now look up the key and name the command.
! 
!     (let ((defn (key-binding key t)))
!       ;; Handle the case where we faked an entry in "Select and Paste" menu.
!       (if (and (eq defn nil)
! 	       (stringp (aref key (1- (length key))))
! 	       (eq (key-binding (substring key 0 -1)) 'yank-menu))
! 	  (setq defn 'menu-bar-select-yank))
!       (if (or (null defn) (integerp defn) (equal defn 'undefined))
! 	  (message "%s is undefined" (help-key-description key untranslated))
! 	(help-setup-xref (list #'describe-function defn) (interactive-p))
! 	;; Don't bother user with strings from (e.g.) the select-paste menu.
! 	(if (stringp (aref key (1- (length key))))
! 	    (aset key (1- (length key)) "(any string)"))
! 	(if (and untranslated
! 		 (stringp (aref untranslated (1- (length untranslated)))))
! 	    (aset untranslated (1- (length untranslated))
! 		  "(any string)"))
! 	(with-output-to-temp-buffer (help-buffer)
! 	  (princ (help-key-description key untranslated))
! 	  (if mousep
! 	      (princ " at that spot"))
! 	  (princ " runs the command ")
! 	  (prin1 defn)
! 	  (princ "\n   which is ")
! 	  (describe-function-1 defn)
! 	  (when up-event
! 	    (let ((type (event-basic-type up-event))
! 		  (hdr "\n\n-------------- up event ---------------\n\n")
! 		  defn sequence
! 		  mouse-1-tricky mouse-1-remapped)
! 	      (setq sequence (vector up-event))
! 	      (when (and (eq type 'mouse-1)
! 			 mouse-1-click-follows-link
! 			 (not (eq mouse-1-click-follows-link 'double))
! 			 (setq mouse-1-remapped
! 			       (mouse-on-link-p (event-start up-event))))
! 		(setq mouse-1-tricky (and (integerp mouse-1-click-follows-link)
! 					  (> mouse-1-click-follows-link 0)))
! 		(cond ((stringp mouse-1-remapped)
! 		       (setq sequence mouse-1-remapped))
! 		      ((vectorp mouse-1-remapped)
! 		       (setcar up-event (elt mouse-1-remapped 0)))
! 		      (t (setcar up-event 'mouse-2))))
! 	      (setq defn (key-binding sequence nil nil (event-start up-event)))
! 	      (unless (or (null defn) (integerp defn) (equal defn 'undefined))
! 		(princ (if mouse-1-tricky
! 			   "\n\n----------------- up-event (short click) ----------------\n\n"
! 			 hdr))
! 		(setq hdr nil)
! 		(princ (symbol-name type))
! 		(if mousep
! 		    (princ " at that spot"))
! 		(if mouse-1-remapped
! 		    (princ " is remapped to <mouse-2>\n  which" ))
! 		(princ " runs the command ")
! 		(prin1 defn)
! 		(princ "\n   which is ")
! 		(describe-function-1 defn))
! 	      (when mouse-1-tricky
! 		(setcar up-event 'mouse-1)
! 		(setq defn (key-binding (vector up-event) nil nil
! 					(event-start up-event)))
! 		(unless (or (null defn) (integerp defn) (eq defn 'undefined))
! 		  (princ (or hdr
! 			     "\n\n----------------- up-event (long click) ----------------\n\n"))
! 		  (princ "Pressing mouse-1")
! 		  (if mousep
  		      (princ " at that spot"))
! 		  (princ (format " for longer than %d milli-seconds\n"
! 				 mouse-1-click-follows-link))
  		  (princ " runs the command ")
  		  (prin1 defn)
  		  (princ "\n   which is ")
! 		  (describe-function-1 defn)))))
! 	  (print-help-return-message))))))
  \f
  (defun describe-mode (&optional buffer)
    "Display documentation of current major mode and minor modes.
Index: lisp/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/lisp/ChangeLog,v
retrieving revision 1.10062
diff -u -r1.10062 ChangeLog
*** lisp/ChangeLog	14 Sep 2006 17:52:07 -0000	1.10062
--- lisp/ChangeLog	14 Sep 2006 22:23:44 -0000
***************
*** 1,4 ****
! 2006-09-005  Ken Manheimer  <address@hidden>
  
  	* allout.el (allout-regexp, allout-line-boundary-regexp)
  	(allout-bob-regexp): Correct grouping and boundaries to fix
--- 1,22 ----
! 2006-09-15  David Kastrup  <dak@gnu.org>
! 
! 	* mouse-sel.el (mouse-sel-follow-link-p): Use event position
! 	instead of buffer position for `mouse-on-link-p'.
! 
! 	* mouse.el (mouse-posn-property): New function looking up the
! 	properties at a click position in overlays and text properties in
! 	either buffer or strings.
! 	(mouse-on-link-p): Use `mouse-posn-property' to streamline lookup
! 	of both `follow-link' as well as `mouse-face' properties.
! 	(mouse-drag-track): Check `mouse-on-link-p' on event position, not
! 	buffer position.
! 
! 	* help.el (describe-key-briefly): When reading a down-event on
! 	mode lines or scroll bar, swallow the following up event, too.
! 	Use the new mouse sensitity of `key-binding' for lookup.
! 	(describe-key): The same here.
! 
! 2006-09-05  Ken Manheimer  <address@hidden>
  
  	* allout.el (allout-regexp, allout-line-boundary-regexp)
  	(allout-bob-regexp): Correct grouping and boundaries to fix

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-14 11:57                             ` David Kastrup
  2006-09-14 22:34                               ` David Kastrup
@ 2006-09-15  3:14                               ` Richard Stallman
  2006-09-15  8:20                                 ` David Kastrup
  2006-09-15 23:47                               ` David Kastrup
  2 siblings, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-15  3:14 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

Please install it.  However, you will need to get rid of the first hunk
for keymaps.texi; your change showed me that the pseudo-program
for searching the keymaps was duplicated, so I deleted one copy.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-15  3:14                               ` Richard Stallman
@ 2006-09-15  8:20                                 ` David Kastrup
  0 siblings, 0 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-15  8:20 UTC (permalink / raw)
  Cc: cyd, storm, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> Please install it.  However, you will need to get rid of the first hunk
> for keymaps.texi; your change showed me that the pseudo-program
> for searching the keymaps was duplicated, so I deleted one copy.

Done.  I'll be checking in the changes to help.el, mouse.el and
mouse-sel.el presently.  They don't introduce new features, but use
the new key-binding stuff instead of a thicket of ad-hoc code in order
to get reliable results, so they are actually bugfixes.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-14 11:57                             ` David Kastrup
  2006-09-14 22:34                               ` David Kastrup
  2006-09-15  3:14                               ` Richard Stallman
@ 2006-09-15 23:47                               ` David Kastrup
  2006-09-16  0:14                                 ` Kim F. Storm
  2006-09-16 16:41                                 ` Stefan Monnier
  2 siblings, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-15 23:47 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

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

David Kastrup <dak@gnu.org> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>> Ok, I agree to adding the LOCATION arg.  But it needs to be documented.
>
> Here is the latest patch iteration.  I think it should be fit for
> checking in.  It might also seem reasonable to add the optional
> `position' argument to `current-active-maps', but this has not been
> done.

Here is a patch that would achieve that.  After scanning through
keymap.c, I believe that this should be the last change required to
provide APIs to the keymap access on overlays and display strings,
thus making manual simulation of keymap search order for whatever
purpose unnecessary.  At a cursory glance through callers, I don't see
code within Emacs itself which would really require this at the
current point of time.  I remember some stuff by now implemented with
unread-command-events that went through hoops in order to repeat a key
lookup with one keymap excluded.

This kind of thing would be more reliably achieved by using
`current-active-maps' with a position argument, removing the
non-needed keymap from the list, and repeating the lookup.

While I don't think we should change any of the existing callers right
now, it might be a good idea to have this API available in future.
The implementation is reasonably simple.  I don't think a GCPRO is
needed here, but I can't say that I know garbage collection inside
out.

If there is agreement to put this in in order to complete the accessor
functions to mouse-dependent keymaps, I would add this to the manual
and NEWS file, too.

What do you think?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 8661 bytes --]

Index: src/keymap.h
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.h,v
retrieving revision 1.15
diff -u -r1.15 keymap.h
*** src/keymap.h	15 Sep 2006 07:19:14 -0000	1.15
--- src/keymap.h	15 Sep 2006 23:46:07 -0000
***************
*** 34,40 ****
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);
! EXFUN (Fcurrent_active_maps, 1);
  extern Lisp_Object access_keymap P_ ((Lisp_Object, Lisp_Object, int, int, int));
  extern Lisp_Object get_keyelt P_ ((Lisp_Object, int));
  extern Lisp_Object get_keymap P_ ((Lisp_Object, int, int));
--- 34,40 ----
  EXFUN (Fkey_description, 2);
  EXFUN (Fsingle_key_description, 2);
  EXFUN (Fwhere_is_internal, 5);
! EXFUN (Fcurrent_active_maps, 2);
  extern Lisp_Object access_keymap P_ ((Lisp_Object, Lisp_Object, int, int, int));
  extern Lisp_Object get_keyelt P_ ((Lisp_Object, int));
  extern Lisp_Object get_keymap P_ ((Lisp_Object, int, int));
Index: src/keymap.c
===================================================================
RCS file: /sources/emacs/emacs/src/keymap.c,v
retrieving revision 1.334
diff -u -r1.334 keymap.c
*** src/keymap.c	15 Sep 2006 07:19:14 -0000	1.334
--- src/keymap.c	15 Sep 2006 23:46:10 -0000
***************
*** 1510,1523 ****
  }
  
  DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps,
!        0, 1, 0,
         doc: /* Return a list of the currently active keymaps.
  OLP if non-nil indicates that we should obey `overriding-local-map' and
! `overriding-terminal-local-map'.  */)
!      (olp)
!      Lisp_Object olp;
  {
!   Lisp_Object keymaps = Fcons (current_global_map, Qnil);
  
    if (!NILP (olp))
      {
--- 1510,1556 ----
  }
  
  DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps,
!        0, 2, 0,
         doc: /* Return a list of the currently active keymaps.
  OLP if non-nil indicates that we should obey `overriding-local-map' and
! `overriding-terminal-local-map'.  POSITION can specify a click position
! like in the respective argument of `key-binding'. */)
!     (olp, position)
!     Lisp_Object olp, position;
  {
!   int count = SPECPDL_INDEX ();
! 
!   Lisp_Object keymaps;
! 
!   /* If a mouse click position is given, our variables are based on
!      the buffer clicked on, not the current buffer.  So we may have to
!      switch the buffer here. */
!   
!   if (CONSP (position))
!     {
!       Lisp_Object window;
!       
!       window = POSN_WINDOW (position);
! 	  
!       if (WINDOWP (window)
! 	  && BUFFERP (XWINDOW (window)->buffer)
! 	  && XBUFFER (XWINDOW (window)->buffer) != current_buffer)
! 	{
! 	  /* Arrange to go back to the original buffer once we're done
! 	     processing the key sequence.  We don't use
! 	     save_excursion_{save,restore} here, in analogy to
! 	     `read-key-sequence' to avoid saving point.  Maybe this
! 	     would not be a problem here, but it is easier to keep
! 	     things the same.
! 	  */
! 	      
! 	  record_unwind_protect (Fset_buffer, Fcurrent_buffer ());
! 	  
! 	  set_buffer_internal (XBUFFER (XWINDOW (window)->buffer));
! 	}
!     }
! 
!   keymaps = Fcons (current_global_map, Qnil);  
  
    if (!NILP (olp))
      {
***************
*** 1531,1545 ****
      }
    if (NILP (XCDR (keymaps)))
      {
-       Lisp_Object local;
        Lisp_Object *maps;
        int nmaps, i;
  
!       /* This usually returns the buffer's local map,
! 	 but that can be overridden by a `local-map' property.  */
!       local = get_local_map (PT, current_buffer, Qlocal_map);
!       if (!NILP (local))
! 	keymaps = Fcons (local, keymaps);
  
        /* Now put all the minor mode keymaps on the list.  */
        nmaps = current_minor_maps (0, &maps);
--- 1564,1640 ----
      }
    if (NILP (XCDR (keymaps)))
      {
        Lisp_Object *maps;
        int nmaps, i;
  
!       Lisp_Object keymap, local_map;
!       EMACS_INT pt;
! 
!       pt = INTEGERP (position) ? XINT (position)
! 	: MARKERP (position) ? marker_position (position)
! 	: PT;
! 
!       /* Get the buffer local maps, possibly overriden by text or
! 	 overlay properties */
! 
!       local_map = get_local_map (pt, current_buffer, Qlocal_map); 
!       keymap = get_local_map (pt, current_buffer, Qkeymap); 
! 
!       if (CONSP (position))
! 	{
! 	  Lisp_Object string, window;
! 
! 	  window = POSN_WINDOW (position);
! 
! 	  /* For a mouse click, get the local text-property keymap
! 	     of the place clicked on, rather than point.  */
! 	  
! 	  if (POSN_INBUFFER_P (position))
! 	    {
! 	      Lisp_Object pos;
! 
! 	      pos = POSN_BUFFER_POSN (position);
! 	      if (INTEGERP (pos)
! 		  && XINT (pos) >= BEG && XINT (pos) <= Z)
! 		{
! 		  local_map = get_local_map (XINT (pos),
! 					     current_buffer, Qlocal_map);
! 		  
! 		  keymap = get_local_map (XINT (pos),
! 					  current_buffer, Qkeymap);
! 		}
! 	    }
! 
! 	  /* If on a mode line string with a local keymap,
! 	     or for a click on a string, i.e. overlay string or a
! 	     string displayed via the `display' property,
! 	     consider `local-map' and `keymap' properties of
! 	     that string.  */
! 	  
! 	  if (string = POSN_STRING (position),
! 	      (CONSP (string) && STRINGP (XCAR (string))))
! 	    {
! 	      Lisp_Object pos, map;
! 	      
! 	      pos = XCDR (string);
! 	      string = XCAR (string);
! 	      if (XINT (pos) >= 0
! 		  && XINT (pos) < SCHARS (string))
! 		{
! 		  map = Fget_text_property (pos, Qlocal_map, string);
! 		  if (!NILP (map))
! 		    local_map = map;
! 
! 		  map = Fget_text_property (pos, Qkeymap, string);
! 		  if (!NILP (map))
! 		    keymap = map;
! 		}
! 	    }
! 	  
! 	}
! 
!       if (!NILP (local_map))
! 	keymaps = Fcons (local_map, keymaps);
  
        /* Now put all the minor mode keymaps on the list.  */
        nmaps = current_minor_maps (0, &maps);
***************
*** 1548,1559 ****
  	if (!NILP (maps[i]))
  	  keymaps = Fcons (maps[i], keymaps);
  
!       /* This returns nil unless there is a `keymap' property.  */
!       local = get_local_map (PT, current_buffer, Qkeymap);
!       if (!NILP (local))
! 	keymaps = Fcons (local, keymaps);
      }
  
    return keymaps;
  }
  
--- 1643,1654 ----
  	if (!NILP (maps[i]))
  	  keymaps = Fcons (maps[i], keymaps);
  
!       if (!NILP (keymap))
! 	keymaps = Fcons (keymap, keymaps);
      }
  
+   unbind_to (count, Qnil);
+ 
    return keymaps;
  }
  
***************
*** 2805,2811 ****
    else if (!NILP (keymap))
      keymaps = Fcons (keymap, Fcons (current_global_map, Qnil));
    else
!     keymaps = Fcurrent_active_maps (Qnil);
  
    /* Only use caching for the menubar (i.e. called with (def nil t nil).
       We don't really need to check `keymap'.  */
--- 2900,2906 ----
    else if (!NILP (keymap))
      keymaps = Fcons (keymap, Fcons (current_global_map, Qnil));
    else
!     keymaps = Fcurrent_active_maps (Qnil, Qnil);
  
    /* Only use caching for the menubar (i.e. called with (def nil t nil).
       We don't really need to check `keymap'.  */
Index: src/doc.c
===================================================================
RCS file: /sources/emacs/emacs/src/doc.c,v
retrieving revision 1.120
diff -u -r1.120 doc.c
*** src/doc.c	18 Jul 2006 13:26:24 -0000	1.120
--- src/doc.c	15 Sep 2006 23:46:12 -0000
***************
*** 883,889 ****
  	  struct buffer *oldbuf;
  	  int start_idx;
  	  /* This is for computing the SHADOWS arg for describe_map_tree.  */
! 	  Lisp_Object active_maps = Fcurrent_active_maps (Qnil);
  	  Lisp_Object earlier_maps;
  
  	  changed = 1;
--- 883,889 ----
  	  struct buffer *oldbuf;
  	  int start_idx;
  	  /* This is for computing the SHADOWS arg for describe_map_tree.  */
! 	  Lisp_Object active_maps = Fcurrent_active_maps (Qnil, Qnil);
  	  Lisp_Object earlier_maps;
  
  	  changed = 1;
Index: src/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/src/ChangeLog,v
retrieving revision 1.5302
diff -u -r1.5302 ChangeLog
*** src/ChangeLog	15 Sep 2006 21:01:03 -0000	1.5302
--- src/ChangeLog	15 Sep 2006 23:46:40 -0000
***************
*** 1,3 ****
--- 1,14 ----
+ 2006-09-16  David Kastrup  <dak@gnu.org>
+ 
+ 	* keymap.c (Fcurrent_active_maps): Add `position' argument.
+ 	(Fwhere_is_internal): Adjust call to `current-active-maps' to
+ 	cater for additional parameter.
+ 
+ 	* keymap.h: Adjust number of parameters to `current-active-maps'.
+ 
+ 	* doc.c (Fsubstitute_command_keys): Adjust call of
+ 	`current-active-maps'.
+ 
  2006-09-15  Kim F. Storm  <storm@cua.dk>
  
  	* window.c (Fwindow_line_visibility): New defun for line-move-partial.

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-15 23:47                               ` David Kastrup
@ 2006-09-16  0:14                                 ` Kim F. Storm
  2006-09-16 19:05                                   ` Richard Stallman
  2006-09-16 16:41                                 ` Stefan Monnier
  1 sibling, 1 reply; 55+ messages in thread
From: Kim F. Storm @ 2006-09-16  0:14 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> If there is agreement to put this in in order to complete the accessor
> functions to mouse-dependent keymaps, I would add this to the manual
> and NEWS file, too.
>
> What do you think?

I agree that it seems logical to include this to complement the
changes to key-binding and command-remapping.  

OTOH, I'm not convinced that we really need this (now or later).

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-15 23:47                               ` David Kastrup
  2006-09-16  0:14                                 ` Kim F. Storm
@ 2006-09-16 16:41                                 ` Stefan Monnier
  1 sibling, 0 replies; 55+ messages in thread
From: Stefan Monnier @ 2006-09-16 16:41 UTC (permalink / raw)
  Cc: cyd, storm, rms, emacs-devel

> If there is agreement to put this in in order to complete the accessor
> functions to mouse-dependent keymaps, I would add this to the manual
> and NEWS file, too.

> What do you think?

Agreed,


        Stefan "whose local definition of `key-binding' is simply
                (lookup-key (current-active-keymaps) key)"

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-16  0:14                                 ` Kim F. Storm
@ 2006-09-16 19:05                                   ` Richard Stallman
  2006-09-18 15:43                                     ` David Kastrup
  0 siblings, 1 reply; 55+ messages in thread
From: Richard Stallman @ 2006-09-16 19:05 UTC (permalink / raw)
  Cc: cyd, emacs-devel

    I agree that it seems logical to include this to complement the
    changes to key-binding and command-remapping.  

    OTOH, I'm not convinced that we really need this (now or later).

That is what I feel.  Let's put it off.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-16 19:05                                   ` Richard Stallman
@ 2006-09-18 15:43                                     ` David Kastrup
  2006-09-18 20:34                                       ` Kim F. Storm
  2006-09-18 23:39                                       ` Richard Stallman
  0 siblings, 2 replies; 55+ messages in thread
From: David Kastrup @ 2006-09-18 15:43 UTC (permalink / raw)
  Cc: cyd, emacs-devel, Kim F. Storm

Richard Stallman <rms@gnu.org> writes:

>     I agree that it seems logical to include this to complement the
>     changes to key-binding and command-remapping.  
>
>     OTOH, I'm not convinced that we really need this (now or later).
>
> That is what I feel.  Let's put it off.

Well, I proposed it because it is rather easy to do, does not change
the behavior of the old function unless the additional argument is
used (and thus can't introduce a bug unless a user chooses to exercise
this feature), and because there is otherwise an omission in the APIs.

I'd rather have people use this in case they actually need it than
fabricate workarounds.

I agree that there does not appear a particular need for existing
applications that I know.

Should I keep this around for putting it in after the release?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-18 15:43                                     ` David Kastrup
@ 2006-09-18 20:34                                       ` Kim F. Storm
  2006-09-18 23:39                                       ` Richard Stallman
  1 sibling, 0 replies; 55+ messages in thread
From: Kim F. Storm @ 2006-09-18 20:34 UTC (permalink / raw)
  Cc: cyd, rms, emacs-devel

David Kastrup <dak@gnu.org> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>>     I agree that it seems logical to include this to complement the
>>     changes to key-binding and command-remapping.  
>>
>>     OTOH, I'm not convinced that we really need this (now or later).
>>
>> That is what I feel.  Let's put it off.
>
> Well, I proposed it because it is rather easy to do, does not change
> the behavior of the old function unless the additional argument is
> used (and thus can't introduce a bug unless a user chooses to exercise
> this feature), and because there is otherwise an omission in the APIs.
>
> I'd rather have people use this in case they actually need it than
> fabricate workarounds.
>
> I agree that there does not appear a particular need for existing
> applications that I know.
>
> Should I keep this around for putting it in after the release?

IMO, Yes.

The problem I see with installing it now is that it will not
get any testing before the release.

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

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: local keymap patch for key-binding
  2006-09-18 15:43                                     ` David Kastrup
  2006-09-18 20:34                                       ` Kim F. Storm
@ 2006-09-18 23:39                                       ` Richard Stallman
  1 sibling, 0 replies; 55+ messages in thread
From: Richard Stallman @ 2006-09-18 23:39 UTC (permalink / raw)
  Cc: cyd, emacs-devel, storm

    Should I keep this around for putting it in after the release?

Please do.

^ permalink raw reply	[flat|nested] 55+ messages in thread

end of thread, other threads:[~2006-09-18 23:39 UTC | newest]

Thread overview: 55+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-10  9:34 longlines-mode doesn't seem to work with some non-english text Miles Bader
2006-02-11  4:35 ` Chong Yidong
2006-02-13  0:32   ` Kevin Ryde
2006-09-09 15:08 ` local keymap patch for key-binding Chong Yidong
2006-09-09 15:15   ` David Kastrup
2006-09-09 15:26     ` Chong Yidong
2006-09-10 12:44     ` Chong Yidong
2006-09-10 13:25       ` David Kastrup
2006-09-11  2:36         ` Chong Yidong
2006-09-11  6:25           ` David Kastrup
2006-09-11  7:05           ` David Kastrup
2006-09-11  8:52             ` Kim F. Storm
2006-09-11  9:48               ` David Kastrup
2006-09-11 10:11                 ` David Kastrup
2006-09-11 12:53                   ` Kim F. Storm
2006-09-11 13:04                     ` David Kastrup
2006-09-11 13:07                   ` Chong Yidong
2006-09-11 19:58                 ` Richard Stallman
2006-09-12 10:04                   ` David Kastrup
2006-09-12 15:21                     ` David Kastrup
2006-09-12 15:39                       ` PCL-CVS's diff and marks (was: local keymap patch for key-binding) Stefan Monnier
2006-09-12 15:42                         ` PCL-CVS's diff and marks David Kastrup
2006-09-12 15:54                           ` Stefan Monnier
2006-09-12 21:45                       ` local keymap patch for key-binding Richard Stallman
2006-09-12 22:23                         ` David Kastrup
2006-09-12 23:44                           ` David Kastrup
2006-09-13  7:45                             ` Kim F. Storm
2006-09-13  8:11                               ` David Kastrup
2006-09-13 11:05                                 ` Kim F. Storm
2006-09-13 11:38                                   ` David Kastrup
2006-09-13 12:23                                     ` Kim F. Storm
2006-09-13 12:32                                       ` David Kastrup
2006-09-13 13:45                                         ` Kim F. Storm
2006-09-13 19:25                                     ` Richard Stallman
2006-09-13 19:49                                       ` David Kastrup
2006-09-13 19:25                                 ` Richard Stallman
2006-09-13 20:02                                   ` David Kastrup
2006-09-13 19:24                           ` Richard Stallman
2006-09-13 15:10                       ` Richard Stallman
2006-09-13 15:25                         ` David Kastrup
2006-09-14  2:34                           ` Richard Stallman
2006-09-14 11:57                             ` David Kastrup
2006-09-14 22:34                               ` David Kastrup
2006-09-14 22:36                                 ` David Kastrup
2006-09-15  3:14                               ` Richard Stallman
2006-09-15  8:20                                 ` David Kastrup
2006-09-15 23:47                               ` David Kastrup
2006-09-16  0:14                                 ` Kim F. Storm
2006-09-16 19:05                                   ` Richard Stallman
2006-09-18 15:43                                     ` David Kastrup
2006-09-18 20:34                                       ` Kim F. Storm
2006-09-18 23:39                                       ` Richard Stallman
2006-09-16 16:41                                 ` Stefan Monnier
2006-09-10 18:52       ` Richard Stallman
2006-09-11  2:39         ` Chong Yidong

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).