bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Drew Adams]

Starting with Emacs 23, you changed `menu-bar-update-buffers' so that it
no longer uses `menu-bar-select-buffer'.  (That command is now used only
in msb.el.)
 
Instead of `menu-bar-select-buffer', `menu-bar-update-buffers' now uses
this:
 
 `(lambda ()
   (interactive)
   (switch-to-buffer ,(cdr pair))
 
Before Emacs 23 it was trivial to make menu choice use a separate frame:
trivially redefine `menu-bar-select-buffer' (or advise it, I suppose).
Here, for instance, is a redefinition that covers all versions of Emacs:
 
(defun menu-bar-select-buffer ()
  "Switch to `last-command-event' buffer in other frame."
  (interactive)
  (if (fboundp 'pop-to-buffer-other-frame) ; Emacs 24+
      (pop-to-buffer-other-frame last-command-event)
    (switch-to-buffer-other-frame last-command-event)))
 
But that is useless now, since `menu-bar-select-buffer' is no longer
called.
 
Dunno what the problem was with using `last-command-event', which caused
you to switch to the lambda code above.  Wouldn't
`menu-bar-select-buffer' work just as well?
 
What to do now, to make menu selection use another frame?  Copy and
modify trivially the long `menu-bar-update-buffers' definition, either
redefining it or (preferably) replacing it on `menu-bar-update-hook'?
Do you have another suggestion?
 
It was so simple and flexible before; it seems so complex and monolithic
now.
 
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-06-13 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/build/include'
 

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Stefan Monnier]

> Dunno what the problem was with using `last-command-event', which caused
> you to switch to the lambda code above.

Somehow I do remember why: using buffers as events is not really
supported and was leading to some bugs (which I can't remember) in some
corner cases.  So I changed the code to use more normal events (numbers)
and embed the buffer in the command itself (as a closure, basically)
which is conceptually much cleaner.

> It was so simple and flexible before; it seems so complex and monolithic
> now.
 
Feel free to change it so that instead of calling `switch-to-buffer' it
calls some alias or it (funcall menu-bar-select-buffer-function <buf>).


        Stefan

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Drew Adams]

> Feel free to change it so that instead of calling 
> `switch-to-buffer' it
> calls some alias or it (funcall 
> menu-bar-select-buffer-function <buf>).

That would be great.  What do you need from me - a patch?

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Stefan Monnier]

>> Feel free to change it so that instead of calling 
>> `switch-to-buffer' it
>> calls some alias or it (funcall 
>> menu-bar-select-buffer-function <buf>).
> That would be great.  What do you need from me - a patch?

For example, yes, including ChangeLog entry and a tiny note in etc/NEWS.


        Stefan

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Drew Adams]

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

> >> Feel free to change it so that instead of calling 
> >> `switch-to-buffer' it calls some alias or it (funcall 
> >> menu-bar-select-buffer-function <buf>).
> > That would be great.  What do you need from me - a patch?
> 
> For example, yes, including ChangeLog entry and a tiny note 
> in etc/NEWS.

Attached.  Changelog entry:

2011-06-17  Drew Adams <drew.adams@oracle.com>

	* menu-bar.el: Use function variable instead of switch-to-buffer.
	(menu-bar-select-buffer-function): New variable.
	(menu-bar-update-buffers): Use menu-bar-select-buffer-function.

[-- Attachment #2: menu-bar-2011-06-17.patch --]
[-- Type: application/octet-stream, Size: 1399 bytes --]

diff -c -w "menu-bar.el" "menu-bar-patched-2011-06-17.el"
*** menu-bar.el	Fri Jun 17 06:52:34 2011
--- menu-bar-patched-2011-06-17.el	Fri Jun 17 07:14:10 2011
***************
*** 1977,1982 ****
--- 1977,1986 ----
  ;; Used to cache the menu entries for commands in the Buffers menu
  (defvar menu-bar-buffers-menu-command-entries nil)
  
+ (defvar menu-bar-select-buffer-function 'switch-to-buffer
+   "Function applied to a buffer chosen from the `Buffers' menu-bar menu.
+ It must accept a buffer as its only required argument.")
+ 
  (defun menu-bar-update-buffers (&optional force)
    ;; If user discards the Buffers item, play along.
    (and (lookup-key (current-global-map) [menu-bar buffer])
***************
*** 2022,2028 ****
  					(cons nil nil))
  				  `(lambda ()
                                       (interactive)
!                                      (switch-to-buffer ,(cdr pair))))))
                     (list buffers-vec))))
  
  	 ;; Make a Frames menu if we have more than one frame.
--- 2026,2032 ----
  					(cons nil nil))
  				  `(lambda ()
                                       (interactive)
!                                      (funcall menu-bar-select-buffer-function ,(cdr pair))))))
                     (list buffers-vec))))
  
  	 ;; Make a Frames menu if we have more than one frame.

Diff finished at Fri Jun 17 07:14:31

[-- Attachment #3: NEWS-2011-06-17.patch --]
[-- Type: application/octet-stream, Size: 600 bytes --]

diff -c -w "NEWS" "NEWS-patched-2011-06-17"
*** NEWS	Fri Jun 17 07:23:06 2011
--- NEWS-patched-2011-06-17	Fri Jun 17 07:22:50 2011
***************
*** 309,314 ****
--- 309,320 ----
  The elisp implementation sha1.el is removed. Feature sha1 is provided
  by default.
  
+ ** Menu-bar changes
+ 
+ *** Buffers menu uses variable `menu-bar-select-buffer-function'.
+ The value of this new variable is the function invoked when you choose
+ a buffer from the menu.  Default value: `switch-to-buffer'.
+ 
  \f
  * Editing Changes in Emacs 24.1
  

Diff finished at Fri Jun 17 07:23:19

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Stefan Monnier]

