Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use window-total-width' instead of window-width'. #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use window-total-width' instead of window-width'. #119

wants to merge 1 commit into from

Conversation

joostkremers
Copy link

The function window-width returns the width of the text area, which leads to wrong results with windows that have (wide) margins:

screenshot from 2015-02-17 13 30 00

The window to the left is originally a frame-wide window with large margins. The window to the right is created with popwin (used in the guide-key package). It's obviously far too wide. The problem can be solved by using window-total-width, which takes the window margins into account:

screenshot from 2015-02-17 13 33 46

@m2ym
Copy link
Contributor

m2ym commented Mar 1, 2015

Looks good, but after applying this patch, make test failed. Could you please look into this problem?

@joostkremers
Copy link
Author

The test that fails contains a call to window-width. Replacing that with window-total-width makes the test pass.

There are two more instances of window-width in popwin-test.el. I can't really judge if they should be replaced as well.

I could create a patch if you like.

@m2ym
Copy link
Contributor

m2ym commented Mar 1, 2015

I have experienced the following error.

Selector: t
Passed: 40
Failed: 2 (2 unexpected)
Total:  42/42

Started at:   2015-03-01 23:13:58+0900
Finished.
Finished at:  2015-03-01 23:13:59+0900

..............FF..........................

F popup-at-right
    (ert-test-failed
     ((should
       (eq
    (nth 2
         (window-inside-edges))
    right))
      :form
      (eq 68 67)
      :value nil))

F popup-at-right-with-three-columes
    (ert-test-failed
     ((should
       (eq
    (nth 2
         (window-inside-edges))
    right))
      :form
      (eq 68 67)
      :value nil))

This means that the window configuration has been changed little a bit after calling popwin. I didn't find yet how to fix this problem.

@joostkremers
Copy link
Author

Mmm, I get neither of those errors. I’m running Emacs 24.4.1, perhaps that is of relevance. Only one test failed for me:

Selector: t
Passed: 41
Failed: 1 (1 unexpected)
Total:  42/42

Started at:   2015-03-02 02:46:37+0100
Finished.
Finished at:  2015-03-02 02:46:39+0100

............F.............................

F popup-at-left-with-50%
    (ert-test-failed
     ((should
       (<=
    (1-
     (/ width 2))
    (window-width)))
      :form
      (<= 39 38)
      :value nil))

It should be noted that window-total-width not only takes the window’s margins into account, but also its fringes, scroll bars and right divider. It is possible that any of these may cause problems in certain cases. A safer alternative to using window-total-width may be to calculate the width on the basis of the body width and the margins, something along the following lines:

(defun my-window-width (&optional win)
  "Return a window's text plus margin width.
WIN defaults to the selected window."
  (+ (window-width win)
     (or (car (window-margins win)) 0)
     (or (cdr (window-margins win)) 0)))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants