Paolo Bonzini <paolo.bonzini@xxxxxxxxxxx> writes:
> ...
> Signed-off-by: Paolo Bonzini <bonzini@xxxxxxx>
Beautifully done, with even a sign-off ;-).
> @@ -45,6 +46,16 @@ OPTIONS
> by gitlink:git-check-ref-format[1]. Some of these checks
> may restrict the characters allowed in a branch name.
>
> +--track::
> + When -b is given and a branch is created off a remote branch,
> + setup so that git-pull will automatically retrieve data from
> + the remote branch.
> +
> +--no-track::
> + When -b is given and a branch is created off a remote branch,
> + force that git-pull will automatically retrieve data from
> + the remote branch independent of the configuration settings.
> +
> -l::
> Create the new branch's ref log. This activates recording of
> all changes to made the branch ref, enabling use of date
Earlier I said "checkout -b" vs "checkout -B", but deciding the
tracking default with congiguration mechanism and overriding
with an additional --track/--no-track as you did here is way
superiour. Very nice.
> diff --git a/builtin-branch.c b/builtin-branch.c
> index d371849..34b1bbf 100644
> --- a/builtin-branch.c
> +++ b/builtin-branch.c
> @@ -308,14 +313,102 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev)
> +static int get_remote_branch_name(const char* value)
> +{
> + const char *colon;
> + const char *end;
> +
> + if (*value == '+')
> + value++;
> +
> + colon = strchr(value, ':');
> + end = value + strlen(value);
> +
> + /* Try an exact match first. */
> + if (!strcmp(colon + 1, start_ref)) {
Careful. colon may be NULL here, in which case you would want
to return with 0 from this function before this strcmp.
You seem to declare variables with "const char *colon" but
function args with "const char* value". Are they deliberately
done differently? The existing code seems to favor asterisk
next to the variable name, not type name.
Other than that, I find this function and get_remote_config()
very nicely done.
> +static void set_branch_defaults(const char *name, const char *real_ref)
> +{
> + char key[1024];
> + const char *slash = strrchr(real_ref, '/');
> +
> + if (!slash)
> + return;
> +
> + start_ref = real_ref;
> + start_len = strlen(real_ref);
> + base_len = slash - real_ref;
> + git_config(get_remote_config);
> +
> + /* Change to != 0 to enable this feature by default. */
> + if (config_repo && config_remote) {
> + if (snprintf(key, sizeof(key), "branch.%s.remote", name)
> + > sizeof(key))
> + die("what a long branch name you have!");
> + git_config_set(key, config_remote);
> +
> + snprintf(key, sizeof(key), "branch.%s.merge", name);
> + git_config_set(key, config_repo);
This is nice, but you might want to have a comment that says why
you do not have to check the return value from the second
snprintf(). Otherwise, somebody less careful may update this
part of the code and swap the order of config_remote and
config_repo are set up without thinking in the future.
> diff --git a/trace.c b/trace.c
> index 27fef86..1d4179d 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -41,13 +41,11 @@ static int git_vasprintf(char **strp, const char *fmt, va_list ap)
> return len;
> }
>
> +int nfasprintf(char **str, const char *fmt, ...)
> {
> + va_list args;
> + va_start(args, fmt);
> + return nfvasprintf(str, fmt, args);
> }
>
> /* Get a trace file descriptor from GIT_TRACE env variable. */
It might not matter on common architectures in practice, but
va_start() must be matched by a corresponding va_end() in the
same function.
-
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