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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).