"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:
Stefan Sperling <stsp@stsp.name>, gameoftrees@openbsd.org
Date:
Thu, 08 Aug 2024 20:00:47 +1000

Download raw body.

Thread
Mark Jamsek <mark@jamsek.com> wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
> > 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 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).
> 
> No, I hadn't considered using 'Enter' instead of 't' but I like it!
> Let's give it a go :)

The below diff is the same feature but using stsp's suggestion to reuse
the 'return' key instead of 't' to open the diff view showing changes
between the selected commit and the most recently tagged commit.

I must admit, I like this idea better than the previous version;
thanks, Stefan :)

I'm not sure if I've repeated myself too much in tog(1). OTOH, I like
documentation being self-contained in the sense that reading the docs
for the 't' keymap tells me all I need to know, but I'm happy to pare
it back.


diffstat 11f34534de34dc3b24c47b54c86b1fd8aaf8699d 55218e51a9b82fca4649f7fa58e67689c40cce6a
 M  tog/tog.1  |  13+   0-
 M  tog/tog.c  |  85+  21-

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

diff 11f34534de34dc3b24c47b54c86b1fd8aaf8699d 55218e51a9b82fca4649f7fa58e67689c40cce6a
commit - 11f34534de34dc3b24c47b54c86b1fd8aaf8699d
commit + 55218e51a9b82fca4649f7fa58e67689c40cce6a
blob - df994b229891a4a8a5d0f863fe8b5e815bc640f1
blob + 978d6156e4395cc1aaafb6a3d52b70ea2746f6df
--- tog/tog.1
+++ tog/tog.1
@@ -193,10 +193,23 @@ but defaults to the oldest commit.
 Open a
 .Cm diff
 view showing file changes made in the currently selected commit.
+If a commit is tagged with the
+.Cm t
+key map, open a diff view showing file changes made between the tagged commit
+and the currently selected commit.
 .It Cm T
 Open a
 .Cm tree
 view showing the tree for the currently selected commit.
+.It Cm t
+Tag or untag the selected commit.
+When a commit is tagged,
+pressing the
+.Cm enter
+key on another selected commit opens a
+.Cm diff
+view showing the changes between the tagged commit 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 + 3aaf848f7ddd853b1436cfb076494d5ecc75036a
--- tog/tog.c
+++ tog/tog.c
@@ -390,6 +390,7 @@ struct tog_log_view_state {
 	struct commit_queue_entry *first_displayed_entry;
 	struct commit_queue_entry *last_displayed_entry;
 	struct commit_queue_entry *selected_entry;
+	struct commit_queue_entry *tagged_entry;
 	struct commit_queue real_commits;
 	int selected;
 	char *in_repo_path;
@@ -571,6 +572,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 or untag the selected entry for diffing with the next" \
+	    " selected 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 +1699,12 @@ done:
 	return err;
 }
 
+static void
+log_tag_clear(struct tog_log_view_state *s)
+{
+	s->tagged_entry = 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 +1833,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);
+
+			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 +2442,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)
 {
@@ -2515,17 +2552,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 (s->tagged_entry == entry && 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 +3186,28 @@ 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;
+	struct tog_log_view_state *ls = NULL;
 
 	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)
+		ls = &log_view->state.log;
+
+	if (ls != NULL && ls->tagged_entry != NULL &&
+	    ls->tagged_entry != ls->selected_entry)
+		parent_id = log_view->state.log.tagged_entry->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);
+
 	err = stop_log_thread(s);
 
 	errcode = pthread_cond_destroy(&s->thread_args.need_commits);
@@ -4148,6 +4198,15 @@ horizontal_scroll_input(struct tog_view *view, int ch)
 	}
 }
 
+static void
+log_tag_commit(struct tog_log_view_state *s)
+{
+	if (s->selected_entry == s->tagged_entry)
+		s->tagged_entry = NULL;
+	else
+		s->tagged_entry = s->selected_entry;
+}
+
 static const struct got_error *
 input_log_view(struct tog_view **new_view, struct tog_view *view, int ch)
 {
@@ -4348,6 +4407,9 @@ input_log_view(struct tog_view **new_view, struct tog_
 		s->search_entry = NULL;
 		view->offset = 0;
 		break;
+	case 't':
+		log_tag_commit(s);
+		break;
 	case 'R':
 		view->count = 0;
 		err = view_request_new(new_view, view, TOG_VIEW_REF);
@@ -6000,6 +6062,8 @@ input_diff_view(struct tog_view **new_view, struct tog
 			if (old_selected_entry == ls->selected_entry)
 				break;
 
+			log_tag_clear(ls);
+
 			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