From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 3/3] checkout: move branch guessing code out as a separate function
Date: Tue, 28 Aug 2012 20:49:08 +0700 [thread overview]
Message-ID: <1346161748-25651-4-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1346161748-25651-1-git-send-email-pclouds@gmail.com>
This makes cmd_checkout() shorter, therefore easier to get the big
picture.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
In general I like it, except that parse_branchname_arg() pulls so many
options from cmd_checkout(). I would rather have update_new_branch()
that only takes "struct checkout_opts *" and returns a new branch. So
I'm not sure if we should do this. If we do, perhaps we can merge it
into the previous patch.
builtin/checkout.c | 184 ++++++++++++++++++++++++++++-------------------------
1 file changed, 96 insertions(+), 88 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3f1a436..e9d503d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -914,6 +914,99 @@ static int parse_branchname_arg(int argc, const char **argv,
return argcount;
}
+static const char *update_new_branch(const struct checkout_opts *opts,
+ int argc, const char **argv,
+ const char *prefix, const char ***pathspec,
+ int dwim_ok,
+ struct branch_info *new,
+ struct tree **source_tree,
+ unsigned char *rev)
+{
+ const char *branch = opts->new_branch;
+
+ if (opts->new_branch_force) {
+ /* we can assume from now on new_branch = !new_branch_force */
+ if (branch)
+ die(_("New branch '%s' is already specified by other options"),
+ branch);
+
+ /* copy -B over to -b, so that we can just check the latter */
+ if (opts->new_branch_force)
+ branch = opts->new_branch_force;
+ }
+
+ if (opts->new_orphan_branch) {
+ if (opts->track > 0)
+ die(_("--orphan cannot be used with -t"));
+ if (branch)
+ die(_("New branch '%s' is already specified by other options"),
+ branch);
+ branch = opts->new_orphan_branch;
+ }
+
+ /* --track without -b should DWIM */
+ if (0 < opts->track && !branch) {
+ const char *argv0 = argv[0];
+ if (!argc || !strcmp(argv0, "--"))
+ die (_("--track needs a branch name"));
+ 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"));
+ branch = argv0 + 1;
+ }
+
+ /*
+ * Extract branch name from command line arguments, so
+ * all that is left is pathspecs.
+ *
+ * Handle
+ *
+ * 1) git checkout <tree> -- [<paths>]
+ * 2) git checkout -- [<paths>]
+ * 3) git checkout <something> [<paths>]
+ *
+ * including "last branch" syntax and DWIM-ery for names of
+ * remote branches, erroring out for invalid or ambiguous cases.
+ */
+ if (argc) {
+ dwim_ok = dwim_ok &&
+ opts->track == BRANCH_TRACK_UNSPECIFIED &&
+ !branch;
+ int n = parse_branchname_arg(argc, argv, dwim_ok,
+ new, source_tree, rev, &branch);
+ argv += n;
+ argc -= n;
+ }
+
+ /* After DWIM, these must be pathspec */
+ if (argc) {
+ *pathspec = get_pathspec(prefix, argv);
+
+ if (!*pathspec)
+ die(_("invalid path specification"));
+
+ /* Try to give more helpful suggestion, new_branch &&
+ argc > 1 will be caught later */
+ if (branch && argc == 1)
+ die(_("Cannot update paths and switch to branch '%s' at the same time.\n"
+ "Did you intend to checkout '%s' which can not be resolved as commit?"),
+ branch, argv[0]);
+
+ if (opts->force_detach)
+ die(_("git checkout: --detach does not take a path argument"));
+
+ if (1 < !!opts->writeout_stage + !!opts->force + !!opts->merge)
+ die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\n"
+ "checking out of the index."));
+ }
+
+ return branch;
+}
+
static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
{
int status;
@@ -989,94 +1082,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
}
- /*
- * opts.new_branch calculation, it could be from -b or -B or
- * --track or --orphan or other command line arguments.
- *
- * The following code until new branch validation should not
- * update any fields in opts but opts.new_branch.
- */
- if (opts.new_branch_force) {
- /* we can assume from now on new_branch = !new_branch_force */
- if (opts.new_branch)
- die(_("New branch '%s' is already specified by other options"),
- opts.new_branch);
-
- /* copy -B over to -b, so that we can just check the latter */
- if (opts.new_branch_force)
- opts.new_branch = opts.new_branch_force;
- }
-
- if (opts.new_orphan_branch) {
- if (opts.track > 0)
- die(_("--orphan cannot be used with -t"));
- if (opts.new_branch)
- die(_("New branch '%s' is already specified by other options"),
- opts.new_branch);
- opts.new_branch = opts.new_orphan_branch;
- }
-
- /* --track without -b should DWIM */
- if (0 < opts.track && !opts.new_branch) {
- const char *argv0 = argv[0];
- if (!argc || !strcmp(argv0, "--"))
- die (_("--track needs a branch name"));
- 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 = argv0 + 1;
- }
-
- /*
- * Extract branch name from command line arguments, so
- * all that is left is pathspecs.
- *
- * Handle
- *
- * 1) git checkout <tree> -- [<paths>]
- * 2) git checkout -- [<paths>]
- * 3) git checkout <something> [<paths>]
- *
- * including "last branch" syntax and DWIM-ery for names of
- * remote branches, erroring out for invalid or ambiguous cases.
- */
- if (argc) {
- int dwim_ok =
- !patch_mode &&
- dwim_new_local_branch &&
- opts.track == BRANCH_TRACK_UNSPECIFIED &&
- !opts.new_branch;
- int n = parse_branchname_arg(argc, argv, dwim_ok,
- &new, &source_tree, rev, &opts.new_branch);
- argv += n;
- argc -= n;
- }
-
- /* After DWIM, these must be pathspec */
- if (argc) {
- pathspec = get_pathspec(prefix, argv);
-
- if (!pathspec)
- die(_("invalid path specification"));
-
- /* Try to give more helpful suggestion, new_branch &&
- argc > 1 will be caught later */
- if (opts.new_branch && argc == 1)
- die(_("Cannot update paths and switch to branch '%s' at the same time.\n"
- "Did you intend to checkout '%s' which can not be resolved as commit?"),
- opts.new_branch, argv[0]);
-
- if (opts.force_detach)
- die(_("git checkout: --detach does not take a path argument"));
-
- if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
- die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\n"
- "checking out of the index."));
- }
+ opts.new_branch = update_new_branch(&opts, argc, argv, prefix, &pathspec,
+ !patch_mode && dwim_new_local_branch,
+ &new, &source_tree, rev);
/* new branch calculation done, validate it */
if (opts.new_branch) {
--
1.7.12.rc2.18.g61b472e
prev parent reply other threads:[~2012-08-28 13:56 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-28 2:29 [PATCH] checkout: verify new branch name's validity early Nguyễn Thái Ngọc Duy
2012-08-28 3:55 ` Junio C Hamano
2012-08-28 13:49 ` [PATCH 0/3] Refactor checkout option handling Nguyễn Thái Ngọc Duy
2012-08-28 13:49 ` [PATCH 1/3] checkout: pass "struct checkout_opts *" as const pointer Nguyễn Thái Ngọc Duy
2012-08-28 13:49 ` [PATCH 2/3] checkout: reorder option handling Nguyễn Thái Ngọc Duy
2012-08-28 20:45 ` Junio C Hamano
2012-08-29 12:16 ` Nguyen Thai Ngoc Duy
2012-08-29 13:55 ` [PATCH v2 1/3] checkout: pass "struct checkout_opts *" as const pointer Nguyễn Thái Ngọc Duy
2012-08-29 13:55 ` [PATCH v2 2/3] checkout: move more parameters to struct checkout_opts Nguyễn Thái Ngọc Duy
2012-08-29 13:55 ` [PATCH v2 3/3] checkout: reorder option handling Nguyễn Thái Ngọc Duy
2012-08-29 18:37 ` Junio C Hamano
2012-08-29 18:55 ` Junio C Hamano
2012-08-30 12:45 ` [PATCH v3 " Nguyễn Thái Ngọc Duy
2012-08-30 16:10 ` Junio C Hamano
2012-09-07 20:19 ` Junio C Hamano
2012-08-28 13:49 ` Nguyễn Thái Ngọc Duy [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1346161748-25651-4-git-send-email-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.