girzel / ebdb

An EIEIO port of BBDB, Emacs' contact-management package
67 stars 12 forks source link

ebdb-mu4e popup does not go away and takes a lot of space #77

Closed brabalan closed 5 years ago

brabalan commented 5 years ago

I use the default settings for ebdb. Each time the ebdb-mu4e popup opens, I have to manually close it. For instance, if I use address completion when writing a message, the popup remains after the message is sent. As it takes a lot of space (the right-hand side of the screen), it breaks the layout of mu4e.

Could it be possible to close the popup automatically, or to display it at the bottom of the screen?

Note that I use doom emacs, which changes many things regarding popups, so the problem might come from there.

brabalan commented 5 years ago

An additional problem is that I can get two popups open: one for mu4e and one for the message mode.

girzel commented 5 years ago

I use the default settings for ebdb. Each time the ebdb-mu4e popup opens, I have to manually close it. For instance, if I use address completion when writing a message, the popup remains after the message is sent. As it takes a lot of space (the right-hand side of the screen), it breaks the layout of mu4e.

Could it be possible to close the popup automatically, or to display it at the bottom of the screen?

I'm still missing a bunch of possible configuration knobs for ebdb-mu4e, the first thing I'll do is add a config option to control the width of the popup.

Does mu4e use message-mode for mail composition? I suppose it works in Gnus because Gnus itself re-organizes the windows after a message is sent. If mu4e uses message-mode I can set an after-send hook, or if it uses its own mode I can probably do something similar there.

An additional problem is that I can get two popups open: one for mu4e and one for the message mode.

That's a feature, not a bug! :)

If I can get popups sorted, it might not be so annoying anymore, since you won't see it hanging around. But if it really bugs you, it's possible to give popups in both modes the same buffer name, meaning that they will reuse the same buffer.

girzel commented 5 years ago

Okay, it does look like mu4e uses message-mode for composition, so it should be a simple thing to add a function to the sent hook to quit the EBDB window.

brabalan commented 5 years ago

Great, thanks!

girzel commented 5 years ago

I use the default settings for ebdb. Each time the ebdb-mu4e popup opens, I have to manually close it. For instance, if I use address completion when writing a message, the popup remains after the message is sent. As it takes a lot of space (the right-hand side of the screen), it breaks the layout of mu4e.

Could it be possible to close the popup automatically, or to display it at the bottom of the screen?

Yes, ebdb-mu4e.el needs a lot more configuration options. First of all, are you using message-mode to compose emails? Or does mu4e have its own composition mode? I guess I just need to install all of this and test it locally...

On 05/30/19 00:42 AM, Alan Schmitt wrote:

An additional problem is that I can get two popups open: one for mu4e and one for the message mode.

That's a feature, not a bug! It should be less annoying if the windows get quit correctly.

girzel commented 5 years ago

Would you eval and use this function for a while, and see if it does anything weird?

(defun ebdb-insinuate-mu4e ()
  (define-key mu4e-view-mode-map ";" ebdb-mua-keymap)
  (add-hook 'message-sent-hook
        (lambda ()
          (let ((win (get-buffer-window (ebdb-message-buffer-name))))
        (when (and win
               (window-live-p win))
          (quit-window nil win))))))

Thanks!

brabalan commented 5 years ago

Sorry for the delay. It seems to be working great, the popups disappear.

girzel commented 5 years ago

Cool! In a day or so I'll add configuration options for controlling the size of the popup.

brabalan commented 5 years ago

I found a small issue. Here is how to reproduce it:

brabalan commented 5 years ago

I've been experimenting some more, and it seems I have trouble having the hook stick. More precisely, when the hook is present in message-sent-hook, all goes well, but I need to make sure it gets installed when I start emacs.

As I use doom emacs, the issue might come from there.

girzel commented 5 years ago

Once the hook is in there, does it stay there? I mean does anything delete the function from the hook later on?

Right now EBDB insinuates itself into mu4e using the mu4e-main-mode-hook. Maybe it would work better to use the mu4e-view-mode-hook? Otherwise, I don't see how you could be starting mu4e but the message-sent-hook doesn't get added...

