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

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."?

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.

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