On 22-06-26 04:44pm, Stefan Sperling wrote:
> On Sun, Jun 26, 2022 at 11:08:55PM +1000, Mark Jamsek wrote:
> > The below diff adds support for horizontal splits in tog.
> > 
> > We use a couple new envvars to configure splits:
> > 
> > TOG_VIEW_SPLIT_MODE=(h|v)
> >   v: force vertical split
> >   h: force horizontal split
> >   default/unset: if COLUMNS >= 120 we open a vsplit, else hsplit
> > TOG_VIEW_SPLIT_HEIGHT=n such that 1 <= n <= LINES - 3
> >   default/unset: 60% of LINES
> 
> Does split height really need to be configurable? Vertical split
> is not configurable and so far nobody has complained about that.

I think it makes more sense to enable configurable hsplits than vsplits.
But I don't have any strong convictions either so I'm more than happy to
defer to you on this.

> Assuming we really want config knobs for this:
> LINES will change when the terminal is resized or made fullscreen,
> and this can happen while tog is running. Would it make sense to
> allow setting hsplit size in terms of percentage rather than as
> an absolute value 1 <= n <= LINES - 3? Another way would be to
> make it work like mutt's pager_index_lines variable, which specifies
> the minimum number of lines which should be left for the parent window,
> and the rest will be used for the new horizontal child window.
> In mutt, pager_index_lines=6 has been working fine for me.
> Setting pager_index_lines=0 disables horizontal split in mutt.

I really like the idea of making TOG_VIEW_SPLIT_HEIGHT relative--I think
it's a great idea! Certainly much saner and also more comprehensible;
It's more common to think, "I want the bottom split about half my screen
height."

> > So if the user does nothing the only change in existing behaviour is
> > that we'll open a horizontal split when COLUMNS < 120 (i.e., when we
> > would normally open child views in fullscreen mode now).
> > 
> > Horizontal splits are supported in log -> diff, tree -> blame, ref ->
> > log, and blame -> commit. So all the main views will open their primary
> > child view in a horizontal split, but we haven't yet got some of the
> > secondary child views (e.g., 't' or 'r' key maps from log view).
> 
> Ideally, horizontal split would happen wherever vertical split happens now.
> Otherwise the new behaviour is inconsistent and will be confusing people.

Okay. How about something like this:

  - TOG_VIEW_SPLIT_MODE is not set (i.e., user does nothing): keep the
    current behaviour (i.e., views open in vsplits when terminal is wide
    enough).
  - TOG_VIEW_SPLIT_MODE is set to 'h': open child views in hsplits
    (vsplits are never opened).
  - TOG_VIEW_SPLIT_MODE is set to 'a' (auto): open child views in
    vsplits if the terminal is wide enough, otherwise we open them in
    hsplits. I would use this mode myself as I like vsplits when I run
    wide terminals, which is most of the time. But when I open vertical
    panes in tmux and they're no longer wide enough for a vsplit, I'd
    like a hsplit to open instead. This way I don't need to keep
    resetting TOG_VIEW_SPLIT_MODE from v to h when I'm in different
    sized tmux panes. I can just set it to auto and forget.

So this way if the user does nothing, the behaviour stays the same. If
they explicitly set hsplits then only hsplits are opened instead of
vsplits so behaviour remains consistent. And only if they explicitly set
auto mode then we get dynamic sizing.

> > There are a couple bugs that need squishing:
> >   - resizing with an open hsplit sometimes kills the border and there
> >     seems to be an OB1 that makes the top split misrender a line
> >   - the double-width bug in the log view has returned where we overwrite
> >     the vertical border by one column; strangely enough if you open the
> >     commit in ja.git in a vsplit then press F, tab, tab, F, it renders
> >     correctly
> 
> I've tried to spot a possible reason for this, but failed so far.
>  
> > The diff's already quite large though (sorry!) and I think it's ready
> > for more eyes if not use to see what other bugs I've introduced and if
> > the interface is friendly enough. One thing in particular that I'm not
> > sure about is, if the user sets an invalid TOG_VIEW_SPLIT_HEIGHT, we
> > error out. But perhaps it'd be better to fallback to the default value
> > instead. This is trivial though and can be refined later.
> 
> Falling back on the default behaviour seems better to me than
> erroring out.

Yes, I agree. I'll make some changes and resend tonight. Thanks, Stefan!

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