On 22-06-30 11:01am, Stefan Sperling 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.

After following the first four steps, I can't reproduce this^. Rather
than a splitscreen with a garbled rhs, the splitscreen is lost and the
parent (log) view goes to fullscreen.

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

This happens in my tests too.

> Patch below fixes this for me.

The patch fixes the return to splitscreen when increasing the terminal;
but I can't speak to the first problem at the end of step 4.

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

I was thinking about a basic test harness that performs a screen dump to
compare with the expected output. In the meantime, I'll get in the habit
of running tog at different dimensions to try catch some of these cases.

> -----------------------------------------------
> 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;
> +}
> +
>  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) {
> 

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