On 22-09-14 05:30PM, Stefan Sperling wrote:
> 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.

Thanks! I was admittedly worried how you'd feel about this after talking
to you about it in Germany :)

> 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.

That's a good idea! I like the percentage indicator, I think I'll go
with that and see how it feels. Or maybe even both, a percentage and
a comment. I'll have a play and shoot through an update.

> 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.

I think so too. Thanks for the feedback; I'll build on them tomorrow.

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