Glen Choo <chooglen@xxxxxxxxxx> writes:
> Hm, what do you think of an alternate approach of storing of the
> matching remotes in a string_list, something like:
>
> struct find_tracked_branch_cb {
> struct tracking *tracking;
> struct string_list matching_remotes;
> };
>
> static int find_tracked_branch(struct remote *remote, void *priv)
> {
> struct tracking *tracking = priv;
> struct find_tracked_branch_cb *ftb = priv;
> struct tracking *tracking = ftb->tracking;
>
> if (!remote_find_tracking(remote, &tracking->spec)) {
> if (++tracking->matches == 1) {
> string_list_append(tracking->srcs, tracking->spec.src);
> tracking->remote = remote->name;
> } else {
> free(tracking->spec.src);
> string_list_clear(tracking->srcs, 0);
> }
> string_list_append(&ftb->matching_remotes, remote->name);
> tracking->spec.src = NULL;
> }
>
> then construct the advice message in setup_tracking()? To my untrained
> eye, "case 2" requires a bit of extra work to understand.
If I were writing this code from scratch, I would have probably
collected the names first in this callback, and assembled them at
the end into a single string when we need a single string to show.
Having said that, as long as you do that lazily not to penalize
those who have sane setting without the need for advice/error to
trigger, I do not particularly care how the list of matching remote
names are kept. Having string_list_append() unconditionally like
the above patch has, even for folks with just a single match without
need for the advice/error message is suboptimal, I would think.
