This makes cmd_checkout() shorter, therefore easier to get the big
picture.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to