From: Mark Jamsek Subject: Re: fix cycle view in fullscreen regression To: Omar Polo Cc: gameoftrees@openbsd.org Date: Mon, 20 Jun 2022 14:09:36 +1000 On 22-06-20 11:50am, Mark Jamsek wrote: > On 22-06-19 06:20pm, Omar Polo wrote: > > Mark Jamsek 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': > > Actually, I think this is better. And like the other problem you noticed, view_resize() now also honours fullscreen when resizing. 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 @@ -748,6 +748,13 @@ view_is_parent_view(struct tog_view *view) return view->parent == NULL; } +static int +view_is_splitscreen(struct tog_view *view) +{ + return view->begin_x > 0; +} + + static const struct got_error * view_resize(struct tog_view *view) { @@ -763,7 +770,7 @@ view_resize(struct tog_view *view) else ncols = view->ncols + (COLS - view->cols); - if (view->child) { + if (view->child && view_is_splitscreen(view->child)) { view->child->begin_x = view_split_begin_x(view->begin_x); if (view->child->begin_x == 0) { ncols = COLS; @@ -818,12 +825,6 @@ view_set_child(struct tog_view *view, struct tog_view return view_resize(view); } -static int -view_is_splitscreen(struct tog_view *view) -{ - return view->begin_x > 0; -} - static void tog_resizeterm(void) { @@ -956,6 +957,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 GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68