bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Daimrod]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

Hi,

The attached patch prevents `rcirc-omit-mode' to call `recenter' when
`window-buffer' isn't the same as `current-buffer'.


If `rcirc-omit-mode' is enabled by the user interactively (e.g. with C-c
C-o) then it will do the right thing.

If `rcirc-omit-mode' was called from elisp (e.g. from a hook), then we
don't need to `recenter' the buffer, because it will be done the next
time the user switch to the buffer with `rcirc-next-active-buffer'.

I'm not an RCIRC expert, but I don't think anything else is required.
WDYT?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-net-rcirc.el-rcirc-omit-mode-Do-not-recenter-if.patch --]
[-- Type: text/x-diff, Size: 1074 bytes --]

From 7bb493830bb0aaebeab69851ed044a5562875641 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gr=C3=A9goire=20Jadi?= <gregoire.jadi@gmail.com>
Date: Fri, 13 Jun 2014 14:12:52 +0900
Subject: [PATCH] * lisp/net/rcirc.el (rcirc-omit-mode): Do not `recenter' if
 the `current-buffer' isn't the buffer displayed by the selected window.

---
 lisp/net/rcirc.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index 41cc002..7bd7e93 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -1903,7 +1903,8 @@ Uninteresting lines are those whose responses are listed in
 	(message "Rcirc-Omit mode enabled"))
     (remove-from-invisibility-spec '(rcirc-omit . nil))
     (message "Rcirc-Omit mode disabled"))
-    (recenter (when (> (point) rcirc-prompt-start-marker) -1)))
+  (when (eq (current-buffer) (window-buffer))
+    (recenter (when (> (point) rcirc-prompt-start-marker) -1))))
 
 (defun rcirc-switch-to-server-buffer ()
   "Switch to the server buffer associated with current channel buffer."
-- 
1.8.0.2722.gc0242e5


[-- Attachment #3: Type: text/plain, Size: 17 bytes --]


--
Daimrod/Greg

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Eli Zaretskii]

> From: Daimrod <daimrod@gmail.com>
> Date: Fri, 13 Jun 2014 15:59:33 +0900
> 
> The attached patch prevents `rcirc-omit-mode' to call `recenter' when
> `window-buffer' isn't the same as `current-buffer'.
> 
> 
> If `rcirc-omit-mode' is enabled by the user interactively (e.g. with C-c
> C-o) then it will do the right thing.
> 
> If `rcirc-omit-mode' was called from elisp (e.g. from a hook), then we
> don't need to `recenter' the buffer, because it will be done the next
> time the user switch to the buffer with `rcirc-next-active-buffer'.
> 
> I'm not an RCIRC expert, but I don't think anything else is required.
> WDYT?

This doesn't sound like the right fix to me.  Instead, the code should
momentarily switch to the right window, and call recenter there.
That's surely what the author of this code intended, AFAIU.

Thanks.

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Daimrod]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daimrod <daimrod@gmail.com>
>> Date: Fri, 13 Jun 2014 15:59:33 +0900
>> 
>> The attached patch prevents `rcirc-omit-mode' to call `recenter' when
>> `window-buffer' isn't the same as `current-buffer'.
>> 
>> 
>> If `rcirc-omit-mode' is enabled by the user interactively (e.g. with C-c
>> C-o) then it will do the right thing.
>> 
>> If `rcirc-omit-mode' was called from elisp (e.g. from a hook), then we
>> don't need to `recenter' the buffer, because it will be done the next
>> time the user switch to the buffer with `rcirc-next-active-buffer'.
>> 
>> I'm not an RCIRC expert, but I don't think anything else is required.
>> WDYT?
>
> This doesn't sound like the right fix to me.  Instead, the code should
> momentarily switch to the right window, and call recenter there.
> That's surely what the author of this code intended, AFAIU.

But the buffer might not be displayed in any window.

For example, when one invoked `rcirc-connect' with some channels by
default:
: (rcirc-connect "localhost" 6667 nil nil nil '("#bar") nil)

Then a buffer "#bar@localhost" will be created but not displayed.

Regards,

-- 
Daimrod/Greg

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Eli Zaretskii]

> From: Daimrod <daimrod@gmail.com>
> Cc: 17769@debbugs.gnu.org
> Date: Fri, 13 Jun 2014 18:14:09 +0900
> 
> > This doesn't sound like the right fix to me.  Instead, the code should
> > momentarily switch to the right window, and call recenter there.
> > That's surely what the author of this code intended, AFAIU.
> 
> But the buffer might not be displayed in any window.

