Re: [PATCH v2 6/6] worktree: teach `list` verbose mode
flat view
From: Eric Sunshine <hidden>
Date: 2021-01-18 05:16:03
On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva [off-list ref] wrote:
"git worktree list" annotates each worktree according to its state such as "prunable" or "locked", however it is not immediately obvious why these worktrees are being annotated. For prunable worktrees a reason is available that is returned by should_prune_worktree() and for locked worktrees a reason might be available provided by the user via `lock` command. Let's teach "git worktree list" to output the reason why the worktrees are being annotated. The reason is a text that can take virtually any size and appending the text on the default columned format will make it difficult to extend the command with other annotations and not fit nicely on the screen. In order to address this shortcoming the annotation is then moved to the next line indented followed by the reason, if the reason is not available the annotation stays on the same line as the worktree itself.
If you're re-rolling, let's mention the new `--verbose` option
somewhere in the commit message since that is the focus of this patch.
The second paragraph would be a good place:
Let's teach "git worktree list" a --verbose mode which
outputs the reason...
Also, the final sentence is a bit difficult to follow due to the comma
before "if the reason is not available". If you make the "if the
reason is not available" a separate sentence, it becomes simple to
understand.
quoted hunk ↗ jump to hunk
diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh@@ -135,6 +135,33 @@ test_expect_success '"list" all worktrees with prunable consistent with "prune"' +test_expect_success '"list" all worktrees --verbose with locked' ' + test_when_finished "rm -rf locked out actual expect && git worktree prune" && + git worktree add locked --detach && + git worktree lock locked --reason "with reason" && + test_when_finished "git worktree unlock locked" && + echo "$(git -C locked rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >expect && + printf "\tlocked: with reason\n" >>expect && + git worktree list --verbose >out && + sed -n "s/ */ /g;/\/locked *[0-9a-f].*$/,/locked: .*$/p" <out >actual && + test_cmp actual expect +'
At first, I wondered if we would also want this test to have a locked-no-reason worktree to ensure that its `locked` annotation stays on the same line as the worktree, but that's not needed because that case is already covered by the existing test. Fine.
quoted hunk ↗ jump to hunk
+test_expect_success '"list" all worktrees --verbose with prunable' ' + test_when_finished "rm -rf prunable out actual expect && git worktree prune" && + git worktree add prunable --detach && + echo "$(git -C prunable rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >expect && + printf "\tprunable: gitdir file points to non-existent location\n" >>expect && + rm -rf prunable && + git worktree list --verbose >out && + sed -n "s/ */ /g;/\/prunable *[0-9a-f].*$/,/prunable: .*$/p" <out >actual && + test_i18ncmp actual expect +'
An alternative would be to have a single test of --verbose which includes a locked-no-reason worktree, a locked-with-reason worktree, and a prunable worktree. However, that's a very minor and subjective point and certainly not worth a re-roll or changing unless you think it's a nice simplification.