casouri / eldoc-box

childframe doc for eglot and anything that uses eldoc
342 stars 27 forks source link

Child frame inherits parent's window margin widths #107

Closed brownts closed 3 weeks ago

brownts commented 4 weeks ago

The child frame inherits the margin widths from the parent window. In my parent window, running window-margins returns (nil . 2) since I have flymake diagnostics highlighted in the right margin (visible in the snapshot showing a nerd icon). This margin width ends up being inherited by the child window and causes the window size to be computed incorrectly, which ultimately causes the text to wrap unnecessarily. The text in the eldoc buffer can be seen at the bottom of the snapshot, so it's clear the text should not be wrapping in the child frame. Additionally eldoc-box-max-pixel-width has been set to 1500, so that it's not limiting the window width.

Screenshot_2024-08-17_before

I'm able to work around this by using the eldoc-box-frame-hook and explicitly setting the window margins to 0, although this likely should be incorporated directly into eldoc-box.

(defun init.el/fix-margins/eldoc-box (_main-frame)
  (let* ((frame (selected-frame))
         (window (frame-selected-window frame)))
    (set-window-margins window 0)))

(add-hook 'eldoc-box-frame-hook #'init.el/fix-margins/eldoc-box)

With the above hook in place, the window content is displayed correctly. The following is a snapshot taken with the above hook installed.

Screenshot_2024-08-17_after

casouri commented 3 weeks ago

Thank you! I updated eldoc-box as you suggested. Could you check if it's fixed now?

brownts commented 3 weeks ago

Thank you! I updated eldoc-box as you suggested. Could you check if it's fixed now?

Unfortunately, that did not fix it. That ends up clearing the margins of the parent window, as the child window may not even exist at that point in time. I've created a PR which does address the issue by moving the call into eldoc-box--get-frame.

casouri commented 3 weeks ago

Thank you! You even wrote very nice commit messages! Merged it.