I often run tog log, find either the two hashes or base-commit-relative
indexes of two commits I want to diff, drop back to my shell and run:

  `tog diff hash1 hash2` or `tog diff :base:-n :base:-n'`

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.

It's best to try out both patches to see which you prefer. In my case,
I think I rarely--if ever--want to diff the first tagged commit against
a commit other than the one I first diff it against. But this might not
be how others tog, and the automatic untag approach may not even be the
most intuitive. Comparable UI features (e.g., checkboxes, radio buttons)
might usually remain selected until the user explicitly deselects them,
so from that perspective it might be more user friendly.

I'm happy with either approach so I'll leave it to the consensus but
either way am floating this for after release so no rush.


-----------------------------------------------
commit 9e1d8e22783331d38085516eb64ceea59c1792a9 (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Wed Aug  7 14:53:20 2024 UTC
 
 add tog log 't' keymap to tag arbitrary commits to diff
 
 Enable selecting arbitrary commits from the log view to open a diff view
 showing the changes between them.
 
 M  tog/tog.1  |   5+   0-
 M  tog/tog.c  |  68+  13-

2 files changed, 73 insertions(+), 13 deletions(-)

diff 11f34534de34dc3b24c47b54c86b1fd8aaf8699d 9e1d8e22783331d38085516eb64ceea59c1792a9
commit - 11f34534de34dc3b24c47b54c86b1fd8aaf8699d
commit + 9e1d8e22783331d38085516eb64ceea59c1792a9
blob - df994b229891a4a8a5d0f863fe8b5e815bc640f1
blob + 92f5c68acc711e3cb1f27a967d4a0e8ad165a2f8
--- tog/tog.1
+++ tog/tog.1
@@ -197,6 +197,11 @@ view showing file changes made in the currently select
 Open a
 .Cm tree
 view showing the tree for the currently selected commit.
+.It Cm t
+Tag or untag the selected commit.
+If another commit is already tagged, open a
+.Cm diff
+view showing the changes between it and the currently selected commit.
 .It Cm Backspace
 Show log entries for the parent directory of the currently selected path.
 However when an active search is in progress or when additional commits
blob - 674de2c51543e1af563af32b17a4d609805167ec
blob + ac2baf0d2d7c9f88a6c2d7c0fb77b0b74fd5e9be
--- tog/tog.c
+++ tog/tog.c
@@ -407,6 +407,10 @@ struct tog_log_view_state {
 	int limit_view;
 	regex_t limit_regex;
 	struct commit_queue limit_commits;
+	struct log_tag {
+		struct commit_queue_entry *lhs;
+		struct commit_queue_entry *rhs;
+	} tag;
 };
 
 #define TOG_COLOR_DIFF_MINUS		1
@@ -571,6 +575,8 @@ struct tog_help_view_state {
 	KEY_("R", "Open ref view of all repository references"), \
 	KEY_("T", "Display tree view of the repository from the selected" \
 	    " commit"), \
+	KEY_("t", "Tag/untag the selected entry (or diff with the currently " \
+	    "tagged commit)"), \
 	KEY_("@", "Toggle between displaying author and committer name"), \
 	KEY_("&", "Open prompt to enter term to limit commits displayed"), \
 	KEY_("C-g Backspace", "Cancel current search or log operation"), \
@@ -1696,6 +1702,13 @@ done:
 	return err;
 }
 
+static void
+log_tag_clear(struct log_tag *tag)
+{
+	tag->lhs = NULL;
+	tag->rhs = NULL;
+}
+
 static const struct got_error *
 view_input(struct tog_view **new, int *done, struct tog_view *view,
     struct tog_view_list_head *views, int fast_refresh)
@@ -1824,14 +1837,23 @@ view_input(struct tog_view **new, int *done, struct to
 		}
 		break;
 	case 'q':
-		if (view->parent && view->mode == TOG_VIEW_SPLIT_HRZN) {
-			if (view->parent->resize) {
-				/* might need more commits to fill fullscreen */
-				err = view->parent->resize(view->parent, 0);
-				if (err)
-					break;
+		if (view->parent != NULL) {
+			if (view->parent->type == TOG_VIEW_LOG)
+				log_tag_clear(&view->parent->state.log.tag);
+
+			if (view->mode == TOG_VIEW_SPLIT_HRZN) {
+				if (view->parent->resize) {
+					/*
+					 * Might need more commits
+					 * to fill fullscreen.
+					 */
+					err = view->parent->resize(
+					    view->parent, 0);
+					if (err)
+						break;
+				}
+				offset_selection_up(view->parent);
 			}
-			offset_selection_up(view->parent);
 		}
 		err = view->input(new, view, ch);
 		view->dying = 1;
@@ -2995,12 +3017,15 @@ draw_commits(struct tog_view *view)
 	s->last_displayed_entry = s->first_displayed_entry;
 	ncommits = 0;
 	while (entry) {
+		int tagged = s->tag.lhs == entry && s->tag.rhs == NULL;
+
 		if (ncommits >= limit - 1)
 			break;
-		if (ncommits == s->selected)
+
+		if (ncommits == s->selected || tagged)
 			wstandout(view->window);
 		err = draw_commit(view, entry, date_display_cols, author_cols);
-		if (ncommits == s->selected)
+		if (ncommits == s->selected || tagged)
 			wstandend(view->window);
 		if (err)
 			goto done;
@@ -3150,16 +3175,23 @@ open_diff_view_for_commit(struct tog_view **new_view, 
     struct tog_view *log_view, struct got_repository *repo)
 {
 	const struct got_error *err;
-	struct got_object_qid *parent_id;
+	struct got_object_qid *p;
+	struct got_object_id *parent_id;
 	struct tog_view *diff_view;
 
 	diff_view = view_open(0, 0, begin_y, begin_x, TOG_VIEW_DIFF);
 	if (diff_view == NULL)
 		return got_error_from_errno("view_open");
 
-	parent_id = STAILQ_FIRST(got_object_commit_get_parent_ids(commit));
-	err = open_diff_view(diff_view, parent_id ? &parent_id->id : NULL,
-	    commit_id, NULL, NULL, 3, 0, 0, log_view, repo);
+	if (log_view != NULL && log_view->state.log.tag.lhs != NULL)
+		parent_id = log_view->state.log.tag.lhs->id;
+	else if ((p = STAILQ_FIRST(got_object_commit_get_parent_ids(commit))))
+		parent_id = &p->id;
+	else
+		parent_id = NULL;
+
+	err = open_diff_view(diff_view, parent_id, commit_id,
+	    NULL, NULL, 3, 0, 0, log_view, repo);
 	if (err == NULL)
 		*new_view = diff_view;
 	return err;
@@ -3523,6 +3555,8 @@ close_log_view(struct tog_view *view)
 	struct tog_log_view_state *s = &view->state.log;
 	int errcode;
 
+	log_tag_clear(&s->tag);
+
 	err = stop_log_thread(s);
 
 	errcode = pthread_cond_destroy(&s->thread_args.need_commits);
@@ -4148,6 +4182,22 @@ horizontal_scroll_input(struct tog_view *view, int ch)
 	}
 }
 
+static int
+log_tag_commit(struct tog_log_view_state *s)
+{
+	if (s->tag.rhs != NULL)
+		log_tag_clear(&s->tag);
+
+	if (s->tag.lhs == NULL)
+		s->tag.lhs = s->selected_entry;
+	else if (s->selected_entry == s->tag.lhs)
+		s->tag.lhs = NULL;	/* untag selected entry */
+	else
+		s->tag.rhs = s->selected_entry;
+
+	return s->tag.lhs != NULL && s->tag.rhs != NULL;
+}
+
 static const struct got_error *
 input_log_view(struct tog_view **new_view, struct tog_view *view, int ch)
 {
@@ -4261,6 +4311,7 @@ input_log_view(struct tog_view **new_view, struct tog_
 	case KEY_ENTER:
 	case '\r':
 		view->count = 0;
+		log_tag_clear(&s->tag);
 		if (s->selected_entry == NULL)
 			break;
 		err = view_request_new(new_view, view, TOG_VIEW_DIFF);
@@ -4348,6 +4399,10 @@ input_log_view(struct tog_view **new_view, struct tog_
 		s->search_entry = NULL;
 		view->offset = 0;
 		break;
+	case 't':
+		if (log_tag_commit(s))
+			err = view_request_new(new_view, view, TOG_VIEW_DIFF);
+		break;
 	case 'R':
 		view->count = 0;
 		err = view_request_new(new_view, view, TOG_VIEW_REF);


**second diff (keep tags active)**

diff 11f34534de34dc3b24c47b54c86b1fd8aaf8699d bc893721f9fdca8f4556129f3a0dac8cc90c39ef
commit - 11f34534de34dc3b24c47b54c86b1fd8aaf8699d
commit + bc893721f9fdca8f4556129f3a0dac8cc90c39ef
blob - df994b229891a4a8a5d0f863fe8b5e815bc640f1
blob + 92f5c68acc711e3cb1f27a967d4a0e8ad165a2f8
--- tog/tog.1
+++ tog/tog.1
@@ -197,6 +197,11 @@ view showing file changes made in the currently select
 Open a
 .Cm tree
 view showing the tree for the currently selected commit.
+.It Cm t
+Tag or untag the selected commit.
+If another commit is already tagged, open a
+.Cm diff
+view showing the changes between it and the currently selected commit.
 .It Cm Backspace
 Show log entries for the parent directory of the currently selected path.
 However when an active search is in progress or when additional commits
blob - 674de2c51543e1af563af32b17a4d609805167ec
blob + 18903538abee333ba5e4db75cbad526198808c25
--- tog/tog.c
+++ tog/tog.c
@@ -407,6 +407,10 @@ struct tog_log_view_state {
 	int limit_view;
 	regex_t limit_regex;
 	struct commit_queue limit_commits;
+	struct log_tag {
+		struct commit_queue_entry *lhs;
+		struct commit_queue_entry *rhs;
+	} tag;
 };
 
 #define TOG_COLOR_DIFF_MINUS		1
@@ -571,6 +575,8 @@ struct tog_help_view_state {
 	KEY_("R", "Open ref view of all repository references"), \
 	KEY_("T", "Display tree view of the repository from the selected" \
 	    " commit"), \
+	KEY_("t", "Tag/untag the selected entry (or diff with the currently " \
+	    "tagged commit)"), \
 	KEY_("@", "Toggle between displaying author and committer name"), \
 	KEY_("&", "Open prompt to enter term to limit commits displayed"), \
 	KEY_("C-g Backspace", "Cancel current search or log operation"), \
@@ -1696,6 +1702,13 @@ done:
 	return err;
 }
 
+static void
+log_tag_clear(struct log_tag *tag)
+{
+	tag->lhs = NULL;
+	tag->rhs = NULL;
+}
+
 static const struct got_error *
 view_input(struct tog_view **new, int *done, struct tog_view *view,
     struct tog_view_list_head *views, int fast_refresh)
@@ -1824,14 +1837,23 @@ view_input(struct tog_view **new, int *done, struct to
 		}
 		break;
 	case 'q':
-		if (view->parent && view->mode == TOG_VIEW_SPLIT_HRZN) {
-			if (view->parent->resize) {
-				/* might need more commits to fill fullscreen */
-				err = view->parent->resize(view->parent, 0);
-				if (err)
-					break;
+		if (view->parent != NULL) {
+			if (view->parent->type == TOG_VIEW_LOG)
+				log_tag_clear(&view->parent->state.log.tag);
+
+			if (view->mode == TOG_VIEW_SPLIT_HRZN) {
+				if (view->parent->resize) {
+					/*
+					 * Might need more commits
+					 * to fill fullscreen.
+					 */
+					err = view->parent->resize(
+					    view->parent, 0);
+					if (err)
+						break;
+				}
+				offset_selection_up(view->parent);
 			}
-			offset_selection_up(view->parent);
 		}
 		err = view->input(new, view, ch);
 		view->dying = 1;
@@ -2995,12 +3017,15 @@ draw_commits(struct tog_view *view)
 	s->last_displayed_entry = s->first_displayed_entry;
 	ncommits = 0;
 	while (entry) {
+		int tagged = s->tag.lhs == entry || s->tag.rhs == entry;
+
 		if (ncommits >= limit - 1)
 			break;
-		if (ncommits == s->selected)
+
+		if (ncommits == s->selected || tagged)
 			wstandout(view->window);
 		err = draw_commit(view, entry, date_display_cols, author_cols);
-		if (ncommits == s->selected)
+		if (ncommits == s->selected || tagged)
 			wstandend(view->window);
 		if (err)
 			goto done;
@@ -3150,16 +3175,23 @@ open_diff_view_for_commit(struct tog_view **new_view, 
     struct tog_view *log_view, struct got_repository *repo)
 {
 	const struct got_error *err;
-	struct got_object_qid *parent_id;
+	struct got_object_qid *p;
+	struct got_object_id *parent_id;
 	struct tog_view *diff_view;
 
 	diff_view = view_open(0, 0, begin_y, begin_x, TOG_VIEW_DIFF);
 	if (diff_view == NULL)
 		return got_error_from_errno("view_open");
 
-	parent_id = STAILQ_FIRST(got_object_commit_get_parent_ids(commit));
-	err = open_diff_view(diff_view, parent_id ? &parent_id->id : NULL,
-	    commit_id, NULL, NULL, 3, 0, 0, log_view, repo);
+	if (log_view != NULL && log_view->state.log.tag.lhs != NULL)
+		parent_id = log_view->state.log.tag.lhs->id;
+	else if ((p = STAILQ_FIRST(got_object_commit_get_parent_ids(commit))))
+		parent_id = &p->id;
+	else
+		parent_id = NULL;
+
+	err = open_diff_view(diff_view, parent_id, commit_id,
+	    NULL, NULL, 3, 0, 0, log_view, repo);
 	if (err == NULL)
 		*new_view = diff_view;
 	return err;
@@ -3523,6 +3555,8 @@ close_log_view(struct tog_view *view)
 	struct tog_log_view_state *s = &view->state.log;
 	int errcode;
 
+	log_tag_clear(&s->tag);
+
 	err = stop_log_thread(s);
 
 	errcode = pthread_cond_destroy(&s->thread_args.need_commits);
@@ -4148,6 +4182,24 @@ horizontal_scroll_input(struct tog_view *view, int ch)
 	}
 }
 
+static int
+log_tag_commit(struct tog_log_view_state *s)
+{
+	if (s->selected_entry == s->tag.lhs)
+		s->tag.lhs = NULL;
+	else if (s->selected_entry == s->tag.rhs)
+		s->tag.rhs = NULL;
+	else if (s->tag.lhs == NULL && s->tag.rhs != NULL) {
+		s->tag.lhs = s->tag.rhs;
+		s->tag.rhs = s->selected_entry;
+	} else if (s->tag.lhs == NULL) {
+		s->tag.lhs = s->selected_entry;
+	} else
+		s->tag.rhs = s->selected_entry;
+
+	return s->tag.lhs != NULL && s->tag.rhs != NULL;
+}
+
 static const struct got_error *
 input_log_view(struct tog_view **new_view, struct tog_view *view, int ch)
 {
@@ -4261,6 +4313,7 @@ input_log_view(struct tog_view **new_view, struct tog_
 	case KEY_ENTER:
 	case '\r':
 		view->count = 0;
+		log_tag_clear(&s->tag);
 		if (s->selected_entry == NULL)
 			break;
 		err = view_request_new(new_view, view, TOG_VIEW_DIFF);
@@ -4348,6 +4401,10 @@ input_log_view(struct tog_view **new_view, struct tog_
 		s->search_entry = NULL;
 		view->offset = 0;
 		break;
+	case 't':
+		if (log_tag_commit(s))
+			err = view_request_new(new_view, view, TOG_VIEW_DIFF);
+		break;
 	case 'R':
 		view->count = 0;
 		err = view_request_new(new_view, view, TOG_VIEW_REF);
@@ -6000,6 +6057,8 @@ input_diff_view(struct tog_view **new_view, struct tog
 			if (old_selected_entry == ls->selected_entry)
 				break;
 
+			log_tag_clear(&ls->tag);
+
 			err = set_selected_commit(s, ls->selected_entry);
 			if (err)
 				break;


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