Re: [PATCH v3 1/3] get_main_worktree(): allow it to be called in the Git directory

> Hi Eric,
>
> On Mon, Feb 24, 2020 at 7:12 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>>
>> This title is a bit too generic; it fails to explain what this patch
>> is really fixing. Perhaps:
>>
>>     get_main_worktree: correctly normalize worktree path when in .git dir
>>
>> or something.
>>
>> "Git directory" is imprecise. As a reader, I can't tell if this means
>> the main worktree into which the project is checked out or the `.git`
>> directory itself. Please write it instead as "`.git` directory".
>> [...]
>> This change makes the code unnecessarily confusing and effectively
>> turns the final line into dead code. I would much rather see the three
>> cases spelled out explicitly, perhaps like this:
>>
>>     if (!strbuf_strip_suffix(&worktree_path, "/.git/.") && /* in .git dir */
>>         !strbuf_strip_suffix(&worktree_path, "/.git/")) /* in worktree */
>>             strbuf_strip_suffix(&worktree_path, "/."); /* in bare repo */
>
> I'll implement these comments in the next revision for sure.
>
>> Also, please add a test to ensure that this behavior doesn't regress
>> in the future. You can probably test it via the "git worktree list"
>> command, so perhaps add the test to t/t2402-worktree-list.sh.
>
> There already exists tests in "t/t2402-worktree-list.sh" which lists and
> verifies all worktrees. Does this make sense to write a new test that
> also does kinda the same thing?