From: Phillip Wood <phillip.wood123@gmail.com>
To: Denton Liu <liu.denton@gmail.com>,
Git Mailing List <git@vger.kernel.org>
Cc: Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH v2] checkout: teach --worktree
Date: Tue, 16 Jun 2020 17:01:53 +0100 [thread overview]
Message-ID: <3c223d33-7f3f-7f4c-df91-3b72271308cd@gmail.com> (raw)
In-Reply-To: <2a434d328429907a304ad6aaad4c1d9261d27831.1592124367.git.liu.denton@gmail.com>
Hi Denton
On 14/06/2020 09:49, Denton Liu wrote:
> A complaint that has come up frequently in the past is that it is not
> possible to checkout files directly into a worktree without modifying
> the index. Even though this could be worked around by redirecting the
> output of `git show` to overwrite files, this was not feasible if one
> wanted to use patch mode.
>
> Since `git restore` was implemented, this has since been possible using
> the `--worktree` option. However, some long-time users of Git still
> prefer to use `git checkout` over `git restore` and would like to see
> the functionality ported over.
>
> Teach `git checkout --worktree`, allowing users to checkout files
> directly into the worktree without affecting the index.
I'm afraid I'm not sure that adding another option to `git checkout` is
a good idea. The behavior of `git checkout` is already complicated
enough which is why we ended up with switch and restore separating out
branch switching from file updating.
Given `git restore` provides a way to update the worktree without
touching the index I'm not convinced we should be further complicating
`git checkout` especially as it defaults to --overlay unless -p is given
which is confusing in itself.
Best Wishes
Phillip
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> I realised that making `git checkout --worktree` without a pathspec to
> check out the current directory doesn't really make sense so I didn't
> make that change.
>
> Range-diff against v1:
> 1: d10cb03dd8 ! 1: 2a434d3284 checkout: teach --worktree
> @@ Documentation/git-checkout.txt: When switching branches with `--merge`, staged c
> +-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>`.
> ++ modify the index. This option does not make sense without a
> ++ `<pathspec>`.
> +
> -p::
> --patch::
> @@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const
> +
> + opts->checkout_index = 0;
> + opts->checkout_worktree = 1;
> ++ opts->empty_pathspec_ok = 0;
> +
> + return 0;
> +}
> @@ builtin/checkout.c: int cmd_checkout(int argc, const char **argv, const char *pr
> OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"),
> N_("create/reset and checkout a branch")),
> + OPT_CALLBACK_F('W', "worktree", &opts, NULL,
> -+ N_("restore the working tree (default)"),
> ++ N_("restore the working tree"),
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, handle_worktree_opt),
> OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
> OPT_BOOL(0, "guess", &opts.dwim_new_local_branch,
> @@ t/t2028-checkout-worktree.sh (new)
> + test_cmp_rev HEAD tip
> +'
> +
> ++test_expect_success 'checkout --worktree without pathspec fails' '
> ++ test_must_fail git checkout --worktree
> ++'
> ++
> +test_expect_success 'checkout --no-worktree fails' '
> + test_must_fail git checkout --no-worktree
> +'
>
> Documentation/git-checkout.txt | 22 +++++++++-----
> builtin/checkout.c | 18 +++++++++++
> t/t2028-checkout-worktree.sh | 55 ++++++++++++++++++++++++++++++++++
> t/t9902-completion.sh | 1 +
> 4 files changed, 89 insertions(+), 7 deletions(-)
> create mode 100755 t/t2028-checkout-worktree.sh
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index 5b697eee1b..c303839920 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -12,9 +12,9 @@ SYNOPSIS
> 'git checkout' [-q] [-f] [-m] --detach [<branch>]
> 'git checkout' [-q] [-f] [-m] [--detach] <commit>
> 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
> -'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <pathspec>...
> -'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]
> -'git checkout' (-p|--patch) [<tree-ish>] [--] [<pathspec>...]
> +'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [--worktree] [<tree-ish>] [--] <pathspec>...
> +'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [--worktree] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]
> +'git checkout' (-p|--patch) [--worktree] [<tree-ish>] [--] [<pathspec>...]
>
> DESCRIPTION
> -----------
> @@ -79,14 +79,16 @@ be used to detach `HEAD` at the tip of the branch (`git checkout
> +
> Omitting `<branch>` detaches `HEAD` at the tip of the current branch.
>
> -'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <pathspec>...::
> -'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]::
> +'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [--worktree] [<tree-ish>] [--] <pathspec>...::
> +'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [--worktree] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]::
>
> Overwrite the contents of the files that match the pathspec.
> When the `<tree-ish>` (most often a commit) is not given,
> overwrite working tree with the contents in the index.
> When the `<tree-ish>` is given, overwrite both the index and
> - the working tree with the contents at the `<tree-ish>`.
> + the working tree with the contents at the `<tree-ish>` unless
> + `--worktree` is given in which case _only_ the working tree is
> + overwritten.
> +
> The index may contain unmerged entries because of a previous failed merge.
> By default, if you try to check out such an entry from the index, the
> @@ -96,7 +98,7 @@ specific side of the merge can be checked out of the index by
> using `--ours` or `--theirs`. With `-m`, changes made to the working tree
> file can be discarded to re-create the original conflicted merge result.
>
> -'git checkout' (-p|--patch) [<tree-ish>] [--] [<pathspec>...]::
> +'git checkout' (-p|--patch) [--worktree] [<tree-ish>] [--] [<pathspec>...]::
> This is similar to the previous mode, but lets you use the
> interactive interface to show the "diff" output and choose which
> hunks to use in the result. See below for the description of
> @@ -264,6 +266,12 @@ When switching branches with `--merge`, staged changes may be lost.
> "merge" (default) and "diff3" (in addition to what is shown by
> "merge" style, shows the original contents).
>
> +-W::
> +--worktree::
> + When writing contents, only modify files in the worktree. Do not
> + modify the index. This option does not make sense without a
> + `<pathspec>`.
> +
> -p::
> --patch::
> Interactively select hunks in the difference between the
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index af849c644f..8c533a305b 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1741,6 +1741,21 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
> return checkout_branch(opts, &new_branch_info);
> }
>
> +static int handle_worktree_opt(const struct option *opt, const char *arg, int unset)
> +{
> + struct checkout_opts *opts = opt->value;
> +
> + BUG_ON_OPT_NEG(unset);
> + BUG_ON_OPT_ARG(arg);
> +
> + opts->checkout_index = 0;
> + opts->checkout_worktree = 1;
> + opts->empty_pathspec_ok = 0;
> +
> + return 0;
> +}
> +
> +
> int cmd_checkout(int argc, const char **argv, const char *prefix)
> {
> struct checkout_opts opts;
> @@ -1750,6 +1765,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> N_("create and checkout a new branch")),
> OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"),
> N_("create/reset and checkout a branch")),
> + OPT_CALLBACK_F('W', "worktree", &opts, NULL,
> + N_("restore the working tree"),
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, handle_worktree_opt),
> OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
> OPT_BOOL(0, "guess", &opts.dwim_new_local_branch,
> N_("second guess 'git checkout <no-such-branch>' (default)")),
> diff --git a/t/t2028-checkout-worktree.sh b/t/t2028-checkout-worktree.sh
> new file mode 100755
> index 0000000000..b5b5e287c1
> --- /dev/null
> +++ b/t/t2028-checkout-worktree.sh
> @@ -0,0 +1,55 @@
> +#!/bin/sh
> +
> +test_description='checkout --worktree'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> + echo first >file1 &&
> + echo file2 >file2 &&
> + git add file1 file2 &&
> + git commit -m first &&
> +
> + echo second >file1 &&
> + git commit -am second &&
> + git tag tip
> +'
> +
> +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 &&
> + test_cmp_rev HEAD tip
> +'
> +
> +test_expect_success 'checkout --worktree with no commit' '
> + test_when_finished "git reset --hard tip" &&
> + echo worktree >file1 &&
> + git checkout --worktree file1 &&
> + git diff --exit-code &&
> + test_cmp_rev HEAD tip
> +'
> +
> +test_expect_success 'checkout --worktree without pathspec fails' '
> + test_must_fail git checkout --worktree
> +'
> +
> +test_expect_success 'checkout --no-worktree fails' '
> + test_must_fail git checkout --no-worktree
> +'
> +
> +test_expect_success PERL 'git checkout -p --worktree' '
> + test_when_finished "git reset --hard tip" &&
> + echo changed >file2 &&
> + git diff -R --src-prefix=b/ --dst-prefix=a/ >expect &&
> + git commit -am file12 &&
> + test_write_lines n y | git checkout --worktree -p HEAD~2 &&
> + git diff >actual &&
> + test_cmp expect actual &&
> + git diff --cached --exit-code
> +'
> +
> +test_done
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 3c44af6940..1db0bb3a31 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1481,6 +1481,7 @@ test_expect_success 'double dash "git checkout"' '
> --quiet Z
> --detach Z
> --track Z
> + --worktree Z
> --orphan=Z
> --ours Z
> --theirs Z
>
next prev parent reply other threads:[~2020-06-16 16:02 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-13 14:25 [PATCH] checkout: teach --worktree Denton Liu
2020-06-14 2:51 ` Eric Sunshine
2020-06-14 7:44 ` Denton Liu
2020-06-14 8:02 ` Denton Liu
2020-06-14 21:37 ` Junio C Hamano
2020-06-14 8:49 ` [PATCH v2] " Denton Liu
2020-06-16 16:01 ` Phillip Wood [this message]
2020-06-23 15:55 ` Denton Liu
2020-06-24 10:20 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3c223d33-7f3f-7f4c-df91-3b72271308cd@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=git@vger.kernel.org \
--cc=liu.denton@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).