From: Derrick Stolee <stolee@gmail.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 4/4] restore: add --intent-to-add (restoring worktree only)
Date: Thu, 20 Jun 2019 10:34:39 -0400 [thread overview]
Message-ID: <e232fbc4-06ec-d4ed-826a-3bcbc923cafe@gmail.com> (raw)
In-Reply-To: <20190620095523.10003-5-pclouds@gmail.com>
On 6/20/2019 5:55 AM, Nguyễn Thái Ngọc Duy wrote:
> "git restore --source" (without --staged) could create new files
> (i.e. not present in index) on worktree to match the given source. But
> the new files are not tracked, so both "git diff" and "git diff
> <source>" ignore new files. "git commit -a" will not recreate a commit
> exactly as the given source either.
>
> Add --intent-to-add to help track new files in this case, which is the
> default on the least surprise principle.
I was unfamiliar with this behavior, but did check the 'restore' command
myself and saw that it would register the file as untracked. I agree that
could be confusing for a user, so adding it to the staging environment
makes this more in-line with `git checkout <rev> -- <path>`.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> Documentation/git-restore.txt | 7 ++++
> builtin/checkout.c | 78 +++++++++++++++++++++++++++++++++++
> t/t2070-restore.sh | 17 ++++++++
> 3 files changed, 102 insertions(+)
>
> diff --git a/Documentation/git-restore.txt b/Documentation/git-restore.txt
> index d90093f195..43a7f43b2b 100644
> --- a/Documentation/git-restore.txt
> +++ b/Documentation/git-restore.txt
> @@ -93,6 +93,13 @@ in linkgit:git-checkout[1] for details.
> are "merge" (default) and "diff3" (in addition to what is
> shown by "merge" style, shows the original contents).
>
> +--intent-to-add::
> +--no-intent-to-add::
> + When restoring files only on working tree with `--source`,
> + new files are marked as "intent to add" (see
> + linkgit:git-add[1]). This is the default behavior. Use
> + `--no-intent-to-add` to disable it.
> +
> --ignore-unmerged::
> When restoring files on the working tree from the index, do
> not abort the operation if there are unmerged entries and
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index f884d27f1f..c519067d3d 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -70,6 +70,7 @@ struct checkout_opts {
> int checkout_worktree;
> const char *ignore_unmerged_opt;
> int ignore_unmerged;
> + int intent_to_add;
>
> const char *new_branch;
> const char *new_branch_force;
> @@ -392,6 +393,69 @@ static int checkout_worktree(const struct checkout_opts *opts)
> return errs;
> }
>
> +/*
> + * Input condition: r->index contains the file list matching worktree.
> + *
> + * r->index is reloaded with $GIT_DIR/index. Files that exist in the
> + * current worktree but not in $GIT_DIR/index are added back as
> + * intent-to-add.
> + */
Reading this code (and being unfamiliar with the cache array) I thought
it might accidentally add untracked files from the working directory into
the index. A local test verified that was not the case. Is that worth
adding to your test below?
> +test_expect_success 'restore worktree only adds new files back as intent-to-add' '
> + git init ita &&
> + (
> + cd ita &&
> + test_commit one &&
> + test_commit two &&
> + git rm one.t &&
> + git commit -m one-is-gone &&
+ touch garbage &&
> + git restore --source one one.t &&
> + git diff --summary >actual &&
> + echo " create mode 100644 one.t" >expected &&
> + test_cmp expected actual &&
> + git diff --cached >empty &&
> + test_must_be_empty empty
> + )
> +'
> +
> test_done
Perhaps the line I inserted above would suffice to add this extra check?
Outside of that extra test (which may not be necessary), this series makes
sense to me.
Thanks,
-Stolee
next prev parent reply other threads:[~2019-06-20 14:34 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-20 9:55 [PATCH 0/4] Some more on top of nd/switch-and-restore Nguyễn Thái Ngọc Duy
2019-06-20 9:55 ` [PATCH 1/4] t2027: use test_must_be_empty Nguyễn Thái Ngọc Duy
2019-06-20 9:55 ` [PATCH 2/4] switch: allow to switch in the middle of bisect Nguyễn Thái Ngọc Duy
2019-06-20 14:02 ` Derrick Stolee
2019-06-20 15:06 ` Duy Nguyen
2019-06-20 9:55 ` [PATCH 3/4] completion: disable dwim on "git switch -d" Nguyễn Thái Ngọc Duy
2019-06-20 9:55 ` [PATCH 4/4] restore: add --intent-to-add (restoring worktree only) Nguyễn Thái Ngọc Duy
2019-06-20 14:34 ` Derrick Stolee [this message]
2019-06-20 14:58 ` Duy Nguyen
2019-06-26 19:58 ` [PATCH 0/4] Some more on top of nd/switch-and-restore Junio C Hamano
2019-06-27 2:53 ` Duy Nguyen
2019-06-27 8:53 ` Duy Nguyen
2019-06-27 17:53 ` Junio C Hamano
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=e232fbc4-06ec-d4ed-826a-3bcbc923cafe@gmail.com \
--to=stolee@gmail.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.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).