Alex Riesen <raa.lkml@xxxxxxxxx> writes:
> The code handles additionally "refs/remotes/<something>/name",
> "remotes/<something>/name", and "refs/<namespace>/name".
>
> Signed-off-by: Alex Riesen <raa.lkml@xxxxxxxxx>
> ---
> ...
> I just have another quiet evening, so I did that. Johannes, I changed
> your fix a bit: I see that argv[0] is used later (or if I'm blind and
> it actually isn't, it may be used in future: I have a feeling that
> builtint-checkout.c will be popular place).
Thanks; the documentation looks good to me.
> + char *argv0 = argv[0];
> + if (!argc || !strcmp(argv0, "--"))
> die ("--track needs a branch name");
> + slash = strchr(argv0, '/');
> + if (slash && !prefixcmp(argv0, "refs/")) {
> + argv0 = slash + 1;
> + slash = strchr(argv0, '/');
> + }
> + if (slash && !prefixcmp(argv0, "remotes/"))
> slash = strchr(slash + 1, '/');
> if (!slash || !slash[1])
> die ("Missing branch name; try -b");
I however wonder if this is clearer.
* "enum branch_track" was unsigned; comparing equality with -1 was Ok but
we couldn't say 0 < opts.track;
* argv[] is an array of constant strings; cannot point into it with
opts.newbranch without making the latter also a constant string.
* the logic is to strip "refs/" if there is one, "remotes/" if there is
one after that, and then strip one level after that unconditionally.
No need to look explicitly for a slash while doing the first two steps.
cache.h | 1 +
builtin-checkout.c | 26 +++++++++++++-------------
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git i/cache.h w/cache.h
index 928ae9f..a097a95 100644
--- i/cache.h
+++ w/cache.h
@@ -451,6 +451,7 @@ enum safe_crlf {
extern enum safe_crlf safe_crlf;
enum branch_track {
+ BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
BRANCH_TRACK_REMOTE,
BRANCH_TRACK_ALWAYS,
diff --git i/builtin-checkout.c w/builtin-checkout.c
index e95eab9..b380ad6 100644
--- i/builtin-checkout.c
+++ w/builtin-checkout.c
@@ -157,7 +157,7 @@ struct checkout_opts {
int force;
int writeout_error;
- char *new_branch;
+ const char *new_branch;
int new_branch_log;
enum branch_track track;
};
@@ -437,27 +437,27 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
git_config(git_default_config, NULL);
- opts.track = -1;
+ opts.track = BRANCH_TRACK_UNSPECIFIED;
argc = parse_options(argc, argv, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
/* --track without -b should DWIM */
- if (opts.track && opts.track != -1 && !opts.new_branch) {
- char *slash;
- if (!argc || !strcmp(argv[0], "--"))
+ if (0 < opts.track && !opts.new_branch) {
+ const char *argv0 = argv[0];
+ if (!argc || !strcmp(argv0, "--"))
die ("--track needs a branch name");
- slash = strchr(argv[0], '/');
- if (slash && !prefixcmp(argv[0], "refs/"))
- slash = strchr(slash + 1, '/');
- if (slash && !prefixcmp(argv[0], "remotes/"))
- slash = strchr(slash + 1, '/');
- if (!slash || !slash[1])
+ if (!prefixcmp(argv0, "refs/"))
+ argv0 += 5;
+ if (!prefixcmp(argv0, "remotes/"))
+ argv0 += 8;
+ argv0 = strchr(argv0, '/');
+ if (!argv0 || !argv0[1])
die ("Missing branch name; try -b");
- opts.new_branch = slash + 1;
+ opts.new_branch = argv0 + 1;
}
- if (opts.track == -1)
+ if (opts.track == BRANCH_TRACK_UNSPECIFIED)
opts.track = git_branch_track;
if (opts.force && opts.merge)
--
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
