Re: [PATCH v6 4/4] worktree add: add hint to direct users towards --orphan

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Jacob

On 28/12/2022 06:17, Jacob Abel wrote:
> Adds a new advice/hint in `git worktree add` for when the user
> tries to create a new worktree from a reference that doesn't exist.

I was concerned that this description meant that

	git worktree add wt ref-that-does-not-exist

would print a hint rather than just error out but having tested it 
that's not the case. We only print the hint if the user does not give an 
explicit branch name. The implementation looks fine to me and the test 
looks sensible apart from save_param_arr which I think you're already 
planning to address.

Best Wishes

Phillip

> Current Behavior:
> 
> % git init --bare foo.git
> Initialized empty Git repository in /path/to/foo.git/
> % git -C foo.git worktree add main/
> Preparing worktree (new branch 'main')
> fatal: invalid reference: HEAD
> %
> 
> New Behavior:
> 
> % git init --bare foo.git
> Initialized empty Git repository in /path/to/foo.git/
> % git -C foo.git worktree add main/
> Preparing worktree (new branch 'main')
> hint: If you meant to create a worktree containing a new orphan branch
> hint: (branch with no commits) for this repository, you can do so
> hint: using the --orphan option:
> hint:
> hint:   git worktree add --orphan main ./main
> hint:
> hint: Disable this message with "git config advice.worktreeAddOrphan false"
> fatal: invalid reference: HEAD
> %
> 
> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>   Documentation/config/advice.txt |  4 ++++
>   advice.c                        |  1 +
>   advice.h                        |  1 +
>   builtin/worktree.c              |  6 ++++++
>   t/t2400-worktree-add.sh         | 17 +++++++++++++++++
>   5 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
> index a00d0100a8..3e58521613 100644
> --- a/Documentation/config/advice.txt
> +++ b/Documentation/config/advice.txt
> @@ -136,4 +136,8 @@ advice.*::
>   		Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1]
>   		is asked to update index entries outside the current sparse
>   		checkout.
> +	worktreeAddOrphan::
> +		Advice shown when a user tries to create a worktree from an
> +		invalid reference, to instruct how to create a new orphan
> +		branch instead.
>   --
> diff --git a/advice.c b/advice.c
> index fd18968943..53e91fdb85 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -75,6 +75,7 @@ static struct {
>   	[ADVICE_SUBMODULES_NOT_UPDATED] 		= { "submodulesNotUpdated", 1 },
>   	[ADVICE_UPDATE_SPARSE_PATH]			= { "updateSparsePath", 1 },
>   	[ADVICE_WAITING_FOR_EDITOR]			= { "waitingForEditor", 1 },
> +	[ADVICE_WORKTREE_ADD_ORPHAN]			= { "worktreeAddOrphan", 1 },
>   };
> 
>   static const char turn_off_instructions[] =
> diff --git a/advice.h b/advice.h
> index 07e0f76833..919d8c0064 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -50,6 +50,7 @@ struct string_list;
>   	ADVICE_UPDATE_SPARSE_PATH,
>   	ADVICE_WAITING_FOR_EDITOR,
>   	ADVICE_SKIPPED_CHERRY_PICKS,
> +	ADVICE_WORKTREE_ADD_ORPHAN,
>   };
> 
>   int git_default_advice_config(const char *var, const char *value);
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index ac82c5feda..d975628353 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -730,6 +730,12 @@ static int add(int ac, const char **av, const char *prefix)
>   	if (opts.orphan) {
>   		branch = new_branch;
>   	} else if (!lookup_commit_reference_by_name(branch)) {
> +		advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
> +			_("If you meant to create a worktree containing a new orphan branch\n"
> +			"(branch with no commits) for this repository, you can do so\n"
> +			"using the --orphan option:\n"
> +			"\n"
> +			"	git worktree add --orphan %s %s\n"), new_branch, path);
>   		die(_("invalid reference: %s"), branch);
>   	} else if (new_branch) {
>   		struct child_process cp = CHILD_PROCESS_INIT;
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index 43c95e6426..f43de59117 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -397,6 +397,23 @@ test_expect_success '"add" worktree with orphan branch, lock, and reason' '
>   	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
>   '
> 
> +test_wt_add_empty_repo_orphan_hint () {
> +	local context="$1"
> +	shift
> +	local arr=$(save_param_arr "$@")
> +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> +		eval "set -- $arr" &&
> +		test_when_finished "rm -rf empty_repo" &&
> +		GIT_DIR="empty_repo" git init --bare &&
> +		test_must_fail git -C empty_repo worktree add "$@" foobar/ 2> actual &&
> +		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
> +	'
> +}
> +
> +test_wt_add_empty_repo_orphan_hint 'DWIM'
> +test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch
> +test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch
> +
>   test_expect_success 'local clone from linked checkout' '
>   	git clone --local here here-clone &&
>   	( cd here-clone && git fsck )
> --
> 2.38.2
> 
>