On Thu, Aug 24, 2017 at 2:54 AM, Stefan Beller <sbel...@google.com> wrote:
>> +int other_head_refs(each_ref_fn fn, void *cb_data)
>> +{
>> + struct worktree **worktrees, **p;
>> + int ret = 0;
>> +
>> + worktrees = get_worktrees(0);
>> + for (p = worktrees; *p; p++) {
>> + struct worktree *wt = *p;
>> + struct ref_store *refs;
>> +
>> + if (wt->is_current)
>> + continue;
>
> As said in an earlier patch (and now this pattern
> coming up twice in this patch series alone), the lines
> of this function up to here, could become
> part of a worktree iterator function?
There are a couple "loop through all worktrees" code so far, but the
filter condition is not always this.
While I like the idea of iterator function (especially if it does
"struct worktree" memory management internally), I think it's a bit
overkill to go for_each_worktree() with a callback function when the
equivalent for(;;) is so short. We would need to declare struct to
pass callback data, and the reader may have to got read
for_each_worktree() code then come back here.
So, probably no worktree iterator (yet).
>> + refs = get_worktree_ref_store(wt);
>> + ret = refs_head_ref(refs, fn, cb_data);
>> + if (ret)
>> + break;
>
> with these 4 lines in the callback function.
>
>> +/*
>> + * Similar to head_ref() for all HEADs _except_ one from the current
>> + * worktree, which is covered by head_ref().
>> + */
>> +int other_head_refs(each_ref_fn fn, void *cb_data);
>
> This is already such an iterator function, just at another
> abstraction level.
yeah.. but we can't mix and match (or combine) ref/worktree iterator callbacks
--
Duy
