alphapapa / frame-purpose.el

Purpose-specific frames for Emacs
GNU General Public License v3.0
52 stars 4 forks source link

frame-purpose is ineffective in ivy-switch-buffer #13

Open akirak opened 6 years ago

akirak commented 6 years ago

ivy-switch-buffer in ivy package ignores frame-purpose. That is, even if frame-purpose-mode is turned on and you are in a purpose-enabled frame, all buffers are displayed as candidates in the command. This seems to be because the function uses 'internal-complete-buffer to produce a list of buffers:

(defun ivy-switch-buffer ()
  "Switch to another buffer."
  (interactive)
  (let ((this-command 'ivy-switch-buffer))
    (ivy-read "Switch to buffer: " 'internal-complete-buffer
              :matcher #'ivy--switch-buffer-matcher
              :preselect (buffer-name (other-buffer (current-buffer)))
              :action #'ivy--switch-buffer-action
              :keymap ivy-switch-buffer-map
              :caller 'ivy-switch-buffer)))

I am currently using a modified version of helm-mini for switching buffers, because I am mainly using EXWM and prefer selecting an EXWM buffer in a separate Helm source. Helm provides a better display of a buffer list. However, Ivy is faster, so I may switch to Ivy when I leave EXWM (which is likely to happen in the near future). Would you add support for ivy-switch-buffer?

alphapapa commented 6 years ago

Hi Akira,

I don't know if overriding internal-complete-buffer would be appropriate. It might work fine, but I'm not familiar with it, and I don't know where else it is used. If you could test it for a while and confirm that it works without any bad side effects, I'd probably be willing to merge it.

On the other hand, you could probably implement your own version of ivy-switch-buffer that uses the list returned by buffer-list. This seems to work:

(defun ivy-switch-buffer2 ()
  "Switch to another buffer."
  (interactive)
  (let ((this-command 'ivy-switch-buffer))
    (ivy-read "Switch to buffer: " (lambda (&rest args)
                                     (mapcar #'buffer-name (buffer-list)))
              :matcher #'ivy--switch-buffer-matcher
              :preselect (buffer-name (other-buffer (current-buffer)))
              :action #'ivy--switch-buffer-action
              :keymap ivy-switch-buffer-map
              :caller 'ivy-switch-buffer)))
akirak commented 6 years ago

Thank you for your response.

It turns out that frame-purpose slows down buffer-list. With frame-purpose-mode, buffer-list is 50 times slower than internal-complete-buffer. It may not make sense to switch to ivy for better performance when I am using frame-purpose-mode. I felt helm-mini (and helm-buffers-list) was considerably slower than ivy-switch-buffer, but I am not sure whether it is because of helm or because of frame-purpose-mode. Below is the details.

buffer-list:

  (elp-instrument-function #'buffer-list)
  (dotimes (_i 1000)
    (buffer-list))

elp-results with frame-purpose-mode:

buffer-list               1138        0.1305083409  0.0001146821

Without frame-purpose-mode (the function is still advised):

buffer-list               1084        0.0101568510  9.369...e-06

internal-complete-buffer:

(elp-instrument-function #'internal-complete-buffer)
(dotimes (_i 1000)
  (internal-complete-buffer "" nil t))
internal-complete-buffer  1000        0.0020276049  2.027605e-06
akirak commented 6 years ago

Your solution would work correctly, but I found another solution to add support for frame-purpose:

  (defun akirak/ad-around-internal-complete-buffer (orig &rest rest)
    (if (memq this-command '(ivy-switch-buffer
                             ivy-switch-buffer-other-window))
        (mapcar #'buffer-name (buffer-list))
      (apply orig rest)))
  (advice-add #'internal-complete-buffer
              :around #'akirak/ad-around-internal-complete-buffer)

With this advice and frame-purpose enabled (but not on a frame-specific frame), internal-complete-buffer seems to be still faster than buffer-list (but this may depend on how the function is called):

internal-complete-buffer  1000        0.0059932399  5.993...e-06
alphapapa commented 6 years ago

It turns out that frame-purpose slows down buffer-list. With frame-purpose-mode, buffer-list is 50 times slower than internal-complete-buffer.

Yes, it does. It's made much worse by the fact that, apparently, buffer-list is called in many places in Emacs that one wouldn't expect and can't easily see. There are at least a few cases I've encountered where the slowdown is prohibitive and I have to just disable frame-purpose-mode, like when using elfeed (something about the way it updates items or processes curl output causes buffer-list to be called many, many times).

It's disappointing, but frame-purpose-mode is somewhat of a hack, so I guess it's what we get. I'm not sure if there's any way around it other than implementing some of it in Emacs core. I did post to emacs-devel a while back asking about making buffer-list always run in the context of the primary GUI frame, rather than whichever frame was last active, which I think would help this problem, but that seemed hacky to the devs as well, and they didn't think it was a good idea.

It might help if there were a built-in frame-buffer-list function that only returned buffers for a frame. As it is, buffer-list almost does that:

buffer-list is a built-in function in ‘C source code’.

(buffer-list &optional FRAME)

Return a list of all live buffers.
If the optional arg FRAME is a frame, return the buffer list in the
proper order for that frame: the buffers shown in FRAME come first,
followed by the rest of the buffers.

Although, looking at the the code for that, I don't think it would be that simple.

I think that the way to "solve" the problem is to have a dedicated command like frame-purpose-buffer-list that users specifically call, rather than overriding buffer-list for everything. That would avoid causing everything else that calls buffer-list to slow down, but it means that users would have to modify all the code that they want to respect frame-purpose-mode. That might be a good middle-ground though. For example, I think I would get most of the benefits I want by doing that and calling that function instead of the real buffer-list in my Helm buffer-selecting command.

akirak commented 5 years ago

I've implemented an enhanced version of ivy-switch-buffer2 that can switch from purpose-enabled mode to disabled and vice versa. When you press C-c C-p, it toggles the mode without closing the prompt.

(defvar ivy-switch-buffer-2-map
  (let ((map (make-composed-keymap nil ivy-switch-buffer-map)))
    (define-key map (kbd "C-c C-p") 'ivy-switch-buffer-2-toggle-mode)
    map))

(defun ivy-switch-buffer-2--complete (&rest args)
  (mapcar #'buffer-name (buffer-list)))

(defun ivy-switch-buffer-2-toggle-mode ()
  (interactive)
  (let ((text ivy-text)
        collection)
    (pcase (ivy-state-collection ivy-last)
      ('ivy-switch-buffer-2--complete
       (setq collection 'internal-complete-buffer))
      ('internal-complete-buffer
       (setq collection 'ivy-switch-buffer-2--complete)))
    (setf (ivy-state-collection ivy-last) collection)
    (ivy--reset-state ivy-last)
    (setf (ivy-state-text ivy-last) text)))

(defun ivy-switch-buffer-2 (&optional arg)
  "Switch to another buffer."
  (interactive "P")
  (let ((this-command 'ivy-switch-buffer))
    (ivy-read "Switch to buffer: "
              (if arg
                  #'internal-complete-buffer
                #'ivy-switch-buffer-2--complete)
              :matcher #'ivy--switch-buffer-matcher
              :preselect (buffer-name (other-buffer (current-buffer)))
              :action #'ivy--switch-buffer-action
              :keymap ivy-switch-buffer-2-map
              :caller 'ivy-switch-buffer)))