From: Mark Jamsek Subject: Re: add tog log 't' keymap to diff arbitrary commits To: Mark Jamsek Cc: Stefan Sperling , gameoftrees@openbsd.org Date: Thu, 08 Aug 2024 20:41:39 +1000 Mark Jamsek wrote: > Mark Jamsek wrote: > > Mark Jamsek wrote: > > > Stefan Sperling wrote: > > > > On Thu, Aug 08, 2024 at 05:21:36PM +1000, Mark Jamsek wrote: > > > > > Mark Jamsek 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 GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68