[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [screen-devel] [PATCH] Make RC_OTHER always choose some other
From: |
Juergen Weigert |
Subject: |
Re: [screen-devel] [PATCH] Make RC_OTHER always choose some other |
Date: |
Fri, 19 Sep 2008 12:41:37 +0200 |
User-agent: |
Mutt/1.5.7i |
On Sep 19, 08 00:46:19 -0700, Micah Cowan wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Sadrul Habib Chowdhury wrote:
> > * Jeroen Roovers had this to say on [18 Sep 2008, 18:52:11 +0200]:
> > [snip]
> >> Sometimes when you do RC_OTHER, you get presented with "This IS window
> >> (n)."
> >> That's rather stupid as you didn't even ask for window n in the first
> >> place,
> >> but simply _some_ other window. -JeR
> >>
> >> --- process.c.orig 2008-07-23 08:36:05.000000000 +0200
> >> +++ process.c 2008-09-18 18:21:19.000000000 +0200
> >> @@ -1731,7 +1731,7 @@
> >> /* FALLTHROUGH */
> >> case RC_OTHER:
> >> if (MoreWindows())
> >> - SwitchWindow(display && D_other ? D_other->w_number : NextWindow());
> >> + SwitchWindow(display && (D_other != fore) ? D_other->w_number :
> >> NextWindow());
> >> break;
> >> case RC_META:
> >> if (user->u_Esc == -1)
> >
> > The patch, as it is, looks good. Thanks!
I'd write
SwitchWindow(display && D_other && (D_other != fore) ? D_other->w_number :
NextWindow());
I see nothing here that asserts D_other being nonzero otherwise.
As suggested by Jeroen, I am afraid it could crash a fresh screen
session that only has one window.
> > But, perhaps a more appropriate fix would be to make sure that
> > display->d_other is updated appropriately? Do you have a screenrc file
> > that can be used to easily reproduce the bug?
Yes. I'd be very interested to learn the circumstances when the other
command would respond with "This IS window (n)" -- Never seen that happen.
> Also, I'm wondering whether it's still important to check whether
> D_other is null, as in the - line. I applied the patch and couldn't seem
> to get it to crash, but I'd have to look deeper at the code to know.
> OTOH, keeping the check against null isn't going to _hurt_ anything...
Please be save.
I don't think Jeroen meant to remove that assertion.
cheers,
Jw.
--
o \ Juergen Weigert unix-software __/ _=======.=======_
<V> | address@hidden creator __/ _---|____________\/
\ | 0179/2069677 __/ (____/ /\
(/) | ____________________________/ _/ \_ vim:set sw=2 wm=8