Skip to content

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