Re: [PATCH v7 1/3] worktree: add top-level worktree.c

> I realize that this is modeled closely after existing code in
> branch.c, but, with the exception of parsing the ref file and
> constructing a worktree structure, the main worktree case (id == NULL)
> is entirely disjoint from the linked worktree case (id != NULL). This
> suggests strongly that get_worktree() should be split into two
> functions, one for the main worktree and one for linked worktrees,
> which would make the code easier to understand. You might call the
> functions get_main_worktree() and get_linked_worktree(id) (or perhaps
> drop "linked" from the latter name).

>> +
>> +struct worktree_list *get_worktree_list()
>
> Can we be more concise and call this get_worktrees()?
>

>> diff --git a/worktree.h b/worktree.h
>> new file mode 100644
>> index 0000000..2bc0ab8
>> --- /dev/null
>> +++ b/worktree.h
>> @@ -0,0 +1,48 @@
>> +#ifndef WORKTREE_H
>> +#define WORKTREE_H
>> +
>> +struct worktree {
>> +       char *path;
>> +       char *git_dir;
>> +       char *head_ref;
>> +       unsigned char head_sha1[20];
>> +       int is_detached;
>> +       int is_bare;
>> +};
>> +
>> +struct worktree_list {
>> +       struct worktree *worktree;
>> +       struct worktree_list *next;
>> +};
>
> I don't care too strongly, but an alternate approach (which I probably
> would have taken) would be to have get_worktrees() simply return an
> array of 'struct worktree' objects, hence no need for the additional
> 'struct worktree_list'. The slight complication with this approach,
> though, is that get_worktrees() either also needs to return the length
> of the array, or the array should end with some sort of end-of-array
> sentinel. An obvious sentinel would be path==NULL or git_dir==NULL or
> all of the above.
>
> Client iteration is just about the same with the array approach as
> with the linked-list approach.
>