* [PATCH v2] git-apply.txt: update descriptions of --cached, --index
@ 2020-08-20 23:10 Raymond E. Pasco
2020-08-20 23:57 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Raymond E. Pasco @ 2020-08-20 23:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Raymond E. Pasco
The blurb for "--cached" says it implies "--index", but in reality
"--cached" and "--index" are distinct modes with different behavior.
Additionally, the descriptions of "--index" and "--cached" are somewhat
unclear about what might be modified, and what "--index" looks for to
determine that the index and working copy "match".
Rewrite the blurbs for both options for clarity and accuracy.
Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
---
How's this for an updated wording?
Documentation/git-apply.txt | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index b9aa39000f..91d9a8601c 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -61,18 +61,18 @@ OPTIONS
file and detects errors. Turns off "apply".
--index::
- When `--check` is in effect, or when applying the patch
- (which is the default when none of the options that
- disables it is in effect), make sure the patch is
- applicable to what the current index file records. If
- the file to be patched in the working tree is not
- up to date, it is flagged as an error. This flag also
- causes the index file to be updated.
+ Apply the patch to both the index and the working tree (or
+ merely check that it would apply cleanly to both if `--check` is
+ in effect). Note that `--index` expects index entries and
+ working tree copies for relevant paths to be identical (their
+ contents and metadata such as file mode must match), and will
+ raise an error if they are not, even if the patch would apply
+ cleanly to both the index and the working tree in isolation.
--cached::
- Apply a patch without touching the working tree. Instead take the
- cached data, apply the patch, and store the result in the index
- without using the working tree. This implies `--index`.
+ Apply the patch to just the index, without touching the working
+ tree. If `--check` is in effect, merely check that it would
+ apply cleanly to the index entry.
--intent-to-add::
When applying the patch only to the working tree, mark new
--
2.28.0.1.gcf60d27c7c
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] git-apply.txt: update descriptions of --cached, --index
2020-08-20 23:10 [PATCH v2] git-apply.txt: update descriptions of --cached, --index Raymond E. Pasco
@ 2020-08-20 23:57 ` Junio C Hamano
2020-08-21 0:26 ` Raymond E. Pasco
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2020-08-20 23:57 UTC (permalink / raw)
To: Raymond E. Pasco; +Cc: git, Phillip Wood
"Raymond E. Pasco" <ray@ameretat.dev> writes:
> The blurb for "--cached" says it implies "--index", but in reality
> "--cached" and "--index" are distinct modes with different behavior.
>
> Additionally, the descriptions of "--index" and "--cached" are somewhat
> unclear about what might be modified, and what "--index" looks for to
> determine that the index and working copy "match".
>
> Rewrite the blurbs for both options for clarity and accuracy.
>
> Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
> ---
> How's this for an updated wording?
s/blurbs?/description/
> Documentation/git-apply.txt | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index b9aa39000f..91d9a8601c 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -61,18 +61,18 @@ OPTIONS
> file and detects errors. Turns off "apply".
>
> --index::
> - When `--check` is in effect, or when applying the patch
> - (which is the default when none of the options that
> - disables it is in effect), make sure the patch is
> - applicable to what the current index file records. If
> - the file to be patched in the working tree is not
> - up to date, it is flagged as an error. This flag also
> - causes the index file to be updated.
> + Apply the patch to both the index and the working tree (or
> + merely check that it would apply cleanly to both if `--check` is
> + in effect). Note that `--index` expects index entries and
> + working tree copies for relevant paths to be identical (their
> + contents and metadata such as file mode must match), and will
> + raise an error if they are not, even if the patch would apply
> + cleanly to both the index and the working tree in isolation.
I do not see why we want to stress the last part after ", even if".
The safety mechanism insists on the working tree file and the index
entry to be identical, and the location where in the file the
difference is, is irrelevant, whether it is outside the area the
incoming patch touches, or it overlaps.
I however am OK if your thrust is to stress the fact that the paths
must be up to date. I think we can do so by making that the first
thing readers would read about the option, e.g.
After making sure the paths the patch touches in the working
tree are up to date (i.e. have no modifications relative to
their index entries), apply the patch both to the index
entries and to the working tree files (or see if it applies
cleanly, when `--check` is in effect).
> --cached::
> - Apply a patch without touching the working tree. Instead take the
> - cached data, apply the patch, and store the result in the index
> - without using the working tree. This implies `--index`.
> + Apply the patch to just the index, without touching the working
> + tree. If `--check` is in effect, merely check that it would
> + apply cleanly to the index entry.
This side looks good.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] git-apply.txt: update descriptions of --cached, --index
2020-08-20 23:57 ` Junio C Hamano
@ 2020-08-21 0:26 ` Raymond E. Pasco
2020-08-21 16:17 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Raymond E. Pasco @ 2020-08-21 0:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Phillip Wood
On Thu Aug 20, 2020 at 7:57 PM EDT, Junio C Hamano wrote:
> I do not see why we want to stress the last part after ", even if".
> The safety mechanism insists on the working tree file and the index
> entry to be identical, and the location where in the file the
> difference is, is irrelevant, whether it is outside the area the
> incoming patch touches, or it overlaps.
It's because this is the confusing part of the option - it's easy to
grasp "apply the patch to both the working copy and the index", but
that's not exactly what the option does, it applies only in the case of
identical preimages (and therefore, identical postimages). If you do
want to apply it to both the working copy and the index, which aren't
identical (e.g., you're a heavy worktree mangler and "add -p" user, like
me), this points you towards invoking it twice, once with no option and
once with "--cached".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] git-apply.txt: update descriptions of --cached, --index
2020-08-21 0:26 ` Raymond E. Pasco
@ 2020-08-21 16:17 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2020-08-21 16:17 UTC (permalink / raw)
To: Raymond E. Pasco; +Cc: git, Phillip Wood
"Raymond E. Pasco" <ray@ameretat.dev> writes:
> On Thu Aug 20, 2020 at 7:57 PM EDT, Junio C Hamano wrote:
>> I do not see why we want to stress the last part after ", even if".
>> The safety mechanism insists on the working tree file and the index
>> entry to be identical, and the location where in the file the
>> difference is, is irrelevant, whether it is outside the area the
>> incoming patch touches, or it overlaps.
>
> It's because this is the confusing part of the option - it's easy to
> grasp "apply the patch to both the working copy and the index", but
> that's not exactly what the option does, it applies only in the case of
> identical preimages (and therefore, identical postimages).
That's fair.
Back when "git apply" was introduced, workflows to create partial
commits with "edit file; git add file; edit file; git commit" did
exist, but the safety certainly far predates the more aggressive
form of partial commits created by "add -p", "checkout -p",
etc. (which is natural, as "add -p" and friends have to use "apply"
as their implementation detail). As "git apply --index" was created
primarily for preparing the index immediately followed by "git
commit" (as an implementation detail for "git applymbox", which was
"git am"'s precursor), it was one of the most obvious ways to avoid
the situation where _your_ work in progress in the working tree and
in the index gets mixed in the resulting commit made by applying
other's patch to insist that the index and the working tree contents
to match. As you suggest, of course, if the user deliberately wants
to keep the index and the working tree to be different (e.g. changes
in the working tree wrt the index are outside the block of text that
is touched by any incoming patch), it is easy to bypass the safety
feature by applying to the index and to the working tree separately,
or just apply to the working tree and run another "add -p".
Having said that, I still think the half-sentence after "even if"
was of little value. If we want to give the reason why, "even if
the patch may independently apply to the two, the two must be
identical" doesn't at all. It complains that the description does
not explain why the two must be identical without addressing the
complaint the sentence itself raises.
And if we are not going to give why that must be so, "the index and
the working tree file must be identical" is much clearer without the
"even if" part.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-21 17:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20 23:10 [PATCH v2] git-apply.txt: update descriptions of --cached, --index Raymond E. Pasco
2020-08-20 23:57 ` Junio C Hamano
2020-08-21 0:26 ` Raymond E. Pasco
2020-08-21 16:17 ` Junio C Hamano
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 NNTP newsgroup(s).