Then with-current-buffer is your friend, I think.

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Daimrod]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daimrod <daimrod@gmail.com>
>> Cc: 17769@debbugs.gnu.org
>> Date: Fri, 13 Jun 2014 18:14:09 +0900
>> 
>> > This doesn't sound like the right fix to me.  Instead, the code should
>> > momentarily switch to the right window, and call recenter there.
>> > That's surely what the author of this code intended, AFAIU.
>> 
>> But the buffer might not be displayed in any window.
>
> Then with-current-buffer is your friend, I think.

IIUC the goal isn't to recenter the window-buffer but the
current-buffer, but that cannot be done because it is not displayed on
any windows.

So, I with-current-buffer won't help, I would need to use
with-current-buffer-window.

Or did I misundertand you?

-- 
Daimrod/Greg

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Eli Zaretskii]

> From: Daimrod <daimrod@gmail.com>
> Cc: 17769@debbugs.gnu.org
> Date: Fri, 13 Jun 2014 18:54:04 +0900
> 
> IIUC the goal isn't to recenter the window-buffer but the
> current-buffer, but that cannot be done because it is not displayed on
> any windows.

Then don't recenter if it is not displayed in any window on any frame.
But if it is displayed in some window, recenter that window.

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Stefan Monnier]

> But the buffer might not be displayed in any window.

Then I guess it should check that get-buffer-window does return a window
and do nothing if it doesn't.


        Stefan

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[martin rudalics]

 > But if it is displayed in some window, recenter that window.

Strictly spoken this should read "But if it is displayed in the selected
window, recenter that window."

martin

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Eli Zaretskii]

> Date: Fri, 13 Jun 2014 14:51:49 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: 17769@debbugs.gnu.org
> 
>  > But if it is displayed in some window, recenter that window.
> 
> Strictly spoken this should read "But if it is displayed in the selected
> window, recenter that window."

No, because in that case it would already have been done by a normal
call to recenter.

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[martin rudalics]

 >> Strictly spoken this should read "But if it is displayed in the selected
 >> window, recenter that window."
 >
 > No, because in that case it would already have been done by a normal
 > call to recenter.

Unless the selected window displayed another buffer.  Then you probably
mean something like

   (dolist (window (get-buffer-window-list buffer))
     (with-selected-window window (recenter)))

martin

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Daimrod]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

martin rudalics <rudalics@gmx.at> writes:

>>> Strictly spoken this should read "But if it is displayed in the selected
>>> window, recenter that window."
>>
>> No, because in that case it would already have been done by a normal
>> call to recenter.
>
> Unless the selected window displayed another buffer.  Then you probably
> mean something like
>
>   (dolist (window (get-buffer-window-list buffer))
>     (with-selected-window window (recenter)))

I see, thanks for inputs.

Here is an updated patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-net-rcirc.el-rcirc-omit-mode-recenter-the-corre.patch --]
[-- Type: text/x-diff, Size: 1066 bytes --]

From c9fb7786be4fd929ad81861b3ada47d8d2f385e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gr=C3=A9goire=20Jadi?= <gregoire.jadi@gmail.com>
Date: Sat, 14 Jun 2014 00:57:06 +0900
Subject: [PATCH] * lisp/net/rcirc.el (rcirc-omit-mode): `recenter' the correct
 windows.

---
 lisp/net/rcirc.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index 41cc002..a0e72d1 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -1903,7 +1903,9 @@ Uninteresting lines are those whose responses are listed in
 	(message "Rcirc-Omit mode enabled"))
     (remove-from-invisibility-spec '(rcirc-omit . nil))
     (message "Rcirc-Omit mode disabled"))
-    (recenter (when (> (point) rcirc-prompt-start-marker) -1)))
+  (dolist (window (get-buffer-window-list (current-buffer)))
+    (with-selected-window window
+      (recenter (when (> (point) rcirc-prompt-start-marker) -1)))))
 
 (defun rcirc-switch-to-server-buffer ()
   "Switch to the server buffer associated with current channel buffer."
-- 
1.8.0.2722.gc0242e5


[-- Attachment #3: Type: text/plain, Size: 17 bytes --]


--
Daimrod/Greg

bug#17769: 24.4.50; [PATCH] rcirc-omit-mode: `recenter'ing a window that does not display current-buffer.

[Leo Liu]

Fixed in 24.5.

On 2014-06-14 01:08 +0900, Daimrod wrote:
> I see, thanks for inputs.
>
> Here is an updated patch.

Thanks for the patch.

Leo