From: Mark Jamsek Subject: allow log -x to accept keywords and fix diffstat duplicates To: gameoftrees@openbsd.org Date: Sun, 06 Aug 2023 12:31:57 +1000 I'm inlining two somewhat related diffs because they're both small and also both suggested/reported by Lucas on IRC. Neither have tests yet for different reasons but I wanted to send these now. The first fixes^ the diffstat duplication when all three of -dPp are used (e.g., got log -dPp). I actually remember seeing this when first implementing diffstat and could've sworn I already had this check guarding the get_changed_paths() calls but must either be misremembering or ended up committing the wrong diff--I'd say the former is more likely :) The fix is simple enough, we only need to get changed paths once as they are used for the diffstat (-d) and path report (-P), so only call it once when both options are specified; we already had the similar -dp path guarded. Also, I've not added regress for this because I think op has already written a test but if not, I'll write a test before committing. The second allows 'got log -x' to take keywords :) Please see the docs for this; I opted for brevity as just above the -x docs we explain the commit argument in detail under -c. However, perhaps it might be better to provide a slightly less abridged version? I'll send a test for this later today but I've got to step out for now. ^There is an alternative fix for the diffstat duplication, which I've appended. I'm happy to go with whichever you like but do prefer the first because it is clearer; even though the second is shorter, it is not clear why we are doing it so would like to add a comment if you want to go with this fix. ----------------------------------------------- commit 6e5e7693c5beb221fe32f3be13cb2db22821c734 (main) from: Mark Jamsek date: Sat Aug 5 16:01:17 2023 UTC fix 'got log -dPp' diffstat duplication bug Only collect changed paths once if both -d and -P are specified; we already checked for -d and -p. Reported by Lucas on IRC. M got/got.c | 3+ 2- 1 file changed, 3 insertions(+), 2 deletions(-) diff 87853c6a48e0ffc60012a65024f4cdc4b03ed9fe 6e5e7693c5beb221fe32f3be13cb2db22821c734 commit - 87853c6a48e0ffc60012a65024f4cdc4b03ed9fe commit + 6e5e7693c5beb221fe32f3be13cb2db22821c734 blob - 3ca38bd57684cbbf48e72c190d7346237ec825ed blob + 29feb8935ab716a9191034d2ebede3da7cea1f73 --- got/got.c +++ got/got.c @@ -4460,7 +4460,8 @@ print_commits(struct got_object_id *root_id, struct go if (err) break; - if ((show_changed_paths || (show_diffstat && !show_patch)) + if (((show_changed_paths && !show_diffstat) || + (show_diffstat && !show_patch)) && !reverse_display_order) { err = get_changed_paths(&changed_paths, commit, repo, show_diffstat ? &dsa : NULL); @@ -4526,7 +4527,7 @@ print_commits(struct got_object_id *root_id, struct go &qid->id); if (err) break; - if (show_changed_paths || + if ((show_changed_paths && !show_diffstat) || (show_diffstat && !show_patch)) { err = get_changed_paths(&changed_paths, commit, repo, show_diffstat ? &dsa : NULL); ----------------------------------------------- commit 87853c6a48e0ffc60012a65024f4cdc4b03ed9fe from: Mark Jamsek date: Sat Aug 5 16:01:17 2023 UTC allow 'got log -x' to accept keywords Suggested by Lucas on IRC. M got/got.1 | 6+ 0- M got/got.c | 9+ 3- 2 files changed, 15 insertions(+), 3 deletions(-) diff 0f2e686eec562e28977521d25101acfa4396b47a 87853c6a48e0ffc60012a65024f4cdc4b03ed9fe commit - 0f2e686eec562e28977521d25101acfa4396b47a commit + 87853c6a48e0ffc60012a65024f4cdc4b03ed9fe blob - 9c2fa4cf0f0253f379430b774f532e2e54e6ebc3 blob + 10a6aa7aa384b8d8773e28516972143fd0c7c93a --- got/got.1 +++ got/got.1 @@ -1039,6 +1039,12 @@ This option has no effect if the specified Stop traversing commit history immediately after the specified .Ar commit has been traversed. +Like +.Fl c , +the expected +.Ar commit +argument is a commit ID SHA1 hash, or a reference name or a keyword +which will be resolved to a commit ID. This option has no effect if the specified .Ar commit is never traversed. blob - 81971e1f4c71f923c6da083afafa88b6001e10bf blob + 3ca38bd57684cbbf48e72c190d7346237ec825ed --- got/got.c +++ got/got.c @@ -4594,6 +4594,7 @@ cmd_log(int argc, char *argv[]) struct got_worktree *worktree = NULL; struct got_object_id *start_id = NULL, *end_id = NULL; char *repo_path = NULL, *path = NULL, *cwd = NULL, *in_repo_path = NULL; + char *keyword_idstr = NULL; const char *start_commit = NULL, *end_commit = NULL; const char *search_pattern = NULL; int diff_context = -1, ch; @@ -4760,8 +4761,6 @@ cmd_log(int argc, char *argv[]) goto done; got_object_commit_close(commit); } else { - char *keyword_idstr = NULL; - error = got_keyword_to_idstr(&keyword_idstr, start_commit, repo, worktree); if (error != NULL) @@ -4771,11 +4770,17 @@ cmd_log(int argc, char *argv[]) error = got_repo_match_object_id(&start_id, NULL, start_commit, GOT_OBJ_TYPE_COMMIT, &refs, repo); - free(keyword_idstr); if (error != NULL) goto done; } if (end_commit != NULL) { + error = got_keyword_to_idstr(&keyword_idstr, end_commit, + repo, worktree); + if (error != NULL) + goto done; + if (keyword_idstr != NULL) + end_commit = keyword_idstr; + error = got_repo_match_object_id(&end_id, NULL, end_commit, GOT_OBJ_TYPE_COMMIT, &refs, repo); if (error != NULL) @@ -4831,6 +4836,7 @@ done: free(cwd); free(start_id); free(end_id); + free(keyword_idstr); if (worktree) got_worktree_close(worktree); if (repo) { ^Alternative diffstat duplication fix: diff /home/mark/src/got commit - aca79412f514b698a2dc0124e95b35c676db2b6c path + /home/mark/src/got blob - b832189cd0505a6aecd517e5dcd0415a765338b3 file + got/got.c --- got/got.c +++ got/got.c @@ -4381,7 +4381,8 @@ print_commit(struct got_commit_object *commit, struct } } - err = print_patch(commit, id, path, diff_context, diffstat, + err = print_patch(commit, id, path, diff_context, + diffstat != NULL && diffstat->nfiles ? NULL : diffstat, repo, diffstat == NULL ? stdout : f); if (err) goto done; -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68