>> For example, yes, including ChangeLog entry and a tiny note 
>> in etc/NEWS.
> Attached.  Changelog entry:

Thanks, installed,


        Stefan

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Stefan Monnier]

Version: 24.1

Done.

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Drew Adams]

Sending again, since the bug was archived (apparently replying to
the thread sends your reply to /dev/null if a bug has been archived -
quite a feature!).

--------------------------------------------------------

> > Attached.  Changelog entry:
> Thanks, installed,

Well, that only took things so far.  At the time I did that, Martin
had defined `pop-to-buffer-other-frame' in his version of window.el,
which I was using as a fix for another bug (since fixed).  That code
never made it into window.el, however.

So I guess it is not so easy now to assign `pop-to-buffer-other-frame'
to `menu-bar-select-buffer-function', i.e., to make it pop to the
buffer in another frame.

This was Martin's definition, at the time:

(defsubst pop-to-buffer-other-frame
  (&optional buffer-or-name norecord label)
  "Pop to buffer specified by BUFFER-OR-NAME on another frame.
The selected frame will be used only if there's no other choice.
Optional arguments BUFFER-OR-NAME, NORECORD and LABEL are as for
`pop-to-buffer'."
  (interactive "BPop to buffer on another frame:\nP")
  (pop-to-buffer buffer-or-name 'other-frame norecord label))

(I would not use a defsubst.)

But that is not compatible with the latest `pop-to-buffer'.
And when I look at the doc for `pop-to-buffer' I keel over.

It tells me I can use an ACTION argument (as Martin did here,
with value `other-frame').  But it sends me off to the doc for
`display-buffer', whose description of parameter ACTION is
incomprehensible to me.  WAY too complicated.

Can you please tell me what (simple) ACTION argument to pass to
`pop-to-buffer' to get it to use another frame?  The doc does not
make it sound like simply `other-frame' will suffice. ;-)

This is the function I'm using so far, with `pop-up-frames':

(lambda (buffer &optional other-window norecord)
  (interactive "BPop to buffer on another frame:\nP")
  (let ((pop-up-frames  t))
    (pop-to-buffer buffer other-window norecord)))

Is there a different simple way to do this now?  Hope so.

Thx.

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> But that is not compatible with the latest `pop-to-buffer'.
> And when I look at the doc for `pop-to-buffer' I keel over.
>
> It tells me I can use an ACTION argument (as Martin did here,
> with value `other-frame').  But it sends me off to the doc for
> `display-buffer', whose description of parameter ACTION is
> incomprehensible to me.  WAY too complicated.

I made the same experience.  Now, a good and detailed description is
given in the manual under (elisp) Display Action Functions.  IMHO it's
understandable.


Regards,

Michael.

bug#8876: 24.0.50; making `menu-bar-update-buffers' use another frame

[Drew Adams]

> > But that is not compatible with the latest `pop-to-buffer'.
> > And when I look at the doc for `pop-to-buffer' I keel over.
> >
> > It tells me I can use an ACTION argument (as Martin did here,
> > with value `other-frame').  But it sends me off to the doc for
> > `display-buffer', whose description of parameter ACTION is
> > incomprehensible to me.  WAY too complicated.
> 
> I made the same experience.

Thanks for confirming I am not alone.  I doubt that we two are alone.

> Now, a good and detailed description is given in the manual
> under (elisp) Display Action Functions.  IMHO it's
> understandable.

Yes, it helps, but this is all still way too complicated, AFAICT.

That node should be xref'd from the `display-buffer' doc string,
or the same info should be added to the doc string.  Without such
info, you really haven't a clue what the ACTION parameter is all
about.

And it should be made clear in that node that the action functions
described there correspond to the car of the ACTION parameter for
`display-buffer" (and `pop-to-buffer').  This connection is currently
not being made.

The node title says that these are action functions for
`display-buffer', but it should be pointed out that they are used
in the car of the ACTION parameter.

Anyway, I guess the answer to my question is more or less this:

(pop-to-buffer my-buffer '(display-buffer-pop-up-frame))

Ah, alas, no.  That creates a new frame instead of reusing an existing
frame.  And that is true regardless of the value of `pop-up-frames'.

And that's a lot less handy than just (pop-to-buffer 'other-frame).
And it is even less succinct than this (which still works, thank
goodness):

(let ((pop-up-frames t)) (pop-to-buffer mybuf))

And the latter has the advantage that it does reuse an existing frame,
rather than creating a duplicate one.  `pop-to-buffer' has previously
preferred an existing frame/window.  Now it eagerly creates new ones?

Still not clear to me.  Still seems WAY overengineered, to me.

I do appreciate the goal of providing more fine-grained control over
buffer display.  But the result, at least for my use case (separate
frames a lot of the time) seems to be imposed extra complication.

The fine-grained control should be available, but should not be in
your face.  You should not be required to twiddle fine-tuning knobs
to get simple behavior, especially simple behavior that has been
availablein the past in a simple way.

IOW, it seems that we have thrown out the baby with the bathwater.