On Wed, 7 Nov 2018 00:07:59 +0100
Amadeusz Sławiński <am...@asmblr.net> wrote:
> On Fri, 2 Nov 2018 00:04:20 -0500
> Ethan Warth <redyoshi...@gmail.com> 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 <am...@asmblr.net> wrote:
> >
> > >
> > > On Mon, 22 Oct 2018 20:21:16 -0500
> > > Ethan Warth <redyoshi...@gmail.com> 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
>
Did the fixes myself and pushed to master.
Thanks for patches!
Amadeusz
