On Thu, Sep 15, 2022 at 01:08:53AM +1000, Mark Jamsek wrote:
> I started this the last Saturday of the hackathon after floating the
> idea to tb@ who also thought it could be useful (similar to less(1) 'h')
> and just remembered today as I was cleaning out old branches!  We have
> a lot of key maps, and most TUI apps I use provide some runtime
> reference for this (e.g., less(1), mutt, hackernews), so I thought it
> would add value to tog too.
> 
> I've just finished rewriting the implementation because the initial code
> was too naive and somewhat inconsistent with the other views;
> we initially used a curses pad, which did 80% of what we expect, but as
> I read it over, I realised we'd have to write a lot more code to add
> functionality for which we already have code (e.g., search, splits,
> resize) that could instead be reused if I just changed the implementation.
> Plus, it now looks more consistent with the other tog views rather than
> the initial popup dialog box. And I thought being able to have the help
> view open in a split will help new users get familiar with tog.
> 
> I spoke to stsp and tobhe too. tobhe also thought it would be handy, and
> stsp was concerned wrt keeping it in sync, so I've tried to make it
> simple to update keymaps with some x-macros. stsp also suggested it
> should be view-specific; so if you hit '?' in log view, just display log
> key maps, diff keys in diff view, etc.  We've done this, but also made it
> so that hitting '?' again in the help view will toggle display of all
> key maps. But I'm happy to drop this if you think it's too much, so
> I haven't documented anything in the man page yet because I wanted to
> get some feedback first.
> 
> The diff is probably larger than you'd expect, but a fair bit of this is
> due to refactoring the search code; we've deduped the
> search_next_{blame,diff}_view, and reused this for the help search. So
> this will ultimately be committed separately if ok. I've left it as is
> because we reuse the refactored code for the help view, but if you
> prefer, I can split the diff up now for review.
> 
> One pertinent question I have is:
> 
>   I'd eventually like to add '?' to start backwards search like vi(1);
>   if in agreement, should we not use '?' now to toggle help and instead
>   just stick with 'H' and 'F1'?

I like this a lot, it looks great.

I would suggest to provide a visual indication at the bottom of the
help view, to indicate whether scrolling downwards is possible.
In an 80x24 terminal only the global options are visible initially, and
scrolling is required to make any view-specific keys appear. It should
somehow be made very obvious that the help window can be scrolled.

E.g. the last line of the help view could be reserved for a text string such
as "scroll down for more", or a percentage indicator, or similar. And once
the bottom of the view been reached such text could be replaced with the line
"See tog(1) for full documentation" (or perhaps "See the tog(1) manual page
for full documentation" would be clearer).
That's just one idea, perhaps there are better ways to do this.

Regarding consistency, I think having this feature built-in is worth the
extra effort of ensuring that the help view and man page stay in sync.
We'll just have to keep this in mind going forward.