Buffer menu fix

Buffer menu fix

[Chong Yidong]

This is regarding this FOR-RELEASE item:

   ** The header-line buttons in the buffer list buffer should respond
      to Mouse-1.

The following patch fixes the bug and simplifies the code.  (The `if'
condition that checks `Buffer-menu-use-header-line' in the old code is
not necessary, because it does no harm to bind some extra keys.)

If there are no objections over the next few days, I will check it in.



*** emacs/lisp/buff-menu.el.~1.89.~	2005-08-15 17:29:32.000000000 -0400
--- emacs/lisp/buff-menu.el	2005-09-04 15:11:22.000000000 -0400
***************
*** 645,666 ****
  			       "mouse-2: sort by visited order"
  			     "mouse-2, RET: sort by visited order"))
  	      'mouse-face 'highlight
! 	      'keymap (let ((map (make-sparse-keymap)))
! 			(if Buffer-menu-use-header-line
! 			    (define-key map [header-line mouse-2]
! 			      `(lambda (e)
! 				 (interactive "e")
! 				 (save-window-excursion
! 				   (if e (mouse-select-window e))
! 				   (Buffer-menu-sort ,column))))
! 			  (define-key map [mouse-2]
! 			    `(lambda (e)
! 			       (interactive "e")
! 			       (if e (mouse-select-window e))
! 			       (Buffer-menu-sort ,column)))
! 			  (define-key map "\C-m"
! 			    `(lambda () (interactive)
! 			       (Buffer-menu-sort ,column))))
  			map)))
  
  (defun list-buffers-noselect (&optional files-only buffer-list)
--- 645,663 ----
  			       "mouse-2: sort by visited order"
  			     "mouse-2, RET: sort by visited order"))
  	      'mouse-face 'highlight
! 	      'keymap (let ((map (make-sparse-keymap))
! 			    (fun `(lambda (e)
! 				    (interactive "e")
! 				    (if e (mouse-select-window e))
! 				    (Buffer-menu-sort ,column))))
! 			(define-key map [header-line mouse-1] fun)
! 			(define-key map [header-line mouse-2] fun)
! 			(define-key map [header-line down-mouse-1] 'ignore)
! 			(define-key map [mouse-2] fun)
! 			(define-key map [follow-link] 'mouse-face)
! 			(define-key map "\C-m"
! 			  `(lambda () (interactive)
! 			     (Buffer-menu-sort ,column)))
  			map)))
  
  (defun list-buffers-noselect (&optional files-only buffer-list)

Re: Buffer menu fix

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Sun, 04 Sep 2005 15:16:34 -0400
> 
> This is regarding this FOR-RELEASE item:
> 
>    ** The header-line buttons in the buffer list buffer should respond
>       to Mouse-1.
> 
> The following patch fixes the bug and simplifies the code.  (The `if'
> condition that checks `Buffer-menu-use-header-line' in the old code is
> not necessary, because it does no harm to bind some extra keys.)
> 
> If there are no objections over the next few days, I will check it in.

I realize that the original code had the same problem, but since we
are changing it: using lambda expressions to bind mouse clicks has a
negative side effect that "C-h c", "C-h k" and similar help commands,
when used with these mouse clicks, display test that to most users
sounds like nonsensical gibberish.

So may I suggest to define a real function, with a real doc string,
and then bind those clicks to that function?

Re: Buffer menu fix

[Richard M. Stallman]

    This is regarding this FOR-RELEASE item:

       ** The header-line buttons in the buffer list buffer should respond
	  to Mouse-1.

Thanks for taking care of it.  When you install the patch,
presuming no one objects, please delete the FOR-RELEASE item too.

Re: Buffer menu fix

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> I realize that the original code had the same problem, but since we
> are changing it: using lambda expressions to bind mouse clicks has a
> negative side effect that "C-h c", "C-h k" and similar help commands,
> when used with these mouse clicks, display test that to most users
> sounds like nonsensical gibberish.
>
> So may I suggest to define a real function, with a real doc string,
> and then bind those clicks to that function?

I can't see a clean way to do that, without putting in 8 function
definitions that vary from each other by only a couple characters.
Yuck.

Re: Buffer menu fix

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Mon, 05 Sep 2005 08:31:27 -0400
> 
> > So may I suggest to define a real function, with a real doc string,
> > and then bind those clicks to that function?
> 
> I can't see a clean way to do that, without putting in 8 function
> definitions that vary from each other by only a couple characters.
> Yuck.

Perhaps you could use a lambda expression or a macro inside a single
function.  Lisp is an interpreted language, as I'm sure you know.

Re: Buffer menu fix

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> > So may I suggest to define a real function, with a real doc string,
>> > and then bind those clicks to that function?
>> 
>> I can't see a clean way to do that, without putting in 8 function
>> definitions that vary from each other by only a couple characters.
>> Yuck.
>
> Perhaps you could use a lambda expression or a macro inside a single
> function.  Lisp is an interpreted language, as I'm sure you know.

The code in question goes like this:

(defun Buffer-menu-make-sort-button (column ...)
  ...
  (propertize ...
    'keymap (let ((map (make-sparse-keymap))
                  (fun `(lambda ()
                          (interactive)
                          (Buffer-menu-sort ,column))))
              (define-key map [header-line mouse-1] fun)
              ...))

