Re: [PATCH 3/4] worktree add: add --orphan flag

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Nov 04 2022, Jacob Abel wrote:

>  	commit = lookup_commit_reference_by_name(refname);
> -	if (!commit)
> +

Here.

> +	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...

> -	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.

>  	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 "(").

>  		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.

>  		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.

>  		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...