On 22-06-19 06:20pm, Omar Polo wrote:
> Mark Jamsek <mark@jamsek.com> wrote:
> > When in a child diff view in splitscreen, if you toggle fullscreen and
> > then cycle back to the log view with tab, the log view opens in
> > splitscreen with the diff view still visible and no border line.
> > 
> > repro:
> >  $ tog
> >  return
> >  f
> >  tab
> >  # log view opens in a split instead of fullscreen
> 
> good catch!
> 
> I broke this in 0dbbbe79, apologize, when changing the main view to be
> resized when entering the split mode.  I didn't know you could cycle
> fullscreen views, so I missed to test it.
> 
> > What used to happen is we would cycle to the log view in fullscreen.
> > 
> > The below patch appears to fix this while keeping the hscroll fix in
> > splitscreen when drawing double-width chars (tested in the ja.git repo).
> > 
> > This happens because we now resize views _after_ resetting nlines and
> > ncols based on the child start column in splitscreen, which sets the
> > parent view->ncols to the child view's splitscreen start column.
> > 
> > The fix checks if the current view is not in a split when cycling views
> > and, if not, resets fullscreen dimensions, then bypasses resetting
> > these dimensions when resizing.
> 
> I tried to play with an alternative (manually resizing the parent when
> the child view goes fullscreen) but althought it was simpler, it had its
> own share of issues and bringed back a glitch with the double width
> characters.  yours works perfectly! :)

I tried a few different things but had the same problem: either cycling
between fullscreen views or the wide character splitscreen would break.

> However, I don't particularly like how we're adding an extra parameter
> to view_resize.  It would be nice to simplify view_resize somehow:
> having it doing "layout decisions" is a bit awkward IMHO, it would be
> nicer to have these things decided by some higher level code and
> view_resize acting as dumb as possible.
> 
> (to highlight the issue: when we're in fullscreen mode and the terminal
> is resized, view_resize "decides" that we want to exit fullscreen too.)
> 
> I'm not opposing to the patch going in, fixing view_resize may not be
> straightforward so in the meantime we could fix the issue at least.

I wasn't a fan either tbh, but the only other fix I had duplicated a bit
of view_resize(). It's less invasive though and appears to fix the
problem without breaking the double-width glitch either. Same logic as
the previous diff but moved up to view_fullscreen().

diff d8b5af438b16bcea5568b1d4bfc127567e35e2f6 /Users/mark/Library/Mobile Documents/com~apple~CloudDocs/src/got-current
blob - b8935d8f4d93a4f03c1d8727adb23a6137741acf
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -732,9 +732,17 @@ view_fullscreen(struct tog_view *view)
 	view->ncols = COLS;
 	view->lines = LINES;
 	view->cols = COLS;
-	err = view_resize(view);
-	if (err)
-		return err;
+	if (!view->child) {
+		err = view_resize(view);
+		if (err)
+			return err;
+	} else {
+		if (wresize(view->window, view->nlines, view->ncols) == ERR)
+			return got_error_from_errno("wresize");
+		if (replace_panel(view->panel, view->window) == ERR)
+			return got_error_from_errno("replace_panel");
+		wclear(view->window);
+	}

 	if (mvwin(view->window, view->begin_y, view->begin_x) == ERR)
 		return got_error_from_errno("mvwin");
@@ -956,6 +964,8 @@ view_input(struct tog_view **new, int *done, struct to
 			view->focussed = 0;
 			view->parent->focussed = 1;
 			view->parent->focus_child = 0;
+			if (!view_is_splitscreen(view))
+				err = view_fullscreen(view->parent);
 		}
 		break;
 	case 'q':


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