From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 16 Jan 2023 10:52:20 GMT
Subject: Re: [PATCH] worktree add: introduce basic DWYM for --orphan
Message-ID: <cddc6987-3b58-4688-65f8-3da0fbd1cc51@dunelm.org.uk>
In-Reply-To: <20230114224956.24801-1-jacobabel@nullpo.dev>

Hi Jacob

On 14/01/2023 22:50, Jacob Abel wrote:
> Introduces a DWYM shorthand of --orphan for when the worktree directory
> and the to-be-created branch share the same name.
> 
> Current Behavior:
>      % git worktree list
>      /path/to/git/repo        a38d39a4c5 [main]
>      % git worktree add --orphan new_branch ../new_branch/
>      Preparing worktree (new branch 'new_branch')
>      % git worktree add --orphan ../new_branch2/
>      usage: git worktree add [<options>] <path> [<commit-ish>]
>         or: git worktree list [<options>]
>      [...]
>      %
> 
> New Behavior:
> 
>      % git worktree list
>      /path/to/git/repo        a38d39a4c5 [main]
>      % git worktree add --orphan new_branch ../new_branch/
>      Preparing worktree (new branch 'new_branch')
>      % git worktree list
>      /path/to/git/repo        a38d39a4c5 [main]
>      /path/to/git/new_branch  a38d39a4c5 [new_branch]
>      % git worktree add --orphan ../new_branch2/
>      Preparing worktree (new branch 'new_branch2')
>      % git worktree list
>      /path/to/git/repo        a38d39a4c5 [main]
>      /path/to/git/new_branch  a38d39a4c5 [new_branch]
>      /path/to/git/new_branch2 a38d39a4c5 [new_branch2]
>      %

Thanks for working on this. As I said in my previous mail I think it 
would be easier to use OPT_BOOL() for --orphan from the start. By using 
OPT_STRING() you'll run into problems with "git worktree add --orphan 
--lock <directory>"

Best Wishes

Phillip

> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>   Documentation/git-worktree.txt | 13 +++++++++----
>   builtin/worktree.c             | 21 +++++++++++++++------
>   t/t2400-worktree-add.sh        | 10 ++++++++++
>   3 files changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index d78460c29c..a56ddb0185 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>   'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
>   		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
>   'git worktree add' [-f] [--lock [--reason <string>]]
> -		   --orphan <new-branch> <path>
> +		   --orphan [<new-branch>] <path>
>   'git worktree list' [-v | --porcelain [-z]]
>   'git worktree lock' [--reason <string>] <worktree>
>   'git worktree move' <worktree> <new-path>
> @@ -99,13 +99,16 @@ in the new worktree, if it's not checked out anywhere else, otherwise the
>   command will refuse to create the worktree (unless `--force` is used).
>   +
>   ------------
> -$ git worktree add --orphan <branch> <path>
> +$ git worktree add --orphan [<branch>] <path>
>   ------------
>   +
>   Create a worktree containing no files, with an empty index, and associated
>   with a new orphan branch named `<branch>`. The first commit made on this new
>   branch will have no parents and will be the root of a new history disconnected
>   from any other branches.
> ++
> +If a branch name `<branch>` is not supplied, the name is derived from the
> +supplied path `<path>`.
> 
>   list::
> 
> @@ -233,9 +236,11 @@ This can also be set up as the default behaviour by using the
>   	With `prune`, do not remove anything; just report what it would
>   	remove.
> 
> ---orphan <new-branch>::
> +--orphan [<new-branch>]::
>   	With `add`, make the new worktree and index empty, associating
> -	the worktree with a new orphan branch named `<new-branch>`.
> +	the worktree with a new orphan branch named `<new-branch>`. If
> +	`<new-branch>` is not supplied, the new branch name is derived
> +	from `<path>`.
> 
>   --porcelain::
>   	With `list`, output in an easy-to-parse format for scripts.
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index d975628353..481f895075 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -19,7 +19,7 @@
>   	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
>   	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]"), \
>   	N_("git worktree add [-f] [--lock [--reason <string>]]\n" \
> -	   "                 --orphan <new-branch> <path>")
> +	   "                 --orphan [<new-branch>] <path>")
> 
>   #define BUILTIN_WORKTREE_LIST_USAGE \
>   	N_("git worktree list [-v | --porcelain [-z]]")
> @@ -681,10 +681,13 @@ static int add(int ac, const char **av, const char *prefix)
>   	else if (keep_locked)
>   		opts.keep_locked = _("added with --lock");
> 
> -	if (ac < 1 || ac > 2)
> +	if (ac < 1 && opts.orphan) {
> +		path = prefix_filename(prefix, orphan_branch);
> +	} else if (ac >= 1 && ac <= 2) {
> +		path = prefix_filename(prefix, av[0]);
> +	} else {
>   		usage_with_options(git_worktree_add_usage, options);
> -
> -	path = prefix_filename(prefix, av[0]);
> +	}
>   	branch = ac < 2 ? "HEAD" : av[1];
> 
>   	if (!strcmp(branch, "-"))
> @@ -702,14 +705,20 @@ static int add(int ac, const char **av, const char *prefix)
>   		strbuf_release(&symref);
>   	}
> 
> -	if (opts.orphan) {
> -		new_branch = orphan_branch;
> +	if (ac < 1 && opts.orphan) {
> +		const char *s = dwim_branch(path, &orphan_branch);
> +		if (s)
> +			orphan_branch = s;
>   	} else if (ac < 2 && !new_branch && !opts.detach) {
>   		const char *s = dwim_branch(path, &new_branch);
>   		if (s)
>   			branch = s;
>   	}
> 
> +	if (opts.orphan) {
> +		new_branch = orphan_branch;
> +	}
> +
>   	if (ac == 2 && !new_branch && !opts.detach) {
>   		struct object_id oid;
>   		struct commit *commit;
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index 1bf8d619e2..c3de277738 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -354,6 +354,16 @@ test_expect_success '"add --orphan" fails if the branch already exists' '
>   	test_path_is_missing orphandir2
>   '
> 
> +test_expect_success '"add --orphan" with basic DWYM' '
> +	test_when_finished "rm -rf empty_repo" &&
> +	echo refs/heads/worktreedir >expected &&
> +	GIT_DIR="empty_repo" git init --bare &&
> +	# Use non-trivial path to verify it DWYMs properly.
> +	git -C empty_repo worktree add --orphan ../empty_repo/worktreedir &&
> +	git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
> +	test_cmp expected actual
> +'
> +
>   test_expect_success '"add --orphan" with empty repository' '
>   	test_when_finished "rm -rf empty_repo" &&
>   	echo refs/heads/newbranch >expected &&
> --
> 2.38.2
> 
>