"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: add tog log 't' keymap to diff arbitrary commits
To:
Mark Jamsek <mark@jamsek.com>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 08 Aug 2024 17:21:36 +1000

Download raw body.

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


diffstat 11f34534de34dc3b24c47b54c86b1fd8aaf8699d 0c5cbba2652be816a3a406e47905a28b2b7dbbdc
 M  tog/tog.1  |   5+   0-
 M  tog/tog.c  |  96+  21-

2 files changed, 101 insertions(+), 21 deletions(-)

diff 11f34534de34dc3b24c47b54c86b1fd8aaf8699d 0c5cbba2652be816a3a406e47905a28b2b7dbbdc
commit - 11f34534de34dc3b24c47b54c86b1fd8aaf8699d
commit + 0c5cbba2652be816a3a406e47905a28b2b7dbbdc
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 + d986259fab76182915394ed1460203008e498d2f
--- 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;
@@ -2424,6 +2446,25 @@ format_author(wchar_t **wauthor, int *author_width, ch
 }
 
 static const struct got_error *
+draw_commit_marker(struct tog_view *view, char c)
+{
+	struct tog_color *tc;
+
+	if (view->type != TOG_VIEW_LOG)
+		return got_error_msg(GOT_ERR_NOT_IMPL, "view not supported");
+
+	tc = get_color(&view->state.log.colors, TOG_COLOR_COMMIT);
+	if (tc != NULL)
+		wattr_on(view->window, COLOR_PAIR(tc->colorpair), NULL);
+	if (waddch(view->window, c) == ERR)
+		return got_error_msg(GOT_ERR_IO, "waddch");
+	if (tc != NULL)
+		wattr_off(view->window, COLOR_PAIR(tc->colorpair), NULL);
+
+	return NULL;
+}
+
+static const struct got_error *
 draw_commit(struct tog_view *view, struct commit_queue_entry *entry,
     const size_t date_display_cols, int author_display_cols)
 {
@@ -2444,6 +2485,7 @@ draw_commit(struct tog_view *view, struct commit_queue
 	time_t committer_time;
 	struct tog_color *tc;
 	struct got_reflist_head *refs;
+	int tagged = s->tag.lhs == entry || s->tag.rhs == entry;
 
 	if (tog_base_commit.id != NULL && tog_base_commit.idx == -1 &&
 	    got_object_id_cmp(id, tog_base_commit.id) == 0)
@@ -2515,17 +2557,16 @@ draw_commit(struct tog_view *view, struct commit_queue
 	waddwstr(view->window, wauthor);
 	col += author_width;
 	while (col < avail && author_width < author_display_cols + 2) {
-		if (tog_base_commit.marker != GOT_WORKTREE_STATE_UNKNOWN &&
-		    author_width == marker_column &&
+		if (tagged && author_width == marker_column) {
+			err = draw_commit_marker(view, '>');
+			if (err != NULL)
+				goto done;
+		} else if (tog_base_commit.marker != GOT_WORKTREE_STATE_UNKNOWN
+		    && author_width == marker_column &&
 		    entry->idx == tog_base_commit.idx && !s->limit_view) {
-			tc = get_color(&s->colors, TOG_COLOR_COMMIT);
-			if (tc)
-				wattr_on(view->window,
-				    COLOR_PAIR(tc->colorpair), NULL);
-			waddch(view->window, tog_base_commit.marker);
-			if (tc)
-				wattr_off(view->window,
-				    COLOR_PAIR(tc->colorpair), NULL);
+			err = draw_commit_marker(view, tog_base_commit.marker);
+			if (err != NULL)
+				goto done;
 		} else
 			waddch(view->window, ' ');
 		col++;
@@ -3150,16 +3191,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 +3571,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 +4198,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 +4329,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 +4417,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 +6073,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