"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: fix cycle view in fullscreen regression
To:
Mark Jamsek <mark@jamsek.com>
Cc:
gameoftrees@openbsd.org
Date:
Sun, 19 Jun 2022 18:20:09 +0200

Download raw body.

Thread
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! :)

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.