[PATCH] worktree add: introduce basic DWYM for --orphan

[PATCH] worktree add: introduce basic DWYM for --orphan

[Jacob Abel]

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]
    %

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

Re: [PATCH] worktree add: introduce basic DWYM for --orphan

[Jacob Abel]

On 23/01/14 10:50PM, Jacob Abel wrote:
>
> [...]

Oops for some reason this didn't show up as a reply on the lore. This is
supposed to be a reply to the following email chain:

https://lore.kernel.org/git/20230114224715.ewec6sz5h3q3iijs@phi/

Re: [PATCH] worktree add: introduce basic DWYM for --orphan

[Phillip Wood]

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

Re: [PATCH] worktree add: introduce basic DWYM for --orphan

[Jacob Abel]

On 23/01/16 10:52AM, Phillip Wood wrote:
> 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
>
> > [...]

Ah, good point. I missed that.

Also given the way the conversation is going, I'll drop this patch and integrate
the changes into the patches of the main series since I'll be re-rolling.