"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:41:39 +1000

Download raw body.

Thread
Mark Jamsek <mark@jamsek.com> wrote:
> Mark Jamsek <mark@jamsek.com> wrote:
> > 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.
> 
> As discussed with stsp on irc, use keymap 'm' (mnemonic "mark") instead
> of 't' and overloading "tag".

Updated diff actually changes the keymap from 't' to 'm'.


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

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

diff 11f34534de34dc3b24c47b54c86b1fd8aaf8699d e8d850c9484aacea71fe59c81e4cc9769333413d
commit - 11f34534de34dc3b24c47b54c86b1fd8aaf8699d
commit + e8d850c9484aacea71fe59c81e4cc9769333413d
blob - df994b229891a4a8a5d0f863fe8b5e815bc640f1
blob + 0f8f83ded2fa834467a0c0bef5de29b3578ddcaa
--- tog/tog.1
+++ tog/tog.1
@@ -193,6 +193,19 @@ but defaults to the oldest commit.
 Open a
 .Cm diff
 view showing file changes made in the currently selected commit.
+If a commit is marked with the
+.Cm m
+key map, open a diff view showing file changes made between the marked commit
+and the currently selected commit.
+.It Cm m
+Mark or unmark the selected commit.
+When a commit is marked,
+pressing the
+.Cm enter
+key on another selected commit opens a
+.Cm diff
+view showing the changes between the marked commit and the
+currently selected commit.
 .It Cm T
 Open a
 .Cm tree
blob - 674de2c51543e1af563af32b17a4d609805167ec
blob + f9ce0c8c3d60e68ea2f44223560be407416f477a
--- 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 *marked_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_("m", "Mark or unmark 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_mark_clear(struct tog_log_view_state *s)
+{
+	s->marked_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_mark_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->marked_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->marked_entry != NULL &&
+	    ls->marked_entry != ls->selected_entry)
+		parent_id = log_view->state.log.marked_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_mark_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_mark_commit(struct tog_log_view_state *s)
+{
+	if (s->selected_entry == s->marked_entry)
+		s->marked_entry = NULL;
+	else
+		s->marked_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 'm':
+		log_mark_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_mark_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