Re: [PATCH] checkout: teach --worktree

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 14, 2020, 02:51 UTC
Message-ID: <CAPig+cQ_G+N=mYqO+=7UaAYft27MhZ2_3v8=QzYpDn5hQySp+A@mail.gmail.com>
In-Reply-To: <d10cb03dd8ce00cb6033c61ff9b9b30bbf5f9c89.1592058281.git.liu.denton@gmail.com>

On Sat, Jun 13, 2020 at 10:25 AM Denton Liu <liu.denton@gmail.com> wrote:

Show 13 quoted lines

> [...]
> Teach `git checkout --worktree`, allowing users to checkout files
> directly into the worktree without affecting the index.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> @@ -264,6 +266,12 @@ When switching branches with `--merge`, staged changes may be lost.
> +-W::
> +--worktree::
> +       When writing contents, only modify files in the worktree. Do not
> +       modify the index. This option is essentially a no-op when used
> +       without a `<tree-ish>`.

Why a no-op rather than actually diagnosing that --worktree makes no sense in that case and erroring out?

Show 9 quoted lines

> diff --git a/t/t2028-checkout-worktree.sh b/t/t2028-checkout-worktree.sh
> @@ -0,0 +1,51 @@
> +test_expect_success 'checkout --worktree on a commit' '
> +       test_when_finished "git reset --hard tip" &&
> +       git diff HEAD HEAD~ >expect &&
> +       git checkout --worktree HEAD~ file1 &&
> +       git diff >actual &&
> +       test_cmp expect actual &&
> +       git diff --cached --exit-code &&

Would the intent be clearer if you used 'test_expect_code' here?

    test_expect_code 0 git diff --cached --exit-code &&

Same question for remaining tests.