From: Mark Jamsek Subject: Re: fix cycle view in fullscreen regression To: Omar Polo Cc: gameoftrees@openbsd.org Date: Mon, 20 Jun 2022 11:50:53 +1000 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': -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68