[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [screen-devel] Window group code contribution
From: |
Amadeusz Sławiński |
Subject: |
Re: [screen-devel] Window group code contribution |
Date: |
Wed, 7 Nov 2018 00:07:59 +0100 |
On Fri, 2 Nov 2018 00:04:20 -0500
Ethan Warth <address@hidden> wrote:
> I use window groups in my .screenrc. Window groups, as you likely already
> know,
> can be used to group other child windows (including other child window groups)
> together. Window groups do /not/ have their own corresponding shell, though;
> when you switch to a window group's window, you instead see the output of a
> windowlist command (as a consequence, that group window's children are listed,
> and can then be selected from).
>
> Windowlist has two toggles that affect its output: the list can be sorted in
> either index or MRU order, and the list can contain either just immediate
> children windows of all descendant windows. Since window groups use the same
> code for their own output, they also support these toggles.
>
> However, in the current code base, these toggles are reset to index ordering
> and
> display of immediate children only every time a particular window group is
> swapped out of and back into a pane. To instead make these display settings
> persist under such conditions on a per-window group basis, I made the
> following code changes:
>
> * I added two state booleans to the window structure to represent MRU ordering
> and recursive descendant listing for window groups (and added values for these
> two booleans to statically defined window structs in the code).
> * I made the windowlist initialization function ignore the passed in order
> flags
> if the windowlist is being generated for a window group, and I made the
> function
> instead use the window group's stored flags.
> * I made the sections of code that change the aforementioned flags for an
> active
> windowlist also change those flags for the corresponding window group (in the
> case that one exists, i.e., when the windowlist is being displayed as the
> window group's output on a pane).
>
> I tried to make the commit title sufficiently descriptive for the change, but
> I'm open to alternatives for such a description. I have also confirmed the
> trailing whitespace instances that you have alluded to in your response. I'm
> more than happy to submit modified patches to reflect both the potential
> commit description change and the removal of the trailing whitespaces; just
> let me know what (if anything) the first commit's description should be
> changed to.
>
>
> Perfect normality is impossible. Be unique!
> ―redyoshi49q
>
>
> On Thu, Nov 1, 2018 at 3:08 PM Amadeusz Sławiński <address@hidden> wrote:
> >
> > On Mon, 22 Oct 2018 20:21:16 -0500
> > Ethan Warth <address@hidden> wrote:
> >
> > > (I'm resending this, to the mailing list this time. I didn't notice that
> > > my
> > > reply wasn't actually going to the list.)
> > >
> > >
> > > Sorry; your message ended up in my spam folder, and I didn't notice it
> > > immediately.
> > >
> > > I will be including the git format-patch (both as inline text in this
> > > message
> > > and also as file attachments as a redundancy measure). This is my first
> > > time
> > > submitting git patches over email; let me know if I need to make another
> > > attempt at sending the commit patches.
> > >
> >
> > Hi,
> >
> > sorry for delay, few things:
> >
> > Can you describe, what is a goal of first patch? I've read it few times
> > and am a bit unsure. You can describe workflow which it fixes,
> > preferably it should be also included in commit message, so if someone
> > looks at commit later they know what it is about.
> >
> > There also seems to be few leftover whitespace characters at end of
> > lines in first and second patch, you should be able to see them using
> > "git show" command, they will be colored in red.
> >
> > Cheers,
> > Amadeusz
Hi,
Yes, I see now. This should be described in commit message, it's not
clear from topic alone what the patch is about.
At least those lines should be added to commit message:
> Windowlist has two toggles that affect its output: the list can be sorted in
> either index or MRU order, and the list can contain either just immediate
> children windows of all descendant windows. Since window groups use the same
> code for their own output, they also support these toggles.
>
> However, in the current code base, these toggles are reset to index ordering
> and
> display of immediate children only every time a particular window group is
> swapped out of and back into a pane.
As for the patch itself, it looks good, but I think there can be few
improvements.
Instead of naming variables "w_list_mru" and "w_list_group "
name them after variables they are correspong to, so: "w_list_order" and
"w_list_nested".
Also comments in patch:
diff --git a/src/list_window.c b/src/list_window.c
index 620f706..a6a5d02 100644
--- a/src/list_window.c
+++ b/src/list_window.c
@@ -278,6 +278,8 @@ static int gl_Window_input(ListData *ldata, char
**inp, size_t *len)
case 'm':
/* Toggle MRU-ness */
wdata->order = wdata->order == WLIST_MRU ? WLIST_NUM : WLIST_MRU;
+ if (wdata->group)
+ wdata->group->w_list_mru = wdata->order == WLIST_MRU;
Also here make assingment similar to the one for wdata->order few lines above.
glist_remove_rows(ldata);
gl_Window_rebuild(ldata);
break;
@@ -285,6 +287,8 @@ static int gl_Window_input(ListData *ldata, char
**inp, size_t *len)
case 'g':
/* Toggle nestedness */
wdata->nested = !wdata->nested;
+ if (wdata->group)
+ wdata->group->w_list_group = wdata->nested ? true : false;
glist_remove_rows(ldata);
gl_Window_rebuild(ldata);
break;
@@ -453,8 +457,11 @@ void display_windows(int onblank, int order, Window *group)
return;
}
- if (group)
+ if (group) {
onblank = 0; /* When drawing a group window, ignore 'onblank' */
+ order = (group->w_list_group ? WLIST_NESTED : 0) +
+ (group->w_list_mru ? 1 : 0); /* also ignore order flags */
In case of window order use defines here: ? WLIST_MRU : WLIST_NUM
Also added comment is bit confusing, because you are not actually
ignoring order flags, but rather restoring them
+ }
if (onblank) {
if (!display) {
diff --git a/src/window.c b/src/window.c
index e197390..b6d99fb 100644
--- a/src/window.c
+++ b/src/window.c
@@ -95,6 +95,8 @@ struct NewWindow nwin_undef = {
.aflag = false,
.dynamicaka = false,
.flowflag = -1,
+ .list_mru = false,
+ .list_group = false,
.lflag = -1,
.histheight = -1,
.monitor = -1,
@@ -121,6 +123,8 @@ struct NewWindow nwin_default = {
.aflag = false,
.dynamicaka = true,
.flowflag = FLOW_ON,
+ .list_mru = false,
+ .list_group = false,
.lflag = 1,
.histheight = DEFAULTHISTHEIGHT,
.monitor = MON_OFF,
@@ -153,6 +157,8 @@ void nwin_compose(struct NewWindow *def, struct
NewWindow *new, struct NewWindow
COMPOSE(aflag);
COMPOSE(dynamicaka);
COMPOSE(flowflag);
+ COMPOSE(list_mru);
+ COMPOSE(list_group);
COMPOSE(lflag);
COMPOSE(histheight);
COMPOSE(monitor);
@@ -556,6 +562,8 @@ int MakeWindow(struct NewWindow *newwin)
p->w_savelayer = &p->w_layer;
p->w_pdisplay = NULL;
p->w_lastdisp = NULL;
+ p->w_list_mru = nwin.list_mru;
+ p->w_list_group = nwin.list_group;
if (display && !AclCheckPermWin(D_user, ACL_WRITE, p))
p->w_wlockuser = D_user;
diff --git a/src/window.h b/src/window.h
index e6faaee..8989de3 100644
--- a/src/window.h
+++ b/src/window.h
@@ -50,6 +50,8 @@ struct NewWindow {
bool aflag;
bool dynamicaka;
int flowflag;
+ bool list_mru; /* MRU list order for window groups */
+ bool list_group; /* show nested children in window groups */
Those comments seem misaligned when I apply patch.
int lflag;
int histheight;
int monitor;
@@ -139,6 +141,8 @@ struct Window {
Window *w_next; /* next window */
Window *w_prev_mru; /* previous most recently used window */
int w_type; /* type of window */
+ bool w_list_mru; /* MRU list order for window groups */
+ bool w_list_group; /* show nested children in window groups */
Those also are misaligned.
Layer w_layer; /* our layer */
Layer *w_savelayer; /* the layer to keep */
int w_blocked; /* block input */
--
2.17.0
Cheers,
Amadeusz