On Fri, Feb 13, 2009 at 5:09 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Calling the subcommand a "verb" is somewhat new, though. Existing
> documentation for git commands that take multiple actions seem to call
> them subcommands, including "git-remote.txt" itself.
Okay.
> Hmph, what does "-a" stand for? I would have expected to see "-u" that
> stands for "update" here.
--automatic -- as in figure out the name automatically from the other side.
> Also it may be better to be more explicit about both the syntax and the
> semantics of `<branch>`.
Okay.
> Do you expect "refs/remotes/<name>/master" or
> just "master" (I assume the latter)?
Yes, the latter. If you did the wrong thing the error ought clue you in:
$ ./git remote set-head origin refs/remotes/origin/master
error: Not a valid ref: refs/remotes/origin/refs/remotes/origin/master
> Is it an error if the branch does
> not exist in the specified hierarchy?
Yes it is an error per-above. Well, at least on-top of next it is.
> Can you force to set to a branch
> that does not exist in your tracking side (yet) but you know exists on the
> remote side already?
No.
>> diff --git a/builtin-remote.c b/builtin-remote.c
>> index 465c87a..677e20e 100644
>> --- a/builtin-remote.c
>> +++ b/builtin-remote.c
>> @@ -658,7 +659,8 @@ static void free_remote_ref_states(struct ref_states *states)
>> string_list_clear(&states->new, 0);
>> string_list_clear(&states->stale, 0);
>> string_list_clear(&states->tracked, 0);
>> - free(states->head_name);
>> + if (states->head_name)
>> + free(states->head_name);
>> }
>
> Regression?
Indeed.
> set_head()?
Yep.
> The code will scale better, especially for a young subcommand that may acquire
> new options, if the check is done by each codepath that deals with a
> specific option to do this kind of check. That is, e.g.
>
> if (opt_delete) {
> error if the arg is not remote (alone)
> do the "delete" thing
> } else if (opt_update) {
> error if the arg is not remote (alone)
> do the "update" thing
> } else {
> error if the args are not (remote, branch)
> do the "set" thing
> }
Got it. I really am trying to match existing code, but it seems the
standards have gotten higher, so I need to do better than existing
code.
Thanks,
j.
--
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
