Re: [PATCH 3/4] worktree add: add --orphan flag
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 4, 2022, 01:33 UTC
- Message-ID
- <221104.86o7tnzehq.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20221104010242.11555-4-jacobabel@nullpo.dev>
On Fri, Nov 04 2022, Jacob Abel wrote:
> commit = lookup_commit_reference_by_name(refname); > - if (!commit) > +
Here.
Show 12 quoted lines
> + if (!commit && !opts->implicit)
> die(_("invalid reference: %s"), refname);
>
> name = worktree_basename(path, &len);
> @@ -482,10 +487,10 @@ static int add_worktree(const char *path, const char *refname,
> strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
> cp.git_cmd = 1;
>
> - if (!is_branch)
> + if (!is_branch && commit) {
> strvec_pushl(&cp.args, "update-ref", "HEAD",
> oid_to_hex(&commit->object.oid), NULL);And here we have a stray style change, in this case conforming to our CodingGuidelines (it's agnostic on the former), but IMO better to keep this out, or split it into a "various style stuff" commit, makes this harder to review...
Show 19 quoted lines
> - else {
> + } else {
> strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
> symref.buf, NULL);
> if (opts->quiet)
> @@ -516,7 +521,7 @@ static int add_worktree(const char *path, const char *refname,
> * Hook failure does not warrant worktree deletion, so run hook after
> * is_junk is cleared, but do return appropriate code when hook fails.
> */
> - if (!ret && opts->checkout) {
> + if (!ret && opts->checkout && !opts->orphan_branch) {
> struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
>
> strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
> @@ -608,33 +613,52 @@ static int add(int ac, const char **av, const char *prefix)
> const char *opt_track = NULL;
> const char *lock_reason = NULL;
> int keep_locked = 0;
> +ditto, we don't usually \n\n split up varibale decls.
Show 8 quoted lines
> struct option options[] = {
> - OPT__FORCE(&opts.force,
> - N_("checkout <branch> even if already checked out in other worktree"),
> - PARSE_OPT_NOCOMPLETE),
> + OPT__FORCE(
> + &opts.force,
> + N_("checkout <branch> even if already checked out in other worktree"),
> + PARSE_OPT_NOCOMPLETE),This is just a stray refactoring of existing code to not-our-usual-style (first arg is on the same line as the "(", rest aligned with "(").
Show 15 quoted lines
> OPT_STRING('b', NULL, &new_branch, N_("branch"),
> N_("create a new branch")),
> OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
> N_("create or reset a branch")),
> - OPT_BOOL('d', "detach", &opts.detach, N_("detach HEAD at named commit")),
> - OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
> - OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
> + OPT_STRING(0, "orphan", &opts.orphan_branch, N_("branch"),
> + N_("create a new unparented branch")),
> + OPT_BOOL('d', "detach", &opts.detach,
> + N_("detach HEAD at named commit")),
> + OPT_BOOL(0, "checkout", &opts.checkout,
> + N_("populate the new working tree")),
> + OPT_BOOL(0, "lock", &keep_locked,
> + N_("keep the new working tree locked")),Ditto, these look like they're too-long in the pre-image, but please resist re-flowing existing code while at it.
Show 11 quoted lines
> OPT_STRING(0, "reason", &lock_reason, N_("string"),
> N_("reason for locking")),
> OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
> OPT_PASSTHRU(0, "track", &opt_track, NULL,
> N_("set up tracking mode (see git-branch(1))"),
> PARSE_OPT_NOARG | PARSE_OPT_OPTARG),
> - OPT_BOOL(0, "guess-remote", &guess_remote,
> - N_("try to match the new branch name with a remote-tracking branch")),
> + OPT_BOOL(
> + 0, "guess-remote", &guess_remote,
> + N_("try to match the new branch name with a remote-tracking branch")),ditto.
Show 14 quoted lines
> OPT_END()
> };
>
> memset(&opts, 0, sizeof(opts));
> opts.checkout = 1;
> ac = parse_options(ac, av, prefix, options, git_worktree_add_usage, 0);
> - if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
> - die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
> +
> + opts.implicit = ac < 2;
> +
> + if (!!opts.detach + !!new_branch + !!new_branch_force +
> + !!opts.orphan_branch >
> + 1)The continued "if" is mis-indented, and that "1" is on a line of its own...