brabalan commented 5 years ago

Yes, it stays there. I need to make sure the hook is set after whatever configuration overrides it. I don't think this is an ebdb issue, but rather a doom issue.

girzel commented 5 years ago

I've added customization options for controlling popup window sizes in 8f0251f. You can now set ebdb-default-window-size to cover all MUAs, or individually set ebdb-message-window-size and ebdb-mu4e-window-size for finer-grained control.

Give that a shot when you have a moment! If it seems to work as expected, I'll close this issue and do a new release of EBDB.

brabalan commented 5 years ago

Thank you very much. Does this include the possibility of having a new window at the bottom instead of on the right?

I need to figure out how to test this (I'm using the packaged version), then I will tell you how it goes.

girzel commented 5 years ago

You can control the direction of the split, but I have yet to think of a clean way of controlling that using customization options. For the time being you'd have to override the popup method, which looks like this:

(cl-defmethod ebdb-popup-window (&context (major-mode mu4e-view-mode))
  (list (get-buffer-window) ebdb-mu4e-window-size))

You can simply put this same code in your startup files, and change it however you like: instead of (get-buffer-window) you can specify a different window to split, and after the ebdb-mu4e-window-size you can add a direction symbol: 'left, 'right, 'above, or 'below. Play around with that and see how it works.

At some point I'll come up with a simpler way of configuring this, but for now this is probably all you can do.

brabalan commented 5 years ago

I tried something similar for composing messages:

  (cl-defmethod ebdb-popup-window (&context (major-mode message-mode))
    (list (get-buffer-window) 0.3 'below))

but I get an error

split-window: Window #<window 479 on *draft*> too small for splitting

This is surprising as it's a fullscreen window.

girzel commented 5 years ago

Yes! This was a bug, and a fairly fundamental one, thank you for finding it. I'm almost certain I've fixed it, but before I do a new release with the fix, would you just eval this function and confirm that the above error goes away, and the windows split as you expect them to?

(defun ebdb-pop-up-window (buf &optional select pop)
  (let* ((buf (get-buffer buf))
     (split-window (car-safe pop))
     (buffer-window (get-buffer-window buf t))
     (direction (or (nth 2 pop)
            (if (> (window-total-width split-window)
                   (window-total-height split-window))
                'right
              'below)))
     (size (cond ((null pop)
              nil)
             ((integerp (cadr pop))
              (cadr pop))
             ((or (floatp (cadr pop)) (floatp ebdb-default-window-size))
              (let ((flt (or (cadr pop) ebdb-default-window-size)))
            (round (* (if (memq direction '(left right))
                      (window-total-width split-window)
                    (window-total-height split-window))
                  (- 1 flt)))))
             ((integerp ebdb-default-window-size)
              ebdb-default-window-size))))

    (cond (buffer-window
       ;; It's already visible, re-use it.
       (when select
         (select-window buffer-window)))
      ((not (or split-window size))
       ;; Not splitting, but buffer isn't visible, just take up
       ;; the whole window.
           (pop-to-buffer-same-window buf)
       (setq buffer-window (get-buffer-window buf t)))
      (t
       ;; Otherwise split.
       (setq
        buffer-window
        ;;   If the window we're splitting is an atomic window,
        ;; maybe make our buffer part of the atom.
        (if (and ebdb-join-atomic-windows
             (window-atom-root split-window))
        (display-buffer-in-atom-window
         buf `((window . ,split-window)
               (side . ,direction)
               ,(if (eq direction 'below)
                `(window-height . ,size)
              `(window-width . ,size))))
          (split-window
           split-window size direction)))
       (set-window-buffer buffer-window buf)))
    (display-buffer-record-window 'window buffer-window buf)
    (set-window-prev-buffers buffer-window nil)
    (when select
      (select-window buffer-window))))
brabalan commented 5 years ago

It works great, thanks!

brabalan commented 5 years ago

Any plan to release a version on Melpa with this fix in?

girzel commented 5 years ago

Yeah, I was going to try to squeeze another bug fix in before I did another release, but I haven't gotten to it and there's no real reason to delay. I'll do a point bump now.