On Thu, Aug 08, 2024 at 05:21:36PM +1000, Mark Jamsek wrote:
> Mark Jamsek <mark@jamsek.com> wrote:
> > The proposed change enables "tagging" the commits with the 't' keymap
> > instead. So the user can scroll to the first commit, enter 't', and
> > then scroll to the next commit and enter 't' to open a diff view of the
> > changes between the tagged commits. Entering 't' on an already tagged
> > commit untags it. Entering 'return' on any commit untags tagged commits.
> > 
> > I've attached two variants of this feature because I can't decide which
> > I prefer.
> > 
> > topmost diff:
> >   When the second commit is tagged, the diff view opens and the tagged
> >   commits automatically become untagged.
> > 
> > bottom diff:
> >   When the second commit is tagged, the diff view opens but both tags
> >   remain active. J/K keymaps automatically untag tagged commits.
> 
> The more I think about it, I prefer the second approach of keeping tags
> active--but I'm not married to it so am happy to go with the consensus.
> 
> I've attached an updated diff that marks tagged commits with a '>' in
> one of the columns padding the author field (similar to how we mark the
> base commit when in a work tree) instead of highlighting the entire row.
> I think it's less confusing than having multiple rows highlighted albeit
> much more subtle.
> 
> While I prefer this style, I'm happy to keep the row highlight if that's
> otherwise favoured.

I like this feature. I am wondering about one UI tweak:

Pressing 't' a second time to see the diff feels awkward because
I would usually type Enter to see a diff.

Have you considered letting the Enter key open the diff view instead of
the second t, such that any commit in the log view can be diffed against
the _most recently_ tagged commit (while restricting the amount of
commits the user can tag to 1, for now).

I suspect it would be better if 't' only did the tag/untag action.
We might eventually want to use tagging of commits for other purposes.
Keeping the second 't' key free of the additional action of opening
the diff view might give us more flexibility in the future.

Later, we could then let users tag as many commits as they want if that
becomes useful for some reason, and perhaps even tag commits based on a
search expression if that becomes useful for some reason. (One feature
idea: write out the individual diffs of all tagged commits to a patch file.)

We could offer some key binding which clears all existing tags to make
the Enter key act as usual again.