Glen Choo [off-list ref] 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.