From: Omar Polo <op@omarpolo.com>
Subject: Re: small tog split view regression
To: Stefan Sperling <stsp@stsp.name>
Cc: Mark Jamsek <mark@jamsek.com>, gameoftrees@openbsd.org
Date: Thu, 30 Jun 2022 11:09:40 +0200

Stefan Sperling <stsp@stsp.name> wrote:
> On Thu, Jun 30, 2022 at 02:16:18AM +1000, Mark Jamsek wrote:
> > On 22-06-29 06:10pm, Stefan Sperling wrote:
> > > On Thu, Jun 30, 2022 at 01:36:24AM +1000, Mark Jamsek wrote:
> > > > > Yes, this is the right fix. I'd forgotten about this; op also mentioned
> > > > > this earlier. Thanks!
> > > > 
> > > > Actually, no. This breaks something else:
> > > > 
> > > > $ tog # term wide enough to vsplit
> > > > return # open commit
> > > > f # fullscreen commit
> > > > tab # should go to fullscreen log, but it splits the screen
> > > > 
> > > > So effectivley we can't get to a fullscreen parent view when a child
> > > > view is opened.
> > > 
> > > This fixes it for me.
> > 
> > Geez that was quick!
> > 
> > Yes, works for me too :)
> 
> Unfortunately, this fix introduced another problem:
> 
> - start tog in a large terminal
> - Enter (opens diff in split-screen view)
> - Tab (switch focus to parent)
> - Now make the terminal small
> 
> At this point, the parent (log) view is rendered in split-screen and
> the right area of the window where the child view was is garbled.
> 
> - Make the terminal large again
> 
> Now the split screen layout has been lost. You can still cycle between
> views with Tab but they both views remain fullscreen.
> 
> Patch below fixes this for me. However, my confidence in this code is
> a not as high as it once was. With recent fixes for wide-characters
> bleeding into vsplit the border, some assumptions have changed. The parent
> view was always a fullscreen view prior to those changes. I am not sure if
> all relevant code has been updated accordingly.
> We don't have automated tests for tog. It would be good if we could get
> some people to test tog a bit and look for more problems like this.
> 
> -----------------------------------------------
> commit 588acb5d9e16fca111ac97f30385d371a38f02fa (tog-fullscreen)
> from: Stefan Sperling <stsp@stsp.name>
> date: Thu Jun 30 08:51:09 2022 UTC
>  
>  fix fullscreen / split-screen toggle in tog
>  
> diff 9b058f456d15d60a89334ce3e7f0a7c22e182c55 588acb5d9e16fca111ac97f30385d371a38f02fa
> commit - 9b058f456d15d60a89334ce3e7f0a7c22e182c55
> commit + 588acb5d9e16fca111ac97f30385d371a38f02fa
> blob - 6a0c6dbff39ee1a8db173cbc3fa1fbf0918eecab
> blob + 7b60dfafbe237a00595400f2fff42d3f6377939c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -781,6 +781,12 @@ view_is_splitscreen(struct tog_view *view)
>  	return view->begin_x > 0 || view->begin_y > 0;
>  }
>  
> +static int
> +view_is_fullscreen(struct tog_view *view)
> +{
> +	return view->nlines == LINES && view->ncols == COLS;

still have to test the diff but there is a reason for LINES and COLS
here instead of view->lines/cols?  I'm worried that view->lines and
LINES could go out-of-sync temporarly during a resize.

> +}
> +
>  static void
>  view_border(struct tog_view *view)
>  {
> @@ -828,7 +834,7 @@ view_resize(struct tog_view *view)
>  	if (view->child) {
>  		int hs = view->child->begin_y;
>  
> -		if (view->child->focussed)
> +		if (!view_is_fullscreen(view))
>  			view->child->begin_x = view_split_begin_x(view->begin_x);
>  		if (view->mode == TOG_VIEW_SPLIT_HRZN ||
>  		    view->child->begin_x == 0) {