Truncated org-attach popup

[memeplex]

Maybe related to the org-set-tags-command case but worse and doesn't get fixed with the previous redisplay.

I've seen this in both moody and doom modelines.

Also disabled the redisplay advice just in case but to no avail.

Recipe:

1. Open any org file
2. Press C-c C-a
3. You should see something like:

instead of (without customized modeline):

[memeplex]

Ok, I think I understand what the problem is. In org-attach we have:

      (save-excursion
	(save-window-excursion
            ....
            (org-switch-to-buffer-other-window "*Org Attach*")
             ;; write menu and stuff
       )) ;; end excursions
       (org-fit-window-to-buffer (get-buffer-window "*Org Attach*"))

Then org-switch-to-buffer-other-window will create a new window but org-fit-window-to-buffer will happen after the excursion, that is in the original window, and therefore its associated redisplay as well.

That said, I'd assumed that redisplay was a frame-level operation but two observations support the "late redisplay" hypothesis (which implies that redisplay is a window-level operation... still unchecked):

1. If the frame has two windows at the moment I execute org-attach, then the problem won't happen and this likely is because the second window is reused during org-switch-to-buffer-other-window instead of a new one being created.
2. If I put a (redisplay t) inside the excursion the problem won't happen either, which likely is due to the fact that the current window is the one with the org-attach buffer (and here is where the "display is a window-level operation" assumption plays).

This left me thinking whether window creation should be advised with the forced redisplay instead of fit-window-to-buffer, so that window height information is correct from the beginning. I have to sleep now but will continue later. HIH.

PS: in org-fast-tag-selection unless (eq org-fast-tag-selection-single-key 'expert) the call to org-fit-window-to-buffer is done inside the excursion. And even when that condition doesn't hold, there is this sequence: (org-switch-to-buffer-other-window " *Org tags*") (org-fit-window-to-buffer). That is, the fit is always done inside the target window. This may explain the difference in behavior wrt org-attach.

[memeplex]

Ah, one more thing, not specially relevant for the above but something that I realized along the way.

In fit-window-to-buffer there is this code:

	       (height (+ (cdr (window-text-pixel-size
			        window nil t nil (frame-pixel-height frame) t))
		          (window-scroll-bar-height window)
		          (window-bottom-divider-width window))))
          ...
	  (unless pixelwise  ; <--- pixelwise is nil
	    (setq height (/ (+ height char-height -1) char-height)))

So even when the result of window-text-pixel-size is perfectly right (that is, after (redisplay t) with a prettified modeline) this won't really cut the mustard. The height in pixels, which includes the height of the modeline, is at some point transformed to a height in characters by dividing by a common char-height which doesn't actually exist as such since the modeline has its own different height. Given that this number is always rounded up to the closest character size, some spare space is reserved (I checked this, a small gap is to be expected, nothing to worry about though). The assumption of a common character height is part of fit-window-to-buffer.

EDIT: at first I had ignored the + char-height - 1 correction and thought that if the modeline were smaller than the average character, then the computed size could end up being rounded down to a smaller height than the one required. But since this won't happen the consequences are minor and just cosmetic.

[memeplex]

Me again. I just kept thinking about this. I believe the tag selection menu case could also be explained because of redisplay not running early in some popup windows. The current advice is deactivating itself after its first run in a buffer, but while popup windows come and go their buffers may remain lurking hidden. So what about using a window parameter in lieu of the buffer local?

Nevertheless that won't suffice to solve the problem described above concerning the attach menu, because in that case the fit is done from another window. The general solution is to run early just once upon window creation. I'm asking upstream if that would be an overkill, but I don't believe it is.

[memeplex]

I'm experimenting with something as simple as

(defun redisplay-hack (&rest _)
  (when mode-line-format
    (redisplay t)))
(advice-add #'split-window :after #'redisplay-hack)

and so far it seems to cover all cases.

Notice that split-window apparently is the primitive for window creation (but I've asked upstream for clarification on using split-window vs split-window-internal).

Notice also that there is no obvious hook based solution, since all relevant hooks are called by redisplay itself.

Some features:

• There is no need to keep a buffer-local or window-parameter because it's triggered once per window by definition (the downside is "once per every window", but that's not a big penalty).
• It always triggers in the right window, so it solves the issue with org-attach.
• It's not tied to a buffer that may survive its popup, so it solves the issue with org-set-tags-command.

There still is a visual glitch because the window is displayed before the fit and then immediately replaced by the fitted version. But this is a problem shared by all the proposed solutions, since all require to redisplay before fitting, so I see no way around that glitch, we probably will have to live with it.

I'm still discussing other options with upstream, but I believe this is the best one.