unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#31988: scroll-other-window broken on master
@ 2018-06-27 19:54 Daniel Colascione
  2018-06-27 20:44 ` Daniel Colascione
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Colascione @ 2018-06-27 19:54 UTC (permalink / raw)
  To: 31988

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.






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-06-27 19:54 bug#31988: scroll-other-window broken on master Daniel Colascione
@ 2018-06-27 20:44 ` Daniel Colascione
  2018-06-27 21:26   ` Basil L. Contovounesios
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Colascione @ 2018-06-27 20:44 UTC (permalink / raw)
  To: Daniel Colascione, Basil L. Contovounesios; +Cc: 31988

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.






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-06-27 20:44 ` Daniel Colascione
@ 2018-06-27 21:26   ` Basil L. Contovounesios
  2018-07-06 15:18     ` Daniel Colascione
  0 siblings, 1 reply; 10+ messages in thread
From: Basil L. Contovounesios @ 2018-06-27 21:26 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 31988

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

tags 31988 patch
quit

"Daniel Colascione" <dancol@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?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-other-window-scroll-when-showing-same-buffer.patch --]
[-- Type: text/x-diff, Size: 986 bytes --]

From 026b6befc40795d8597e036f930f634b2a17e4f3 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@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


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


Thanks,

-- 
Basil

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-06-27 21:26   ` Basil L. Contovounesios
@ 2018-07-06 15:18     ` Daniel Colascione
  2018-07-06 22:20       ` Basil L. Contovounesios
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Colascione @ 2018-07-06 15:18 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 31988

> "Daniel Colascione" <dancol@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?






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-07-06 15:18     ` Daniel Colascione
@ 2018-07-06 22:20       ` Basil L. Contovounesios
  2018-07-06 23:31         ` Daniel Colascione
  0 siblings, 1 reply; 10+ messages in thread
From: Basil L. Contovounesios @ 2018-07-06 22:20 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 31988

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

>> "Daniel Colascione" <dancol@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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-07-06 22:20       ` Basil L. Contovounesios
@ 2018-07-06 23:31         ` Daniel Colascione
  2018-07-07  7:48           ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Colascione @ 2018-07-06 23:31 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 31988

close 31988
quit

> "Daniel Colascione" <dancol@dancol.org> writes:
>
>>> "Daniel Colascione" <dancol@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!








^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-07-06 23:31         ` Daniel Colascione
@ 2018-07-07  7:48           ` Eli Zaretskii
  2018-07-07 13:40             ` Basil L. Contovounesios
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2018-07-07  7:48 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: contovob, 31988

> Date: Fri, 6 Jul 2018 16:31:24 -0700
> From: "Daniel Colascione" <dancol@dancol.org>
> Cc: 31988@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.)





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-07-07  7:48           ` Eli Zaretskii
@ 2018-07-07 13:40             ` Basil L. Contovounesios
  2018-07-07 13:50               ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Basil L. Contovounesios @ 2018-07-07 13:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 31988, 31988-done

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Fri, 6 Jul 2018 16:31:24 -0700
>> From: "Daniel Colascione" <dancol@dancol.org>
>> Cc: 31988@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@debbugs.gnu.org>; if this was the wrong thing to do, please
let me know what I should have done instead.

Thanks again,

-- 
Basil





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-07-07 13:40             ` Basil L. Contovounesios
@ 2018-07-07 13:50               ` Eli Zaretskii
  2018-07-07 14:14                 ` Basil L. Contovounesios
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2018-07-07 13:50 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 31988

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Daniel Colascione <dancol@dancol.org>, <31988@debbugs.gnu.org>, <31988-done@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@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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#31988: scroll-other-window broken on master
  2018-07-07 13:50               ` Eli Zaretskii
@ 2018-07-07 14:14                 ` Basil L. Contovounesios
  0 siblings, 0 replies; 10+ messages in thread
From: Basil L. Contovounesios @ 2018-07-07 14:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 31988

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: Daniel Colascione <dancol@dancol.org>, <31988@debbugs.gnu.org>,
>> <31988-done@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@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





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-07-07 14:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-27 19:54 bug#31988: scroll-other-window broken on master Daniel Colascione
2018-06-27 20:44 ` Daniel Colascione
2018-06-27 21:26   ` Basil L. Contovounesios
2018-07-06 15:18     ` Daniel Colascione
2018-07-06 22:20       ` Basil L. Contovounesios
2018-07-06 23:31         ` Daniel Colascione
2018-07-07  7:48           ` Eli Zaretskii
2018-07-07 13:40             ` Basil L. Contovounesios
2018-07-07 13:50               ` Eli Zaretskii
2018-07-07 14:14                 ` Basil L. Contovounesios

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).