Hi,
On Mon, 18 Feb 2008, Jay Soffian wrote:
> diff --git a/branch.c b/branch.c
> index 1fc8788..64f0a4a 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -49,8 +49,12 @@ static int setup_tracking(const char *new_ref, const char *orig_ref)
> memset(&tracking, 0, sizeof(tracking));
> tracking.spec.dst = (char *)orig_ref;
> if (for_each_remote(find_tracked_branch, &tracking) ||
> - !tracking.matches)
> - return 1;
> + !tracking.matches) {
> + if (!always)
> + return 1;
> + tracking.matches = 1;
> + tracking.src = xstrdup(orig_ref);
> + }
I think you need to split this into
if (for_each_remote(...))
return 1;
if (!tracking.matches && !always)
return 1;
because an error in for_each_remote() is a proper error, and should not be
ignored (speaking of which, the return value is unchecked...). And then,
later,
if (tracking.matches == 1) {
sprintf(key, "branch.%s.remote", new_ref);
git_config_set(key, tracking.remote ? tracking.remote : ".");
sprintf(key, "branch.%s.merge", new_ref);
- git_config_set(key, tracking.src);
+ git_config_set(key, tracking.src ? tracking.src : orig_ref);
free(tracking.src);
> @@ -130,7 +135,7 @@ void create_branch(const char *head,
> automatically merges from there. So far, this is only done for
> remotes registered via .git/config. */
> if (real_ref && track)
> - setup_tracking(name, real_ref);
> + setup_tracking(name, real_ref, (track == BRANCH_TRACK_ALWAYS));
It is a matter of taste, I guess, but I would remove the parens around the
boolean expression.
FWIW a very good argument in favour of your patch is IMHO that now,
"--track" works as a user would expect, even for local branch-offs.
And of course that you were very responsive, and really put in a lot of
effort.
Thanks,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
