{"thread":{"id":"59931","subject":"[PATCH] fsck: avoid misleading variable name","startedAt":"2023-06-29T18:16:11Z","lastAt":"2023-06-29T19:12:50Z","messageCount":2,"participants":["Eric Sunshine","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479019","messageId":"20230629181333.87465-1-ericsunshine@charter.net","threadId":"59931","inReplyTo":null,"subject":"[PATCH] fsck: avoid misleading variable name","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2023-06-29T18:13:33Z","receivedAt":"2023-06-29T18:16:11Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nWhen reporting a problem, `git fsck` emits a message such as:\n\n    missing blob 1234abcd (:file)\n\nHowever, this can be ambiguous when the problem is detected in the index\nof a worktree other than the one in which `git fsck` was invoked. To\naddress this shortcoming, 592ec63b38 (fsck: mention file path for index\nerrors, 2023-02-24) enhanced the output to mention the path of the index\nwhen the problem is detected in some other worktree:\n\n    missing blob 1234abcd (.git/worktrees/wt/index:file)\n\nUnfortunately, the variable in fsck_index() which controls whether the\nindex path should be shown is misleadingly named \"is_main_index\" which\ncan be misunderstood as referring to the main worktree (i.e. the one\nhousing the .git/ repository) rather than to the current worktree (i.e.\nthe one in which `git fsck` was invoked). Avoid such potential confusion\nby choosing a name more reflective of its actual purpose.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n\nThe associated discussion which led to this patch begins at [1].\n\n[1]: https://lore.kernel.org/git/305ccc55-25e3-6b01-cd86-9a9035839d06@sunshineco.com/\n\n builtin/fsck.c  | 4 ++--\n t/t1450-fsck.sh | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex d9aa4db828..0c00920703 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -808,7 +808,7 @@ static int fsck_resolve_undo(struct index_state *istate,\n }\n \n static void fsck_index(struct index_state *istate, const char *index_path,\n-\t\t       int is_main_index)\n+\t\t       int is_current_worktree)\n {\n \tunsigned int i;\n \n@@ -830,7 +830,7 @@ static void fsck_index(struct index_state *istate, const char *index_path,\n \t\tobj->flags |= USED;\n \t\tfsck_put_object_name(&fsck_walk_options, &obj->oid,\n \t\t\t\t     \"%s:%s\",\n-\t\t\t\t     is_main_index ? \"\" : index_path,\n+\t\t\t\t     is_current_worktree ? \"\" : index_path,\n \t\t\t\t     istate->cache[i]->name);\n \t\tmark_object_reachable(obj);\n \t}\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 8c442adb1a..5805d47eb9 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -1036,9 +1036,9 @@ test_expect_success 'fsck detects problems in worktree index' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'fsck reports problems in main index without filename' '\n+test_expect_success 'fsck reports problems in current worktree index without filename' '\n \ttest_when_finished \"rm -f .git/index && git read-tree HEAD\" &&\n-\techo \"this object will be removed to break the main index\" >file &&\n+\techo \"this object will be removed to break current worktree index\" >file &&\n \tgit add file &&\n \tblob=$(git rev-parse :file) &&\n \tremove_object $blob &&\n-- \n2.41.0.362.gccff93557d\n\n"},{"id":"479024","messageId":"20230629190421.GA592842@coredump.intra.peff.net","threadId":"59931","inReplyTo":"20230629181333.87465-1-ericsunshine@charter.net","subject":"Re: [PATCH] fsck: avoid misleading variable name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-06-29T19:04:21Z","receivedAt":"2023-06-29T19:12:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 29, 2023 at 02:13:33PM -0400, Eric Sunshine wrote:\n\n> From: Eric Sunshine <sunshine@sunshineco.com>\n> \n> When reporting a problem, `git fsck` emits a message such as:\n> \n>     missing blob 1234abcd (:file)\n> \n> However, this can be ambiguous when the problem is detected in the index\n> of a worktree other than the one in which `git fsck` was invoked. To\n> address this shortcoming, 592ec63b38 (fsck: mention file path for index\n> errors, 2023-02-24) enhanced the output to mention the path of the index\n> when the problem is detected in some other worktree:\n> \n>     missing blob 1234abcd (.git/worktrees/wt/index:file)\n> \n> Unfortunately, the variable in fsck_index() which controls whether the\n> index path should be shown is misleadingly named \"is_main_index\" which\n> can be misunderstood as referring to the main worktree (i.e. the one\n> housing the .git/ repository) rather than to the current worktree (i.e.\n> the one in which `git fsck` was invoked). Avoid such potential confusion\n> by choosing a name more reflective of its actual purpose.\n\nThis looks good to me. Thanks for following up!\n\n-Peff\n"}]}