* bug#16017: 24.3.50; windmove is broken
@ 2013-12-01 1:52 Dmitry Gutov
2013-12-02 7:38 ` martin rudalics
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2013-12-01 1:52 UTC (permalink / raw)
To: 16017
windmove-right and windmove-down don't do anything now.
In GNU Emacs 24.3.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.8.6)
of 2013-12-01 on axl
Bzr revision: 115317 eliz@gnu.org-20131130191221-9pxtv2v0eilgek8u
Windowing system distributor `The X.Org Foundation', version 11.0.11403000
System Description: Ubuntu 13.10
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-01 1:52 bug#16017: 24.3.50; windmove is broken Dmitry Gutov
@ 2013-12-02 7:38 ` martin rudalics
2013-12-02 10:29 ` Dmitry Gutov
0 siblings, 1 reply; 16+ messages in thread
From: martin rudalics @ 2013-12-02 7:38 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017
> windmove-right and windmove-down don't do anything now.
As you mentioned elsewhere, this should be a consequence of the
pixelwise change. Can you try to find out why it fails?
In `window-in-direction' I now use `window-pixel-edges' instead of
`window-edges'. Would doing the same in windmove.el fix it?
martin
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-02 7:38 ` martin rudalics
@ 2013-12-02 10:29 ` Dmitry Gutov
2013-12-02 14:38 ` Stefan Monnier
2013-12-02 18:16 ` martin rudalics
0 siblings, 2 replies; 16+ messages in thread
From: Dmitry Gutov @ 2013-12-02 10:29 UTC (permalink / raw)
To: martin rudalics; +Cc: 16017
On 02.12.2013 09:38, martin rudalics wrote:
> In `window-in-direction' I now use `window-pixel-edges' instead of
> `window-edges'. Would doing the same in windmove.el fix it?
It doesn't. If I replace both occurrences of `window-edges' with
`window-pixel-edges', the only windmove command that works is
`windmove-down'. I've also tried replacing `window-inside-edges' with
`window-inside-pixel-edges' and `posn-col-row' with `posn-x-y'.
What does help, is changing `windmove-window-distance-delta' from 1 to
2. So the problem is likely to do with off-by-one errors in `window-edges'.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-02 10:29 ` Dmitry Gutov
@ 2013-12-02 14:38 ` Stefan Monnier
2013-12-02 18:16 ` martin rudalics
2013-12-02 18:16 ` martin rudalics
1 sibling, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2013-12-02 14:38 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017
>> In `window-in-direction' I now use `window-pixel-edges' instead of
>> `window-edges'. Would doing the same in windmove.el fix it?
BTW, shouldn't windmove use window-in-direction?
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-02 10:29 ` Dmitry Gutov
2013-12-02 14:38 ` Stefan Monnier
@ 2013-12-02 18:16 ` martin rudalics
2013-12-02 19:05 ` Dmitry Gutov
1 sibling, 1 reply; 16+ messages in thread
From: martin rudalics @ 2013-12-02 18:16 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017
> What does help, is changing `windmove-window-distance-delta' from 1 to
> 2. So the problem is likely to do with off-by-one errors in `window-edges'.
In a sense, yes. When I assign columns and lines from pixel sizes I
have to round sometimes. IIRC I tried to make the sums and the adjacent
edges fit but maybe I failed or there's another issue involved. I'll
look into this as soon as I sorted out bug 16013 (unless you wanted to
have a look).
martin
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-02 14:38 ` Stefan Monnier
@ 2013-12-02 18:16 ` martin rudalics
0 siblings, 0 replies; 16+ messages in thread
From: martin rudalics @ 2013-12-02 18:16 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 16017, Dmitry Gutov
> BTW, shouldn't windmove use window-in-direction?
It could but IIRC there are some differences. I'll look into it.
martin
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-02 18:16 ` martin rudalics
@ 2013-12-02 19:05 ` Dmitry Gutov
2013-12-03 11:44 ` martin rudalics
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2013-12-02 19:05 UTC (permalink / raw)
To: martin rudalics; +Cc: 16017
On 02.12.2013 20:16, martin rudalics wrote:
> I'll into this as soon as I sorted out bug 16013 (unless you wanted to
> have a look).
Please do. I'm sure you understand the problem area better, and there
are other issues I'd rather devote attention to.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-02 19:05 ` Dmitry Gutov
@ 2013-12-03 11:44 ` martin rudalics
2013-12-04 0:58 ` Dmitry Gutov
0 siblings, 1 reply; 16+ messages in thread
From: martin rudalics @ 2013-12-03 11:44 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017
[-- Attachment #1: Type: text/plain, Size: 2231 bytes --]
>> I'll into this as soon as I sorted out bug 16013 (unless you wanted to
>> have a look).
>
> Please do. I'm sure you understand the problem area better, and there
> are other issues I'd rather devote attention to.
IIUC the bug happens only in a window whose subwindow does not have an
integral number of lines like in a maximized frame. Is that correct?
Here on a maximized frame with two windows above each other and the
upper one selected the following happens
(window-height) => 30
(window-pixel-height) => 484
so the selected window has 484 / 16 lines which is larger than 30.
Now `windmove-other-window-loc' with DIR 'down does
(+ (1- (nth 3 edges))
windmove-window-distance-delta))) ; (x, y1+d-1)
and returns 30. `window-at' returns the selected window since that
window extends until after line 30 and windmove doesn't move because the
target window is the selected one.
In the long run I'd like to use `window-in-direction' here because it
doesn't suffer this problem. Meanwhile I can offer the attached patch
(although I'm not sure whether it is really better than customizing
`windmove-window-distance-delta').
martin
BTW: If you want to test `window-in-direction' directly you can try the
following instead:
(defun select-window-on-left ()
"Select window on the left."
(interactive)
(select-window (window-in-direction 'left)))
(defun select-window-above ()
"Select window above."
(interactive)
(select-window (window-in-direction 'above)))
(defun select-window-on-right ()
"Select window on the right."
(interactive)
(select-window (window-in-direction 'right)))
(defun select-window-below ()
"Select window below."
(interactive)
(select-window (window-in-direction 'below)))
(defun select-window-keybindings (&optional modifier)
"Set up keybindings as in windmove."
(interactive)
(unless modifier (setq modifier 'shift))
(global-set-key (vector (list modifier 'left)) 'select-window-on-left)
(global-set-key (vector (list modifier 'right)) 'select-window-on-right)
(global-set-key (vector (list modifier 'up)) 'select-window-above)
(global-set-key (vector (list modifier 'down)) 'select-window-below))
[-- Attachment #2: windmove.diff --]
[-- Type: text/plain, Size: 1642 bytes --]
=== modified file 'lisp/windmove.el'
--- lisp/windmove.el 2013-01-01 09:11:05 +0000
+++ lisp/windmove.el 2013-12-03 10:19:43 +0000
@@ -438,24 +438,28 @@
to. DIR is one of `left', `up', `right', or `down'; an optional ARG
is handled as by `windmove-reference-loc'; WINDOW is the window that
movement is relative to."
- (let ((edges (window-edges window)) ; edges: (x0, y0, x1, y1)
+ (let ((edges (window-pixel-edges window)) ; edges: (x0, y0, x1, y1)
(refpoint (windmove-reference-loc arg window))) ; (x . y)
(cond
((eq dir 'left)
- (cons (- (nth 0 edges)
+ (cons (- (ceiling (nth 0 edges)
+ (frame-char-width (window-frame window)))
windmove-window-distance-delta)
(cdr refpoint))) ; (x0-d, y)
((eq dir 'up)
(cons (car refpoint)
- (- (nth 1 edges)
+ (- (ceiling (nth 1 edges)
+ (frame-char-height (window-frame window)))
windmove-window-distance-delta))) ; (x, y0-d)
((eq dir 'right)
- (cons (+ (1- (nth 2 edges)) ; -1 to get actual max x
+ (cons (+ (1- (ceiling (nth 2 edges)
+ (frame-char-width (window-frame window)))) ; -1 to get actual max x
windmove-window-distance-delta)
(cdr refpoint))) ; (x1+d-1, y)
((eq dir 'down) ; -1 to get actual max y
(cons (car refpoint)
- (+ (1- (nth 3 edges))
+ (+ (1- (ceiling (nth 3 edges)
+ (frame-char-height (window-frame window))))
windmove-window-distance-delta))) ; (x, y1+d-1)
(t (error "Invalid direction of movement: %s" dir)))))
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-03 11:44 ` martin rudalics
@ 2013-12-04 0:58 ` Dmitry Gutov
2013-12-04 14:31 ` martin rudalics
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2013-12-04 0:58 UTC (permalink / raw)
To: martin rudalics; +Cc: 16017
On 03.12.2013 13:44, martin rudalics wrote:
> IIUC the bug happens only in a window whose subwindow does not have an
> integral number of lines like in a maximized frame. Is that correct?
Seems so. On my machine, the problem is there when the window is
maximized (and the frame has non-integral number of lines), but
disappears when Emacs is in fullscreen mode, without window borders and
everything (then the frame height looks evenly divisible by line height).
> In the long run I'd like to use `window-in-direction' here because it
> doesn't suffer this problem. Meanwhile I can offer the attached patch
> (although I'm not sure whether it is really better than customizing
> `windmove-window-distance-delta').
The patch works, except it doesn't let me move to the minibuffer window
when it's active, and doesn't say so when the minibuffer is inactive
(and it should, according to `windmove-do-window-select'). Changing the
value of `windmove-window-distance-delta' doesn't help with it either,
so looks like the patch is indeed equivalent to that.
> BTW: If you want to test `window-in-direction' directly you can try the
> following instead:
Thank you, this is better. The minibuffer is selectable this way, and
other cases of window navigation work, too.
The "no window there" feedback is non-existent (error "Wrong type
argument: window-live-p, nil"), but that's to be expected, I guess.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-04 0:58 ` Dmitry Gutov
@ 2013-12-04 14:31 ` martin rudalics
2013-12-04 22:13 ` Dmitry Gutov
0 siblings, 1 reply; 16+ messages in thread
From: martin rudalics @ 2013-12-04 14:31 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017
> The patch works, except it doesn't let me move to the minibuffer window
> when it's active, and doesn't say so when the minibuffer is inactive
> (and it should, according to `windmove-do-window-select'). Changing the
> value of `windmove-window-distance-delta' doesn't help with it either,
> so looks like the patch is indeed equivalent to that.
I nevertheless checked it in to allow others basic navigation. I'll
look into the minibuffer case later.
> The "no window there" feedback is non-existent (error "Wrong type
> argument: window-live-p, nil"), but that's to be expected, I guess.
Certainly not (the function I use was too complex to post here): Try
(defun select-window-on-left ()
"Select window on the left."
(interactive)
(let ((window (window-in-direction 'left)))
(when window (select-window window))))
martin
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-04 14:31 ` martin rudalics
@ 2013-12-04 22:13 ` Dmitry Gutov
2013-12-05 6:59 ` martin rudalics
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2013-12-04 22:13 UTC (permalink / raw)
To: martin rudalics; +Cc: 16017
On 04.12.2013 16:31, martin rudalics wrote:
> I nevertheless checked it in to allow others basic navigation. I'll
> look into the minibuffer case later.
I thought you might go ahead and just port windmove to use
`window-in-direction'.
> Certainly not (the function I use was too complex to post here): Try
Thanks, but I didn't mean I couldn't fix it myself. Rather that I didn't
expect much error-handling from those short snippets.
They work well, by the way. winner doesn't do anything essential that
those commands don't do, as far as I can see.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-04 22:13 ` Dmitry Gutov
@ 2013-12-05 6:59 ` martin rudalics
2013-12-05 10:53 ` Dmitry Gutov
0 siblings, 1 reply; 16+ messages in thread
From: martin rudalics @ 2013-12-05 6:59 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017
> I thought you might go ahead and just port windmove to use
> `window-in-direction'.
windmove has an extra ARG to distinguish the upper left corner of a
window and its position of point as reference points. I can't easily
dismiss that.
>> Certainly not (the function I use was too complex to post here): Try
>
> Thanks, but I didn't mean I couldn't fix it myself. Rather that I didn't
> expect much error-handling from those short snippets.
>
> They work well, by the way. winner doesn't do anything essential that
> those commands don't do, as far as I can see.
Do you mean winner or windmove?
martin
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-05 6:59 ` martin rudalics
@ 2013-12-05 10:53 ` Dmitry Gutov
2013-12-13 17:23 ` martin rudalics
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2013-12-05 10:53 UTC (permalink / raw)
To: martin rudalics; +Cc: 16017
On 05.12.2013 08:59, martin rudalics wrote:
> windmove has an extra ARG to distinguish the upper left corner of a
> window and its position of point as reference points. I can't easily
> dismiss that.
Ah, yes, it has. I was not aware.
> Do you mean winner or windmove?
Sorry, windmove, of course.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-05 10:53 ` Dmitry Gutov
@ 2013-12-13 17:23 ` martin rudalics
2013-12-15 13:49 ` Dmitry Gutov
0 siblings, 1 reply; 16+ messages in thread
From: martin rudalics @ 2013-12-13 17:23 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017
> ..., windmove, of course.
Should work again now including wrapping, prefix argument, and
original error messages. Please have a look.
Thanks, martin
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-13 17:23 ` martin rudalics
@ 2013-12-15 13:49 ` Dmitry Gutov
2013-12-16 10:13 ` martin rudalics
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2013-12-15 13:49 UTC (permalink / raw)
To: martin rudalics; +Cc: 16017-done
On 13.12.2013 19:23, martin rudalics wrote:
> Should work again now including wrapping, prefix argument, and
> original error messages. Please have a look.
Thank you, everything seems to work.
The prefix argument handling is weird (`C-u 4' has effect, just `C-u'
doesn't), but that's likely old behavior.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#16017: 24.3.50; windmove is broken
2013-12-15 13:49 ` Dmitry Gutov
@ 2013-12-16 10:13 ` martin rudalics
0 siblings, 0 replies; 16+ messages in thread
From: martin rudalics @ 2013-12-16 10:13 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16017-done
> The prefix argument handling is weird (`C-u 4' has effect, just `C-u'
> doesn't), but that's likely old behavior.
IIUC it's explained in the doc-strings as "With no prefix argument, or
with prefix argument equal to zero, ..." so we probably shouldn't change
it.
martin
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-12-16 10:13 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-01 1:52 bug#16017: 24.3.50; windmove is broken Dmitry Gutov
2013-12-02 7:38 ` martin rudalics
2013-12-02 10:29 ` Dmitry Gutov
2013-12-02 14:38 ` Stefan Monnier
2013-12-02 18:16 ` martin rudalics
2013-12-02 18:16 ` martin rudalics
2013-12-02 19:05 ` Dmitry Gutov
2013-12-03 11:44 ` martin rudalics
2013-12-04 0:58 ` Dmitry Gutov
2013-12-04 14:31 ` martin rudalics
2013-12-04 22:13 ` Dmitry Gutov
2013-12-05 6:59 ` martin rudalics
2013-12-05 10:53 ` Dmitry Gutov
2013-12-13 17:23 ` martin rudalics
2013-12-15 13:49 ` Dmitry Gutov
2013-12-16 10:13 ` martin rudalics
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).