When you bind a function to a key, you can't specify any additional
arguments to pass to that function.  So you have to define one
function for each of the possible values of `column' in the code.

The only way I can think of to get around this is to bind to a single
function that tries to re-construct the value of `column' based on
where the mouse was clicked.  But that seems like a strange thing to
do -- you're throwing away information that you had (i.e., the value
of `column'), only to do a lot of work find out what it was later on.

I'm not sure which approach is cleaner.

Re: Buffer menu fix

[Chong Yidong]

> The only way I can think of to get around this is to bind to a single
> function that tries to re-construct the value of `column' based on
> where the mouse was clicked.  But that seems like a strange thing to
> do -- you're throwing away information that you had (i.e., the value
> of `column'), only to do a lot of work find out what it was later on.

Actually, I'm not even sure how to do this -- the text in question is
located at the header line, so you can't use posn-at-point.  So it's
either (i) generate the keymap automatically using lambda functions,
or (ii) individually define eight nearly identical functions.

Buffer menu fix

[Nick Roberts]

Chong Yidong writes:
 > This is regarding this FOR-RELEASE item:
 > 
 >    ** The header-line buttons in the buffer list buffer should respond
 >       to Mouse-1.
 > 
 > The following patch fixes the bug and simplifies the code.  (The `if'
 > condition that checks `Buffer-menu-use-header-line' in the old code is
 > not necessary, because it does no harm to bind some extra keys.)
 > 
 > If there are no objections over the next few days, I will check it in.

 ...
 >   (defun list-buffers-noselect (&optional files-only buffer-list)
 > --- 645,663 ----
 >   			       "mouse-2: sort by visited order"
 >   			     "mouse-2, RET: sort by visited order"))
 >   	      'mouse-face 'highlight
 > ! 	      'keymap (let ((map (make-sparse-keymap))
 > ! 			    (fun `(lambda (e)
 > ! 				    (interactive "e")
 > ! 				    (if e (mouse-select-window e))
 > ! 				    (Buffer-menu-sort ,column))))
 > ! 			(define-key map [header-line mouse-1] fun)
 > ! 			(define-key map [header-line mouse-2] fun)
 > ! 			(define-key map [header-line down-mouse-1] 'ignore)
 > ! 			(define-key map [mouse-2] fun)
 > ! 			(define-key map [follow-link] 'mouse-face)
 > ! 			(define-key map "\C-m"
 > ! 			  `(lambda () (interactive)
 > ! 			     (Buffer-menu-sort ,column)))
 >   			map)))

With this patch header-line buttons in the buffer list buffer certainly
respond to Mouse-1 but unfortunately they do this even when
mouse-1-click-follows-link is nil.  This i because you have added the line

 > ! 			(define-key map [header-line mouse-1] fun)

I don't think

 > ! 			(define-key map [follow-link] 'mouse-face)

does anything in this case possibly because map here is a text property and
not associated with a mode.

It would be best to omit the line

 > ! 			(define-key map [header-line down-mouse-1] 'ignore)

because then the header line can then respond to mouse-1 clicks and still be
dragged (try it on the mode-line to see what I mean).

 > ! 			(define-key map "\C-m"
 > ! 			  `(lambda () (interactive)
 > ! 			     (Buffer-menu-sort ,column)))
 
I don't see the point of this as you can't place point on the header line.

In summary, I don't see how to solve the problem (neither did the original
author presumably), all I can do is pull your solution apart.

Sorry,

Nick

Re: Buffer menu fix

[Chong Yidong]

> With this patch header-line buttons in the buffer list buffer certainly
> respond to Mouse-1 but unfortunately they do this even when
> mouse-1-click-follows-link is nil.

This behavior is the same as Info-mode.  It makes sense because
mouse-1 on the header line would not set point, as it would in the
main editing area.

> I don't think
>
>  > ! 			(define-key map [follow-link] 'mouse-face)
>
> does anything in this case possibly because map here is a text property and
> not associated with a mode.

This deals with the case when Buffer-menu-use-header-line is nil.

> It would be best to omit the line
>
>  > ! 			(define-key map [header-line down-mouse-1] 'ignore)
>
> because then the header line can then respond to mouse-1 clicks and still be
> dragged (try it on the mode-line to see what I mean).

You can drag it by clicking on any part of the header line outside the
button text.  Again, this is the same as Info-mode.

>  > ! 			(define-key map "\C-m"
>  > ! 			  `(lambda () (interactive)
>  > ! 			     (Buffer-menu-sort ,column)))
>  
> I don't see the point of this as you can't place point on the header line.

This deals with the case when Buffer-menu-use-header-line is nil.

> In summary, I don't see how to solve the problem (neither did the original
> author presumably), all I can do is pull your solution apart.

With my code, clicking mouse-1 actually works, where previously it
didn't.  If there are any other problems with my solution, feel free
to point them out.

Re: Buffer menu fix

[Nick Roberts]

Chong Yidong writes:
 > > With this patch header-line buttons in the buffer list buffer certainly
 > > respond to Mouse-1 but unfortunately they do this even when
 > > mouse-1-click-follows-link is nil.
 > 
 > This behavior is the same as Info-mode.  It makes sense because
 > mouse-1 on the header line would not set point, as it would in the
 > main editing area.

Well it breaks the idiom of mouse-1-click-follows-link.
(mouse-1-click-follows-link-except-for-Info-and-Buffer-menu ?).  If this is
the desired behaviour perhaps a note could be added to the doc for
mouse-1-click-follows-link.

 > > I don't think
 > >
 > >  > ! 			(define-key map [follow-link] 'mouse-face)
 > >
 > > does anything in this case possibly because map here is a text property and
 > > not associated with a mode.
 > 
 > This deals with the case when Buffer-menu-use-header-line is nil.

OK, I understand now, perhaps there could be a comment.

 > > It would be best to omit the line
 > >
 > >  > ! 			(define-key map [header-line down-mouse-1] 'ignore)
 > >
 > > because then the header line can then respond to mouse-1 clicks and still be
 > > dragged (try it on the mode-line to see what I mean).
 > 
 > You can drag it by clicking on any part of the header line outside the
 > button text.  Again, this is the same as Info-mode.

In that case I suggest that this is also changed in Info-mode.


 > >  > ! 			(define-key map "\C-m"
 > >  > ! 			  `(lambda () (interactive)
 > >  > ! 			     (Buffer-menu-sort ,column)))
 > >  
 > > I don't see the point of this as you can't place point on the header line.
 > 
 > This deals with the case when Buffer-menu-use-header-line is nil.

I see.

 > > In summary, I don't see how to solve the problem (neither did the original
 > > author presumably), all I can do is pull your solution apart.
 > 
 > With my code, clicking mouse-1 actually works, where previously it
 > didn't.  If there are any other problems with my solution, feel free
 > to point them out.

Unlike follow-link, the tooltip shows the mouse-2 binding rather than mouse-1.

Nick

Re: Buffer menu fix

[asdf]

Nick Roberts <nickrob@snap.net.nz> writes:

>  > > With this patch header-line buttons in the buffer list buffer certainly
>  > > respond to Mouse-1 but unfortunately they do this even when
>  > > mouse-1-click-follows-link is nil.
>  > 
>  > This behavior is the same as Info-mode.  It makes sense because
>  > mouse-1 on the header line would not set point, as it would in the
>  > main editing area.
>
> Well it breaks the idiom of mouse-1-click-follows-link.
> (mouse-1-click-follows-link-except-for-Info-and-Buffer-menu ?).  If this is
> the desired behaviour perhaps a note could be added to the doc for
> mouse-1-click-follows-link.

Arguably, the mouse-1-click-follows-link (follow-link) idiom doesn't
apply here.  With the follow-link mechanism, if you hold down mouse-1
for about a second, it sets point where you clicked instead of
following the link.  This is not relevant here because you can't set
point on the header line.  (The Lisp code also wouldn't work --
setting just the 'follow-link property on a header line string does
nothing.)

>  > > because then the header line can then respond to mouse-1 clicks
>  > > and still be dragged (try it on the mode-line to see what I
>  > > mean).
>  > 
>  > You can drag it by clicking on any part of the header line outside the
>  > button text.  Again, this is the same as Info-mode.
>
> In that case I suggest that this is also changed in Info-mode.

Why? It is much more convenient to allow users to click on a link.  If
the user wants to move the header line (which is not something that
people do frequently, anyway), all she has to do is to move the mouse
two milimeters to the left, outside the button.  This is what people
are used to in all other graphical applications -- no one sees a
button and thinks, "Yeah, that's for resizing the window."

> Unlike follow-link, the tooltip shows the mouse-2 binding rather
> than mouse-1.

This is true.  (The tooltips in Info-mode also say mouse-2, by the
way.) I think they should be changed to say mouse-1, but could we get
a third opinion on this?

Re: Buffer menu fix

[Nick Roberts]

 > >  > > because then the header line can then respond to mouse-1 clicks
 > >  > > and still be dragged (try it on the mode-line to see what I
 > >  > > mean).
 > >  > 
 > >  > You can drag it by clicking on any part of the header line outside the
 > >  > button text.  Again, this is the same as Info-mode.
 > >
 > > In that case I suggest that this is also changed in Info-mode.
 > 
 > Why? It is much more convenient to allow users to click on a link.  If
 > the user wants to move the header line (which is not something that
 > people do frequently, anyway), all she has to do is to move the mouse
 > two milimeters to the left, outside the button.  This is what people
 > are used to in all other graphical applications -- no one sees a
 > button and thinks, "Yeah, that's for resizing the window."

The choice isn't between clicking or dragging, its between clicking or
clicking *and* dragging.  You're adding a line of code that just prevents the
header line from being dragged at that point.  It seems a bit pointless.

Nick

Re: Buffer menu fix

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Mon, 05 Sep 2005 18:18:18 -0400
> 
> (defun Buffer-menu-make-sort-button (column ...)
>   ...
>   (propertize ...
>     'keymap (let ((map (make-sparse-keymap))
>                   (fun `(lambda ()
>                           (interactive)
>                           (Buffer-menu-sort ,column))))
>               (define-key map [header-line mouse-1] fun)
>               ...))
> 
> When you bind a function to a key, you can't specify any additional
> arguments to pass to that function.  So you have to define one
> function for each of the possible values of `column' in the code.
> 
> The only way I can think of to get around this is to bind to a single
> function that tries to re-construct the value of `column' based on
> where the mouse was clicked.  But that seems like a strange thing to
> do -- you're throwing away information that you had (i.e., the value
> of `column'), only to do a lot of work find out what it was later on.

There must be some kind of misunderstanding here.  I understand that
you need to know where the mouse was clicked, but that information is
stored in the mouse event that invokes the function, so if you use
`(interactive "e")', you will have access to that information inside
the function.  Given this, what information is thrown away?

Re: Buffer menu fix

[Thien-Thi Nguyen]

Eli Zaretskii <eliz@gnu.org> writes:

> There must be some kind of misunderstanding here.  I understand that
> you need to know where the mouse was clicked, but that information is
> stored in the mouse event that invokes the function, so if you use
> `(interactive "e")', you will have access to that information inside
> the function.  Given this, what information is thrown away?

in this case, `column' is not in character units, but a small integer,
like 1, 2, 3.  for single-func approach (which is cleanest, IMHO), you
can put column-specific info that cannot be gleaned from the interactive
spec into a text property under a known-agreed-upon property name.  thus
the event carries location info and the location carries all the rest.

thi

Re: Buffer menu fix

[Tomas Zerolo]

[-- Attachment #1.1: Type: text/plain, Size: 194 bytes --]

Can't you attach a doc-string to an anonymous function, like so?

    `lambda (e)
      ,(concat "Sort buffer by " column)
      ...

(apologies if I am babbling)

regards

-- tomás

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

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

Re: Buffer menu fix

[Richard M. Stallman]

I don't think it is worth spending the time to have a long discussion
about whether to use a macro to define these commands, whether they
should be lambdas or have names, etc.

Does the current version of the patch actually work,
or is there a bug in it?

Re: Buffer menu fix

[Eli Zaretskii]

> From: "Richard M. Stallman" <rms@gnu.org>
> Date: Tue, 06 Sep 2005 07:22:59 -0400
> Cc: nickrob@snap.net.nz, emacs-devel@gnu.org
> 
> I don't think it is worth spending the time to have a long discussion
> about whether to use a macro to define these commands, whether they
> should be lambdas or have names, etc.

This discussion was not about whether to use a macro or a function, it
is about how to supply a doc string for a particular mouse click.  I
suggested to use a function as a means to that end, but I won't object
to a different solution as long as there's a meaningful string that
"C-h c" and "C-h k" can display for that click.

We had in the past complaints about mouse gestures for which Help
commands displayed technobabble understandable only by ELisp gurus.
We fixed those bindings by using functions.  Why add more causes for
users to complain about?

Re: Buffer menu fix

[Chong Yidong]

>  > Why? It is much more convenient to allow users to click on a link.  If
>  > the user wants to move the header line (which is not something that
>  > people do frequently, anyway), all she has to do is to move the mouse
>  > two milimeters to the left, outside the button.  This is what people
>  > are used to in all other graphical applications -- no one sees a
>  > button and thinks, "Yeah, that's for resizing the window."
>
> The choice isn't between clicking or dragging, its between clicking or
> clicking *and* dragging.  You're adding a line of code that just prevents the
> header line from being dragged at that point.  It seems a bit pointless.

Alright.  I've taken out the part inhibiting down-mouse-1 on the
header line.

As for not binding to lambda expressions, that will have to wait until
someone else comes up with a clean way to do it.  Since the patch
works, it's better than nothing, so I've checked it in.

Re: Buffer menu fix

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> When you bind a function to a key, you can't specify any additional
>> arguments to pass to that function.  So you have to define one
>> function for each of the possible values of `column' in the code.
>> 
>> The only way I can think of to get around this is to bind to a single
>> function that tries to re-construct the value of `column' based on
>> where the mouse was clicked.  But that seems like a strange thing to
>> do -- you're throwing away information that you had (i.e., the value
>> of `column'), only to do a lot of work find out what it was later on.
>
> There must be some kind of misunderstanding here.  I understand that
> you need to know where the mouse was clicked, but that information is
> stored in the mouse event that invokes the function, so if you use
> `(interactive "e")', you will have access to that information inside
> the function.  Given this, what information is thrown away?

When you're constructing the keymap, you know what the value of
`column' is that you want to pass to `(Buffer-menu-sort column)'.
Column=1 lists the buffer name, column=2 lists the buffer size, etc.
Your suggestion is to bind to a function, e.g.,

  (Buffer-menu-sort-click (e) (interactive "e") ...)

What this function has to do is reconstruct the value of `column' to
pass to `(Buffer-menu-sort column)' -- information that you've thrown
away, because all you know is the input event e.  To reconstruct
`column', you call (pos-row-col (event-start e)), which gives the col
(character column -- not the `column' we want).  Then you have to
figure out that `column=1' spans col X to col Y, `column=2' spans col
XX to col YY, etc.  Also, you have to substract some value from the
result of pos-row-col, depending on whether you are using a header
line or not, because the left fringe (if the fringe is activated) and
the scroll bar (if the scroll bar is on the left) may take up the
first few character columns on the header line.  It's a giant headache
to code.

Re: Buffer menu fix

[Kevin Rodgers]

Tomas Zerolo wrote:
 > Can't you attach a doc-string to an anonymous function, like so?
 >
 >     `lambda (e)
 >       ,(concat "Sort buffer by " column)
 >       ...

Obviously you can include a doc string in a lambda form.  The real
question is, will Emacs do the right thing with it?  It looks pretty
good to me in Emacs 21:

(documentation (lambda (foo)
                  "*Grok FOO."
                  (interactive "sGrok: ")
                  (grok foo)))
returns:
"*Grok FOO."

And given:
(global-set-key "\C-cz"
                 (lambda (foo)
                   "*Grok FOO."
                   (interactive "sGrok: ")
                   (grok foo)))

,----[ C-h k C-c z ]
| C-c z runs the command (lambda (foo) "*Grok FOO." (interactive "sGrok: 
") (grok foo))
|    which is an interactive Lisp function.
| (anonymous FOO)
|
| *Grok FOO.
`----

-- 
Kevin Rodgers

Re: Buffer menu fix

[Kim F. Storm]

Chong Yidong <cyd@stupidchicken.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> The only way I can think of to get around this is to bind to a single
>>> function that tries to re-construct the value of `column' based on
>>> where the mouse was clicked.  But that seems like a strange thing to
>>> do -- you're throwing away information that you had (i.e., the value
>>> of `column'), only to do a lot of work find out what it was later on.
> [...]
> It's a giant headache to code.

Not if you use the right approach.

Try this patch (not fully tested, but shows the principle):


*** buff-menu.el	06 Sep 2005 23:52:33 +0200	1.91
--- buff-menu.el	07 Sep 2005 00:57:01 +0200	
***************
*** 633,660 ****
  	    (insert m2)))
  	(forward-line)))))
  
  (defun Buffer-menu-make-sort-button (name column)
    (if (equal column Buffer-menu-sort-column) (setq column nil))
    (let* ((downname (downcase name))
!          (map (make-sparse-keymap))
!          (fun `(lambda (&optional e)
!                  ,(concat "Sort the buffer menu by " downname ".")
!                  (interactive (list last-input-event))
!                  (if e (mouse-select-window e))
!                  (Buffer-menu-sort ,column)))
!          (sym (intern (format "Buffer-menu-sort-by-%s-%s" name column))))
!     ;; Use a symbol rather than an anonymous function, to make the output of
!     ;; C-h k less intimidating.
!     (fset sym fun)
!     (setq fun sym)
      ;; This keymap handles both nil and non-nil
      ;; values for Buffer-menu-use-header-line.
!     (define-key map [header-line mouse-1] fun)
!     (define-key map [header-line mouse-2] fun)
!     (define-key map [mouse-2] fun)
      (define-key map [follow-link] 'mouse-face)
!     (define-key map "\C-m" fun)
      (propertize name
                  'help-echo (concat
                              (if Buffer-menu-use-header-line
                                  "mouse-1, mouse-2: sort by "
--- 633,658 ----
  	    (insert m2)))
  	(forward-line)))))
  
+ (defun Buffer-menu-sort-by-column (&optional e)
+   "Sort the buffer menu by this column."
+   (interactive "e")
+   (if e (mouse-select-window e))
+   (let ((obj (posn-object (event-start e))))
+     (Buffer-menu-sort (get-text-property (cdr obj) 'column (car obj)))))
+ 
  (defun Buffer-menu-make-sort-button (name column)
    (if (equal column Buffer-menu-sort-column) (setq column nil))
    (let* ((downname (downcase name))
!          (map (make-sparse-keymap)))
      ;; This keymap handles both nil and non-nil
      ;; values for Buffer-menu-use-header-line.
!     (define-key map [header-line mouse-1] 'Buffer-menu-sort-by-column)
!     (define-key map [header-line mouse-2] 'Buffer-menu-sort-by-column)
!     (define-key map [mouse-2] 'Buffer-menu-sort-by-column)
      (define-key map [follow-link] 'mouse-face)
!     (define-key map "\C-m" 'Buffer-menu-sort-by-column)
      (propertize name
+ 		'column column
                  'help-echo (concat
                              (if Buffer-menu-use-header-line
                                  "mouse-1, mouse-2: sort by "

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

Re: Buffer menu fix

[Chong Yidong]

> Not if you use the right approach.
>
> Try this patch (not fully tested, but shows the principle):

It works.  Very clever!

Re: Buffer menu fix

[Stefan Monnier]

>   (defun Buffer-menu-make-sort-button (name column)
>     (if (equal column Buffer-menu-sort-column) (setq column nil))
>     (let* ((downname (downcase name))
> !          (map (make-sparse-keymap)))
>       ;; This keymap handles both nil and non-nil
>       ;; values for Buffer-menu-use-header-line.
> !     (define-key map [header-line mouse-1] 'Buffer-menu-sort-by-column)
> !     (define-key map [header-line mouse-2] 'Buffer-menu-sort-by-column)
> !     (define-key map [mouse-2] 'Buffer-menu-sort-by-column)
>       (define-key map [follow-link] 'mouse-face)
> !     (define-key map "\C-m" 'Buffer-menu-sort-by-column)
>       (propertize name
> + 		'column column
>                   'help-echo (concat
>                               (if Buffer-menu-use-header-line
>                                   "mouse-1, mouse-2: sort by "

Then the keymap doesn't depend on `name' or `column', so it can be created
once and for all at the toplevel, right?


        Stefan

Re: Buffer menu fix

[Kim F. Storm]

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

>>   (defun Buffer-menu-make-sort-button (name column)
>>     (if (equal column Buffer-menu-sort-column) (setq column nil))
>>     (let* ((downname (downcase name))
>> !          (map (make-sparse-keymap)))
>>       ;; This keymap handles both nil and non-nil
>>       ;; values for Buffer-menu-use-header-line.
>> !     (define-key map [header-line mouse-1] 'Buffer-menu-sort-by-column)
>> !     (define-key map [header-line mouse-2] 'Buffer-menu-sort-by-column)
>> !     (define-key map [mouse-2] 'Buffer-menu-sort-by-column)
>>       (define-key map [follow-link] 'mouse-face)
>> !     (define-key map "\C-m" 'Buffer-menu-sort-by-column)
>>       (propertize name
>> + 		'column column
>>                   'help-echo (concat
>>                               (if Buffer-menu-use-header-line
>>                                   "mouse-1, mouse-2: sort by "
>
> Then the keymap doesn't depend on `name' or `column', so it can be created
> once and for all at the toplevel, right?

Right!

Chong, would you like to work on this?

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

Re: Buffer menu fix

[Chong Yidong]

Stefan just checked in a different fix to buff-menu.el, defining a
lambda expression and interning it.  This fix also has a problem:
`C-h k' can't generate a hyperlink to the Lisp file:

   mouse-2 runs the command Buffer-menu-sort-by-name, which is an
   interactive Lisp function.

Kim's solution doesn't have this problem.

Re: Buffer menu fix

[Chong Yidong]

>> Then the keymap doesn't depend on `name' or `column', so it can be created
>> once and for all at the toplevel, right?
>
> Right!
>
> Chong, would you like to work on this?

Yes, it's not hard.