GNU bug report logs - #31988
scroll-other-window broken on master

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31988 in the body.
You can then email your comments to 31988 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Daniel Colascione" <dancol <at> dancol.org>
To: bug-gnu-emacs <at> gnu.org
Subject: scroll-other-window broken on master
Date: Wed, 27 Jun 2018 12:54:03 -0700

scroll-other-window scrolls the current window _and_ the other window. To
repro, visit a big file, C-x 2, and mash C-M-v. Only the other window
should scroll. Now, both windows scroll.

Message #8 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: "Daniel Colascione" <dancol <at> dancol.org>
To: "Daniel Colascione" <dancol <at> dancol.org>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Wed, 27 Jun 2018 13:44:47 -0700

Basil, it looks like you recently rewrote a big chunk of the window
scrolling code. Can you please take a look?

Thanks.

> scroll-other-window scrolls the current window _and_ the other window. To
> repro, visit a big file, C-x 2, and mash C-M-v. Only the other window
> should scroll. Now, both windows scroll.

Message #11 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: "Daniel Colascione" <dancol <at> dancol.org>
Cc: 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Thu, 28 Jun 2018 00:26:33 +0300

[Message part 1 (text/plain, inline)]

tags 31988 patch
quit

"Daniel Colascione" <dancol <at> dancol.org> writes:

> Basil, it looks like you recently rewrote a big chunk of the window
> scrolling code. Can you please take a look?
>
> Thanks.
>
>> scroll-other-window scrolls the current window _and_ the other window. To
>> repro, visit a big file, C-x 2, and mash C-M-v. Only the other window
>> should scroll. Now, both windows scroll.

This bug was indeed introduced by my recent refactor.
Does the following patch fix it?

[0001-Fix-other-window-scroll-when-showing-same-buffer.patch (text/x-diff, inline)]

From 026b6befc40795d8597e036f930f634b2a17e4f3 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Date: Thu, 28 Jun 2018 00:13:07 +0300
Subject: [PATCH] Fix other window scroll when showing same buffer

src/window.c (scroll_command): Make other window's buffer current
before scrolling, even when displaying the same buffer. (bug#31988)
---
 src/window.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/window.c b/src/window.c
index 81fd7f2b47..42b9522862 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5654,7 +5654,7 @@ scroll_command (Lisp_Object window, Lisp_Object n, int direction)
 
   /* If given window's buffer isn't current, make it current for
      the moment.  But don't screw up if window_scroll gets an error.  */
-  if (XBUFFER (w->contents) != current_buffer)
+  if (other_window || XBUFFER (w->contents) != current_buffer)
     {
       record_unwind_protect_excursion ();
       Fset_buffer (w->contents);
-- 
2.18.0

[Message part 3 (text/plain, inline)]

Thanks,

-- 
Basil

Message #16 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: "Daniel Colascione" <dancol <at> dancol.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Daniel Colascione <dancol <at> dancol.org>, 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Fri, 6 Jul 2018 08:18:14 -0700

> "Daniel Colascione" <dancol <at> dancol.org> writes:
>
>> Basil, it looks like you recently rewrote a big chunk of the window
>> scrolling code. Can you please take a look?
>>
>> Thanks.
>>
>>> scroll-other-window scrolls the current window _and_ the other window.
>>> To
>>> repro, visit a big file, C-x 2, and mash C-M-v. Only the other window
>>> should scroll. Now, both windows scroll.
>
> This bug was indeed introduced by my recent refactor.
> Does the following patch fix it?

That works. Thanks! Do you want to apply the patch?

Message #19 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: "Daniel Colascione" <dancol <at> dancol.org>
Cc: 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Sat, 07 Jul 2018 01:20:54 +0300

"Daniel Colascione" <dancol <at> dancol.org> writes:

>> "Daniel Colascione" <dancol <at> dancol.org> writes:
>>
>>> Basil, it looks like you recently rewrote a big chunk of the window
>>> scrolling code. Can you please take a look?
>>>
>>> Thanks.
>>>
>>>> scroll-other-window scrolls the current window _and_ the other window.
>>>> To
>>>> repro, visit a big file, C-x 2, and mash C-M-v. Only the other window
>>>> should scroll. Now, both windows scroll.
>>
>> This bug was indeed introduced by my recent refactor.
>> Does the following patch fix it?
>
> That works. Thanks! Do you want to apply the patch?

I don't have push access, so someone else will have to apply the patch
for me if there are no objections to it.

Thanks,

-- 
Basil

Message #22 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: "Daniel Colascione" <dancol <at> dancol.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Daniel Colascione <dancol <at> dancol.org>, 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Fri, 6 Jul 2018 16:31:24 -0700

close 31988
quit

> "Daniel Colascione" <dancol <at> dancol.org> writes:
>
>>> "Daniel Colascione" <dancol <at> dancol.org> writes:
>>>
>>>> Basil, it looks like you recently rewrote a big chunk of the window
>>>> scrolling code. Can you please take a look?
>>>>
>>>> Thanks.
>>>>
>>>>> scroll-other-window scrolls the current window _and_ the other
>>>>> window.
>>>>> To
>>>>> repro, visit a big file, C-x 2, and mash C-M-v. Only the other window
>>>>> should scroll. Now, both windows scroll.
>>>
>>> This bug was indeed introduced by my recent refactor.
>>> Does the following patch fix it?
>>
>> That works. Thanks! Do you want to apply the patch?
>
> I don't have push access, so someone else will have to apply the patch
> for me if there are no objections to it.

I just applied it. Thanks!

Message #25 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Daniel Colascione" <dancol <at> dancol.org>
Cc: contovob <at> tcd.ie, 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Sat, 07 Jul 2018 10:48:06 +0300

> Date: Fri, 6 Jul 2018 16:31:24 -0700
> From: "Daniel Colascione" <dancol <at> dancol.org>
> Cc: 31988 <at> debbugs.gnu.org
> 
> > I don't have push access, so someone else will have to apply the patch
> > for me if there are no objections to it.
> 
> I just applied it. Thanks!

Thanks, but why didn't you use the commit log Basil provided?  (It was
somewhat inaccurate, but more informative than what you used, IMO.)

Message #28 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Daniel Colascione <dancol <at> dancol.org>, 31988-done <at> debbugs.gnu.org,
 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Sat, 07 Jul 2018 16:40:17 +0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Fri, 6 Jul 2018 16:31:24 -0700
>> From: "Daniel Colascione" <dancol <at> dancol.org>
>> Cc: 31988 <at> debbugs.gnu.org
>> 
>> > I don't have push access, so someone else will have to apply the patch
>> > for me if there are no objections to it.
>> 
>> I just applied it. Thanks!
>
> Thanks, but why didn't you use the commit log Basil provided?  (It was
> somewhat inaccurate, but more informative than what you used, IMO.)

