Download raw body.
show base commit markers in tog log view
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.
-----------------------------------------------
commit 88c87e08a822bf5882cebc085a5bc80ebb5ffbc1 (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Jul 22 03:51:19 2023 UTC
got: improve reporting accuracy in branch listing output
Show the out-of-date symbol for the work tree branch if it is not only out
of date in relation to the branch tip but also if it contains mixed commits.
Update regress and add new test to check for this case. Suggested by stsp.
M got/got.c | 5+ 12-
M regress/cmdline/branch.sh | 81+ 1-
2 files changed, 86 insertions(+), 13 deletions(-)
diff 4662ce8b75136305615c582c44324229e4baafce 88c87e08a822bf5882cebc085a5bc80ebb5ffbc1
commit - 4662ce8b75136305615c582c44324229e4baafce
commit + 88c87e08a822bf5882cebc085a5bc80ebb5ffbc1
blob - 11de03d9dc00687d501d46094c4ba5444e8c8cb6
blob + ff960485254894c4b8c41fda56a2319bc2b89b17
--- got/got.c
+++ got/got.c
@@ -6797,23 +6797,16 @@ list_branch(struct got_repository *repo, struct got_wo
struct got_reference *ref)
{
const struct got_error *err = NULL;
- const char *refname, *marker = " ";
+ const char *refname;
char *refstr;
+ char marker = ' ';
refname = got_ref_get_name(ref);
if (worktree && strcmp(refname,
got_worktree_get_head_ref_name(worktree)) == 0) {
- struct got_object_id *id = NULL;
-
- err = got_ref_resolve(&id, repo, ref);
- if (err)
+ err = got_worktree_get_state(&marker, repo, worktree);
+ if (err != NULL)
return err;
- if (got_object_id_cmp(id,
- got_worktree_get_base_commit_id(worktree)) == 0)
- marker = "* ";
- else
- marker = "~ ";
- free(id);
}
if (strncmp(refname, "refs/heads/", 11) == 0)
@@ -6827,7 +6820,7 @@ list_branch(struct got_repository *repo, struct got_wo
if (refstr == NULL)
return got_error_from_errno("got_ref_to_str");
- printf("%s%s: %s\n", marker, refname, refstr);
+ printf("%c %s: %s\n", marker, refname, refstr);
free(refstr);
return NULL;
}
blob - 71213582275291870b3a3f5941f02a76914cc1c1
blob + 4bc9143544d7cac2ae3c847fbeb41f48e55a1f0a
--- regress/cmdline/branch.sh
+++ regress/cmdline/branch.sh
@@ -524,7 +524,7 @@ test_branch_packed_ref_collision() {
# variants of ref/heads/zoo:
(cd $testroot/wt && got br -lt > $testroot/stdout)
- echo "* zoo: $commit_id2" > $testroot/stdout.expected
+ echo "~ zoo: $commit_id2" > $testroot/stdout.expected
echo " master: $commit_id" >> $testroot/stdout.expected
cmp -s $testroot/stdout $testroot/stdout.expected
ret=$?
@@ -594,6 +594,85 @@ test_parseargs "$@"
test_done "$testroot" "$ret"
}
+test_branch_list_worktree_state() {
+ local testroot=$(test_init branch_list_worktree_state)
+ local wt="$testroot/wt"
+
+ set -- "$(git_show_head "$testroot/repo")"
+
+ got checkout "$testroot/repo" "$wt" > /dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "checkout failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ (cd "$wt" && got br -n newbranch > /dev/null)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "branch failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # check up-to-date marker is shown with fresh checkout
+ (cd "$wt" && got br -l > "$testroot/stdout")
+ echo "* master: $(pop_id 1 $@)" > $testroot/stdout.expected
+ echo " newbranch: $(pop_id 1 $@)" >> $testroot/stdout.expected
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # check out-of-date marker is shown with mixed-commit state
+ echo "mixed-commit" > "$wt/alpha"
+ (cd "$wt" && got commit -m "mixed-commit" > "$testroot/stdout")
+ set -- "$@" "$(git_show_head "$testroot/repo")"
+
+ (cd "$wt" && got br -l > "$testroot/stdout")
+ echo "~ master: $(pop_id 2 $@)" > $testroot/stdout.expected
+ echo " newbranch: $(pop_id 1 $@)" >> $testroot/stdout.expected
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # check up-to-date marker is shown after 'got update'
+ (cd "$wt" && got up > /dev/null)
+ (cd "$wt" && got br -l > "$testroot/stdout")
+ echo "* master: $(pop_id 2 $@)" > $testroot/stdout.expected
+ echo " newbranch: $(pop_id 1 $@)" >> $testroot/stdout.expected
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # check out-of-date marker is shown with out-of-date base commit
+ (cd "$wt" && got up -c:head:- > /dev/null)
+ (cd "$wt" && got br -l > "$testroot/stdout")
+ echo "~ master: $(pop_id 2 $@)" > $testroot/stdout.expected
+ echo " newbranch: $(pop_id 1 $@)" >> $testroot/stdout.expected
+ cmp -s $testroot/stdout $testroot/stdout.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ test_done "$testroot" "$ret"
+}
+
test_parseargs "$@"
run_test test_branch_create
run_test test_branch_list
@@ -603,3 +682,4 @@ run_test test_branch_commit_keywords
run_test test_branch_show
run_test test_branch_packed_ref_collision
run_test test_branch_commit_keywords
+run_test test_branch_list_worktree_state
-----------------------------------------------
commit 4662ce8b75136305615c582c44324229e4baafce
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Jul 22 03:51:18 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 | 66+ 2-
4 files changed, 215 insertions(+), 3 deletions(-)
diff 9c986b77d3f71b37ff5b5c6b4ec58b495ad8f312 4662ce8b75136305615c582c44324229e4baafce
commit - 9c986b77d3f71b37ff5b5c6b4ec58b495ad8f312
commit + 4662ce8b75136305615c582c44324229e4baafce
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 + 7c749a9c6606cec9ca4f6d47f0b2635d75ed0eb6
--- tog/tog.c
+++ tog/tog.c
@@ -151,6 +151,10 @@ 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;
+ char marker;
+} tog_base_commit;
static enum got_diff_algorithm tog_diff_algo = GOT_DIFF_ALGORITHM_MYERS;
static const struct got_error *
@@ -2411,7 +2415,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 +2480,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 &&
+ got_object_id_cmp(id, tog_base_commit.id) == 0)
+ waddch(view->window, tog_base_commit.marker);
+ else
+ waddch(view->window, ' ');
col++;
author_width++;
}
@@ -4365,6 +4374,18 @@ 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");
+
+ 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 +4539,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 +6075,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 +7232,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 +8229,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 +8249,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 +9087,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