"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: tog: key map to switch split mode
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Mon, 4 Jul 2022 00:21:36 +1000

Download raw body.

Thread
On 22-07-03 04:11pm, Omar Polo wrote:
> Mark Jamsek <mark@jamsek.com> wrote:
> > This introduces the 'S' global key map to switch the current splitscreen
> > mode.
> > 
> > If in a vsplit, it will switch to a hsplit.
> > If in a hsplit, it will switch to a vsplit IFF COLUMNS > 119 else the
> > view will remain unchanged (to honour existing vsplit behaviour).
> 
> it works fine for me, and I really like the idea.
> 
> ok op

Thanks, op!

> two small nit inline, feel free to discard it however.
> 
> > diff /home/mark/src/git/got-current
> > commit - 41e8d27de3dea9d283199eb2e1a7f6a98ee9286f
> > path + /home/mark/src/git/got-current
> > blob - 30d0b7d88189f5969dd01deb66e8f9196bac64d3
> > file + tog/tog.1
> > --- tog/tog.1
> > +++ tog/tog.1
> > @@ -80,6 +80,12 @@ Toggle fullscreen mode for a split-screen view.
> >  .Nm
> >  will automatically use split-screen views if the size of the terminal
> >  window is sufficiently large.
> > +.It Cm S
> > +When in a split-screen view,
> > +.Nm
> > +will switch to the alternate split mode.
> > +If the current view is in a horizontal split and the terminal window is
> > +less than 120 columns wide, the view will remain unchanged.
> 
> I'd avoid documenting the exact values.  Maybe saying "...and the
> terminal window is not wide enough, the view will remain unchanged."?

That's how I initially had it--similar wording to the fullscreen bit
right above (i.e., "if the terminal window is insufficiently wide").

I changed it to an explicit value so the user could know what size to
increase their terminal if they're in tog and the split isn't switching.
They might bring up `man tog` in another term and see they can just
widen their terminal to 120. That was my rationale, but you're likely
right about this.

> Better hear stsp opinion on this thought.
> 
> >  .El
> >  .Pp
> >  Global options must precede the command name, and are as follows:
> > @@ -582,7 +588,7 @@ work tree, use the repository path associated with thi
> >  .El
> >  .El
> >  .Sh ENVIRONMENT
> > -.Bl -tag -width TOG_DIFF_ALGORITHM
> > +.Bl -tag -width TOG_VIEW_SPLIT_MODE
> >  .It Ev TOG_DIFF_ALGORITHM
> >  Determines the default diff algorithm used by
> >  .Nm .
> > @@ -595,6 +601,18 @@ are
> >  and
> >  .Dq myers .
> >  If unset, the Myers diff algorithm will be used by default.
> > +.It Ev TOG_VIEW_SPLIT_MODE
> > +Determines the default split-screen mode used by
> > +.Nm .
> > +Valid values are
> > +.Dq h
> > +for horizontal
> > +and
> > +.Dq v
> > +for vertical.
> > +If unset, views will open in a vertical split-screen by default when the
> > +terminal window is sufficiently large.
> > +Splits can be manipulated in-session as documented above.
> >  .It Ev TOG_COLORS
> >  .Nm
> >  shows colorized output if this variable is set to a non-empty value.
> > blob - 95a351a042eb915c3180784da0a53b2c2d2abca1
> > file + tog/tog.c
> > --- tog/tog.c
> > +++ tog/tog.c
> > @@ -1004,7 +1004,69 @@ view_search_start(struct tog_view *view)
> >  	return NULL;
> >  }
> > 
> > +static void view_get_split(struct tog_view *, int *, int *);
> > +static const struct got_error *view_init_hsplit(struct tog_view *, int);
> > +
> >  /*
> > + * If view is a parent or child view and is currently in a splitscreen, switch
> > + * to the alternate split. If in a hsplit and LINES < 120, don't vsplit.
> > + */
> > +static const struct got_error *
> > +switch_split(struct tog_view *view)
> > +{
> > +	const struct got_error	*err = NULL;
> > +	struct tog_view		*v = NULL;
> > +
> > +	if (view_is_parent_view(view))
> > +		v = view;
> > +	else if (view->parent)
> > +		v = view->parent;
> 
> This reads a bit strange, one may think that there are cases where we
> keep `v' NULL and wonder why we dereference it immediately after.
> 
> I'd turn that 'else if' to an 'else' since a view (AFAIK) is either
> a parent or a child view.

And you're right about this one. That's a hold over; I had the parent
check first and swapped it down with ]e in vim. I'll fix this.

Thanks, op :)

> > +	if (!v->child || !view_is_splitscreen(v->child))
> > +		return NULL;
> > +	if (v->mode == TOG_VIEW_SPLIT_HRZN && v->cols < 120)
> > +		return NULL;
> > +
> 
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68