Download raw body.
show base commit markers in tog log view
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 Fri, Jul 21, 2023 at 10:54:58PM +1000, Mark Jamsek wrote:
> > > > > This is a simple change along the same lines of showing reference labels
> > > > > next to tog log messages insofar as it improves situational awareness,
> > > > > which can be handy at times.
> > > > >
> > > > > For example, in the case where you want to fold or drop some commits.
> > > > > After 'got up -c:head:-N', I often like to confirm I've updated to the
> > > > > correct commit before invoking histedit. With this, you can open tog and
> > > > > at a glance confirm that your base commit is indeed the correct one for
> > > > > the desired histedit operation. Similarly, when your base commit is
> > > > > something other than the tip, it's nice to know where the base commit is
> > > > > in relation to the rest when browsing the log view. This can interop
> > > > > well with the new :base keyword feature.
> > > > >
> > > > > The change will be immediately obvious upon running tog with the patch
> > > > > inside a work tree, but to summarise: we show the "[base commit]" label
> > > > > in the log view headline if the currently selected commit is the work
> > > > > tree's base commit; and, irrespective of which commit is selected, we
> > > > > prepend the base commit's summary log message with the '@' symbol. There
> > > > > is no change to the log view when tog is invoked outside a work tree.
> > > >
> > > > I agree that this is useful.
> > > >
> > > > Some things I would suggest to change:
> > > >
> > > > Could we use * instead of @, for consistency with got branch -l?
> > > > And like got branch -l, could tog show ~ if the work tree is out of
> > > > date with respect to the branch tip?
> > > >
> > > > A problem that applies to both this patch and got branch -l:
> > > > A mixed-commit work tree will be displayed as up-to-date since the current
> > > > logic only considers the work tree's global base commit.
> > > > Could we iterate over the file index and check whether any file is based
> > > > on a different commit, and if so show the out-of-date symbol ~?
> > > >
> > > > It is a matter of taste but the [base commit] marker seems redundant to me
> > > > and makes the display seem too crowded. I would omit this marker for now.
> > >
> > > Yes, sure! The reason I wasn't concerned with distinguishing between
> > > out of date and up-to-date trees is it's easy to see in the log view in
> > > the general case because it won't be the topmost commit; but that's a
> > > good idea about mixed commits--there's no other way to see this without
> > > different symbols.
> > >
> > > The below diffs implement your suggestions: the first adds a new lib
> > > routine to get the tree state, and shows the * or ~ marker in tog (the
> > > headline label has also been removed); the second gives the same
> > > treatment to 'got branch -l' including test coverage.
> > >
> > > I've also added a couple got_worktree_close() calls to cmd_ref() and
> > > cmd_tree() that we were missing; if we hit an error early in these
> > > functions we were leaking the worktree.
> >
> > I'm not sure if the tweak to 'got branch -l' output necessitates a
> > change to the docs too? Just in case, here's a minor addition to
> > mention the mixed commit case.
>
> The previous version of the tog base commit marker diff was inefficient;
> we would compare the id of each commit being drawn on every page being
> rendered even after we'd already drawn the base commit or were rendering
> a page that the base commit was not on.
>
> The below diff improves this by finding the base commit index when
> queuing commits so once we have identified the base commit, we don't
> do any more id comparisons. It's a marginal optimisation that would be
> of more benefit on larger monitors and terminals with many rows, but is
> still worth it imo.
>
> I've also folded the got(1) doc change into the related diff with the
> 'got branch -l' tweak.
I left behind a superfluous tog_base_commit.idx != -1 check that has
now been removed:
-----------------------------------------------
commit d1ff26edd0234c2837a5850d3272ba0b88054af7
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Jul 22 14:04:44 2023 UTC
tog: show work tree base commit marker in log view
If tog is invoked in a work tree, prefix the base commit log message summary
line with a '*' if the work tree is up-to-date, and with a '~' if the base
commit is not up-to-date with respect to the branch tip or it contains mixed
commits. While here, plug a couple worktree leaks in cmd_ref() and cmd_tree().
M include/got_worktree.h | 12+ 0-
M lib/worktree.c | 47+ 0-
M regress/tog/log.sh | 90+ 1-
M tog/tog.c | 75+ 4-
4 files changed, 224 insertions(+), 5 deletions(-)
diff 9c986b77d3f71b37ff5b5c6b4ec58b495ad8f312 d1ff26edd0234c2837a5850d3272ba0b88054af7
commit - 9c986b77d3f71b37ff5b5c6b4ec58b495ad8f312
commit + d1ff26edd0234c2837a5850d3272ba0b88054af7
blob - 53e320eef8622a3384a31606d0cd2fe64d9ea60a
blob + 07cb22fd98f7b10c110a8a5d0c0627770c5b7e76
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -134,6 +134,18 @@ const struct got_error *got_worktree_set_base_commit_i
struct got_repository *, struct got_object_id *);
/*
+ * Get the state of the work tree. If the work tree's global base commit is
+ * the tip of the work tree's current branch, and each file in the index is
+ * based on this same commit, the char out parameter will be
+ * GOT_WORKTREE_UPTODATE, else it will be GOT_WORKTREE_OUTOFDATE.
+ */
+const struct got_error *got_worktree_get_state(char *,
+ struct got_repository *, struct got_worktree *);
+
+#define GOT_WORKTREE_UPTODATE '*'
+#define GOT_WORKTREE_OUTOFDATE '~'
+
+/*
* Obtain a parsed representation of this worktree's got.conf file.
* Return NULL if this configuration file could not be read.
*/
blob - 5400e1336138a373e1307ff721ae4d68d5633075
blob + 6b14bc178bf5fbc6065088656b0ecae4a8fd2aad
--- lib/worktree.c
+++ lib/worktree.c
@@ -3251,6 +3251,53 @@ struct check_merge_conflicts_arg {
return NULL;
}
+const struct got_error *
+got_worktree_get_state(char *state, struct got_repository *repo,
+ struct got_worktree *worktree)
+{
+ const struct got_error *err;
+ struct got_object_id *base_id, *head_id = NULL;
+ struct got_reference *head_ref;
+ struct got_fileindex *fileindex = NULL;
+ char *fileindex_path = NULL;
+
+ if (worktree == NULL)
+ return got_error(GOT_ERR_NOT_WORKTREE);
+
+ err = got_ref_open(&head_ref, repo,
+ got_worktree_get_head_ref_name(worktree), 0);
+ if (err)
+ return err;
+
+ err = got_ref_resolve(&head_id, repo, head_ref);
+ if (err)
+ goto done;
+
+ *state = GOT_WORKTREE_OUTOFDATE;
+ base_id = got_worktree_get_base_commit_id(worktree);
+
+ if (got_object_id_cmp(base_id, head_id) == 0) {
+ err = open_fileindex(&fileindex, &fileindex_path, worktree);
+ if (err)
+ goto done;
+
+ err = got_fileindex_for_each_entry_safe(fileindex,
+ check_mixed_commits, worktree);
+ if (err == NULL)
+ *state = GOT_WORKTREE_UPTODATE;
+ else if (err->code == GOT_ERR_MIXED_COMMITS)
+ err = NULL;
+ }
+
+done:
+ free(head_id);
+ free(fileindex_path);
+ got_ref_close(head_ref);
+ if (fileindex != NULL)
+ got_fileindex_free(fileindex);
+ return err;
+}
+
struct check_merge_conflicts_arg {
struct got_worktree *worktree;
struct got_fileindex *fileindex;
blob - 2a28c65c9bf9e227cba943ce259438976ac67c02
blob + d10c8e4e269c29522a88fa8b7f94b5658cacd469
--- regress/tog/log.sh
+++ regress/tog/log.sh
@@ -448,7 +448,7 @@ test_log_commit_keywords()
commit $(pop_id 5 $ids) [1/5]
$ymd $(pop_id 5 $short_ids) flan_hacker commit 4
$ymd $(pop_id 4 $short_ids) flan_hacker commit 3
- $ymd $(pop_id 3 $short_ids) flan_hacker commit 2
+ $ymd $(pop_id 3 $short_ids) flan_hacker ~commit 2
$ymd $(pop_id 2 $short_ids) flan_hacker commit 1
$ymd $(pop_id 1 $short_ids) flan_hacker adding the test tree
@@ -499,6 +499,94 @@ test_parseargs "$@"
test_done "$testroot" "$ret"
}
+test_log_show_base_commit()
+{
+ # make view wide enough to show full headline
+ test_init log_show_base_commit 80 3
+ local repo="$testroot/repo"
+ local id=$(git_show_head "$repo")
+
+ echo "alpha" >> "$repo/alpha"
+ git_commit "$repo" -m "base commit"
+
+ got checkout "$repo" "$testroot/wt" > /dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got checkout failed unexpectedly"
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # move into the work tree (test is run in a subshell)
+ cd "$testroot/wt"
+
+ local head_id=$(git_show_head "$repo")
+ local author_time=$(git_show_author_time "$repo")
+ local ymd=$(date -u -r "$author_time" +"%G-%m-%d")
+
+ # check up-to-date base commit marker prefixes base commit log message
+ cat <<-EOF >$TOG_TEST_SCRIPT
+ SCREENDUMP
+ EOF
+
+ cat <<-EOF >$testroot/view.expected
+ commit $head_id [1/2] master
+ $ymd flan_hacker *[master] base commit
+ $ymd flan_hacker adding the test tree
+ EOF
+
+ tog log
+ cmp -s "$testroot/view.expected" "$testroot/view"
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u "$testroot/view.expected" "$testroot/view"
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # check marker is not drawn when not in a work tree
+ cat <<-EOF >$testroot/view.expected
+ commit $head_id [1/2] master
+ $ymd flan_hacker [master] base commit
+ $ymd flan_hacker adding the test tree
+ EOF
+
+ tog log -r "$repo"
+ cmp -s "$testroot/view.expected" "$testroot/view"
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u "$testroot/view.expected" "$testroot/view"
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # check out-of-date marker is shown with a mixed-commit tree
+ echo "mixed" > alpha
+ got commit -m "new base mixed-commit" > /dev/null
+ head_id=$(git_show_head "$repo")
+
+ cat <<-EOF >$TOG_TEST_SCRIPT
+ SCREENDUMP
+ EOF
+
+ cat <<-EOF >$testroot/view.expected
+ commit $head_id [1/3] master
+ $ymd flan_hacker ~[master] new base mixed-commit
+ $ymd flan_hacker base commit
+ EOF
+
+ tog log
+ cmp -s "$testroot/view.expected" "$testroot/view"
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u "$testroot/view.expected" "$testroot/view"
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ test_done "$testroot" "$ret"
+}
+
test_parseargs "$@"
run_test test_log_hsplit_diff
run_test test_log_vsplit_diff
@@ -508,3 +596,4 @@ run_test test_log_commit_keywords
run_test test_log_hsplit_tree
run_test test_log_logmsg_widechar
run_test test_log_commit_keywords
+run_test test_log_show_base_commit
blob - 55201d879df128d62d2a76ec797059af85649fa8
blob + 8a1028bb4f2e258d908ee56b92451061bcca21f7
--- tog/tog.c
+++ tog/tog.c
@@ -151,6 +151,11 @@ static enum got_diff_algorithm tog_diff_algo = GOT_DIF
static struct got_reflist_head tog_refs = TAILQ_HEAD_INITIALIZER(tog_refs);
static struct got_reflist_object_id_map *tog_refs_idmap;
+static struct {
+ struct got_object_id *id;
+ int idx;
+ char marker;
+} tog_base_commit;
static enum got_diff_algorithm tog_diff_algo = GOT_DIFF_ALGORITHM_MYERS;
static const struct got_error *
@@ -2399,7 +2404,7 @@ draw_commit(struct tog_view *view, struct got_commit_o
static const struct got_error *
draw_commit(struct tog_view *view, struct got_commit_object *commit,
struct got_object_id *id, const size_t date_display_cols,
- int author_display_cols)
+ int author_display_cols, int entry_idx)
{
struct tog_log_view_state *s = &view->state.log;
const struct got_error *err = NULL;
@@ -2411,7 +2416,7 @@ draw_commit(struct tog_view *view, struct got_commit_o
int author_width, refstr_width, logmsg_width;
char *newline, *line = NULL;
int col, limit, scrollx, logmsg_x;
- const int avail = view->ncols;
+ const int avail = view->ncols, marker_column = author_display_cols + 1;
struct tm tm;
time_t committer_time;
struct tog_color *tc;
@@ -2476,7 +2481,12 @@ draw_commit(struct tog_view *view, struct got_commit_o
waddwstr(view->window, wauthor);
col += author_width;
while (col < avail && author_width < author_display_cols + 2) {
- waddch(view->window, ' ');
+ if (tog_base_commit.id != NULL &&
+ author_width == marker_column &&
+ entry_idx == tog_base_commit.idx)
+ waddch(view->window, tog_base_commit.marker);
+ else
+ waddch(view->window, ' ');
col++;
author_width++;
}
@@ -2691,6 +2701,10 @@ queue_commits(struct tog_log_thread_args *a)
TAILQ_INSERT_TAIL(&a->real_commits->head, entry, entry);
a->real_commits->ncommits++;
+ if (tog_base_commit.id != NULL && tog_base_commit.idx == -1 &&
+ got_object_id_cmp(&id, tog_base_commit.id) == 0)
+ tog_base_commit.idx = entry->idx;
+
if (*a->limiting) {
err = match_commit(&limit_match, &id, commit,
a->limit_regex);
@@ -2949,7 +2963,7 @@ draw_commits(struct tog_view *view)
if (ncommits == s->selected)
wstandout(view->window);
err = draw_commit(view, entry->commit, entry->id,
- date_display_cols, author_cols);
+ date_display_cols, author_cols, entry->idx);
if (ncommits == s->selected)
wstandend(view->window);
if (err)
@@ -4365,6 +4379,20 @@ get_in_repo_path_from_argv0(char **in_repo_path, int a
}
static const struct got_error *
+set_tog_base_commit(struct got_repository *repo, struct got_worktree *worktree)
+{
+ tog_base_commit.id = got_object_id_dup(
+ got_worktree_get_base_commit_id(worktree));
+ if (tog_base_commit.id == NULL)
+ return got_error_from_errno( "got_object_id_dup");
+
+ tog_base_commit.idx = -1;
+
+ return got_worktree_get_state(&tog_base_commit.marker, repo,
+ worktree);
+}
+
+static const struct got_error *
get_in_repo_path_from_argv0(char **in_repo_path, int argc, char *argv[],
struct got_repository *repo, struct got_worktree *worktree)
{
@@ -4518,13 +4546,21 @@ cmd_log(int argc, char *argv[])
in_repo_path, log_branches);
if (error)
goto done;
+
if (worktree) {
+ error = set_tog_base_commit(repo, worktree);
+ if (error != NULL)
+ goto done;
+
/* Release work tree lock. */
got_worktree_close(worktree);
worktree = NULL;
}
+
error = view_loop(view);
+
done:
+ free(tog_base_commit.id);
free(keyword_idstr);
free(in_repo_path);
free(repo_path);
@@ -6046,8 +6082,17 @@ cmd_diff(int argc, char *argv[])
ignore_whitespace, force_text_diff, NULL, repo);
if (error)
goto done;
+
+ if (worktree) {
+ error = set_tog_base_commit(repo, worktree);
+ if (error != NULL)
+ goto done;
+ }
+
error = view_loop(view);
+
done:
+ free(tog_base_commit.id);
free(keyword_idstr1);
free(keyword_idstr2);
free(label1);
@@ -7194,13 +7239,21 @@ cmd_blame(int argc, char *argv[])
view_close(view);
goto done;
}
+
if (worktree) {
+ error = set_tog_base_commit(repo, worktree);
+ if (error != NULL)
+ goto done;
+
/* Release work tree lock. */
got_worktree_close(worktree);
worktree = NULL;
}
+
error = view_loop(view);
+
done:
+ free(tog_base_commit.id);
free(repo_path);
free(in_repo_path);
free(link_target);
@@ -8183,12 +8236,19 @@ cmd_tree(int argc, char *argv[])
}
if (worktree) {
+ error = set_tog_base_commit(repo, worktree);
+ if (error != NULL)
+ goto done;
+
/* Release work tree lock. */
got_worktree_close(worktree);
worktree = NULL;
}
+
error = view_loop(view);
+
done:
+ free(tog_base_commit.id);
free(keyword_idstr);
free(repo_path);
free(cwd);
@@ -8196,6 +8256,8 @@ done:
free(label);
if (ref)
got_ref_close(ref);
+ if (worktree != NULL)
+ got_worktree_close(worktree);
if (repo) {
const struct got_error *close_err = got_repo_close(repo);
if (error == NULL)
@@ -9032,14 +9094,23 @@ cmd_ref(int argc, char *argv[])
goto done;
if (worktree) {
+ error = set_tog_base_commit(repo, worktree);
+ if (error != NULL)
+ goto done;
+
/* Release work tree lock. */
got_worktree_close(worktree);
worktree = NULL;
}
+
error = view_loop(view);
+
done:
+ free(tog_base_commit.id);
free(repo_path);
free(cwd);
+ if (worktree != NULL)
+ got_worktree_close(worktree);
if (repo) {
const struct got_error *close_err = got_repo_close(repo);
if (close_err)
--
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
show base commit markers in tog log view