Thanks Daniel for applying the patch and Eli for your subsequent
efficiency and documentation fix.

I don't yet feel particularly good at or confident with writing
changelog entries, let alone Emacs C code, so any guidance is more than
welcome now and in the future.

I'm not sure whether Daniel's addition of the 'close' tag had the
intended effect, so I'm taking the liberty of CCing
<31988-done <at> debbugs.gnu.org>; if this was the wrong thing to do, please
let me know what I should have done instead.

Thanks again,

-- 
Basil

Message #36 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: dancol <at> dancol.org, 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Sat, 07 Jul 2018 16:50:47 +0300

> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
> Cc: Daniel Colascione <dancol <at> dancol.org>, <31988 <at> debbugs.gnu.org>, <31988-done <at> debbugs.gnu.org>
> Date: Sat, 07 Jul 2018 16:40:17 +0300
> 
> I don't yet feel particularly good at or confident with writing
> changelog entries, let alone Emacs C code, so any guidance is more than
> welcome now and in the future.

I think you are doing just fine, thank you.  There's always some
learning curve, of course.  CONTRIBUTE should tell you enough, and you
can study the log messages by veterans to pick up the spirit.

The "inaccurate" part in my message related not to your wording (which
was fine, AFAICT), but to the fact that the important part of your fix
was to call save-excursion in the described case, not the part that
set the buffer (which was actually a no-op).

> I'm not sure whether Daniel's addition of the 'close' tag had the
> intended effect

It should have, but it looks like it didn't for some reason.

> so I'm taking the liberty of CCing <31988-done <at> debbugs.gnu.org>; if
> this was the wrong thing to do, please let me know what I should
> have done instead.

Doing that for a bug that should be closed can never do any harm, even
if it was already closed.  (You also don't need to CC both NNN@ and
NNN-done@, as the message gets recorded by the tracker even if you
only use the latter.)

Thanks.

Message #39 received at 31988 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dancol <at> dancol.org, 31988 <at> debbugs.gnu.org
Subject: Re: bug#31988: scroll-other-window broken on master
Date: Sat, 07 Jul 2018 17:14:29 +0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
>> Cc: Daniel Colascione <dancol <at> dancol.org>, <31988 <at> debbugs.gnu.org>,
>> <31988-done <at> debbugs.gnu.org>
>> Date: Sat, 07 Jul 2018 16:40:17 +0300
>> 
>> I don't yet feel particularly good at or confident with writing
>> changelog entries, let alone Emacs C code, so any guidance is more than
>> welcome now and in the future.
>
> I think you are doing just fine, thank you.  There's always some
> learning curve, of course.  CONTRIBUTE should tell you enough, and you
> can study the log messages by veterans to pick up the spirit.
>
> The "inaccurate" part in my message related not to your wording (which
> was fine, AFAICT), but to the fact that the important part of your fix
> was to call save-excursion in the described case, not the part that
> set the buffer (which was actually a no-op).
>
>> I'm not sure whether Daniel's addition of the 'close' tag had the
>> intended effect
>
> It should have, but it looks like it didn't for some reason.
>
>> so I'm taking the liberty of CCing <31988-done <at> debbugs.gnu.org>; if
>> this was the wrong thing to do, please let me know what I should
>> have done instead.
>
> Doing that for a bug that should be closed can never do any harm, even
> if it was already closed.  (You also don't need to CC both NNN@ and
> NNN-done@, as the message gets recorded by the tracker even if you
> only use the latter.)

Thanks for explaining everything, it all makes sense.

-- 
Basil

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.