* [PATCH] apply: clarify description of --index
@ 2020-10-23 3:49 Junio C Hamano
2020-10-23 5:31 ` Johannes Sixt
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2020-10-23 3:49 UTC (permalink / raw)
To: git
Instead of explaining the requirement for the paths to be up-to-date,
as if it is an afterthought, state it upfront.
The updated description matches how the checks actually are
performed. A path that is "dirty" stops the patch application from
being attempted to either working tree files or to the index.
Hopefully this change would help users to form a better mental
model.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Just noticed while reviewing how "apply" (and "am") are explained.
Documentation/git-apply.txt | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 91d9a8601c..1be7751f58 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -61,13 +61,11 @@ OPTIONS
file and detects errors. Turns off "apply".
--index::
- 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.
+ After making sure the paths the patch touches in the working
+ tree 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 the patch to just the index, without touching the working
--
2.28.0-462-gf84ddd074d
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] apply: clarify description of --index
2020-10-23 3:49 [PATCH] apply: clarify description of --index Junio C Hamano
@ 2020-10-23 5:31 ` Johannes Sixt
2020-10-23 14:38 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Sixt @ 2020-10-23 5:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Am 23.10.20 um 05:49 schrieb Junio C Hamano:
> Instead of explaining the requirement for the paths to be up-to-date,
> as if it is an afterthought, state it upfront.
>
> The updated description matches how the checks actually are
> performed. A path that is "dirty" stops the patch application from
> being attempted to either working tree files or to the index.
>
> Hopefully this change would help users to form a better mental
> model.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> * Just noticed while reviewing how "apply" (and "am") are explained.
>
> Documentation/git-apply.txt | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index 91d9a8601c..1be7751f58 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -61,13 +61,11 @@ OPTIONS
> file and detects errors. Turns off "apply".
>
> --index::
> - 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.
> + After making sure the paths the patch touches in the working
> + tree 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.
I don't think that this is an improvement. The purpose of --index *is*
to apply the patch to both index and worktree, and that should be
mentioned first. The check that both are identical, is a prerequisite
and not the primary objective of the option.
>
> --cached::
> Apply the patch to just the index, without touching the working
>
-- Hannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] apply: clarify description of --index
2020-10-23 5:31 ` Johannes Sixt
@ 2020-10-23 14:38 ` Junio C Hamano
2020-10-23 18:08 ` Johannes Sixt
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2020-10-23 14:38 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
Johannes Sixt <j6t@kdbg.org> writes:
>> - 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.
>> + After making sure the paths the patch touches in the working
>> + tree 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.
>
> I don't think that this is an improvement. The purpose of --index *is*
> to apply the patch to both index and worktree, and that should be
> mentioned first. The check that both are identical, is a prerequisite
> and not the primary objective of the option.
Yeah, but this was an attempt to clarify what that "apply to both",
which is the central part of the operation, exactly means.
The only mode of operation we offer is that we start from identical
index and working tree, and make the same change so that we arrive
at the same outcome. It is not like you can have some changes in
the working tree file as long as they do not overlap and collide
with the incoming patch, make the same change with the patch to
arrive at different contents as the outcome. We explicitly forbid
that, but "apply to both" does not exactly tell it to the readers.
But I am OK to drop this, if you do not think it clarifies the
description.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] apply: clarify description of --index
2020-10-23 14:38 ` Junio C Hamano
@ 2020-10-23 18:08 ` Johannes Sixt
2020-10-23 20:18 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Sixt @ 2020-10-23 18:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Am 23.10.20 um 16:38 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>>> - 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.
>>> + After making sure the paths the patch touches in the working
>>> + tree 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.
>>
>> I don't think that this is an improvement. The purpose of --index *is*
>> to apply the patch to both index and worktree, and that should be
>> mentioned first. The check that both are identical, is a prerequisite
>> and not the primary objective of the option.
>
> Yeah, but this was an attempt to clarify what that "apply to both",
> which is the central part of the operation, exactly means.
>
> The only mode of operation we offer is that we start from identical
> index and working tree, and make the same change so that we arrive
> at the same outcome. It is not like you can have some changes in
> the working tree file as long as they do not overlap and collide
> with the incoming patch, make the same change with the patch to
> arrive at different contents as the outcome. We explicitly forbid
> that, but "apply to both" does not exactly tell it to the readers.
Your have point that the original text muddies the preconditions a bit,
but I still think that "what it does" must be the first thing to be
mentioned, and the preconditions the second.
-- Hannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] apply: clarify description of --index
2020-10-23 18:08 ` Johannes Sixt
@ 2020-10-23 20:18 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2020-10-23 20:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
Johannes Sixt <j6t@kdbg.org> writes:
> Am 23.10.20 um 16:38 schrieb Junio C Hamano:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>>> - 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.
>>>> + After making sure the paths the patch touches in the working
>>>> + tree 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.
>>>
>>> I don't think that this is an improvement. The purpose of --index *is*
>>> to apply the patch to both index and worktree, and that should be
>>> mentioned first. The check that both are identical, is a prerequisite
>>> and not the primary objective of the option.
>>
>> Yeah, but this was an attempt to clarify what that "apply to both",
>> which is the central part of the operation, exactly means.
>>
>> The only mode of operation we offer is that we start from identical
>> index and working tree, and make the same change so that we arrive
>> at the same outcome. It is not like you can have some changes in
>> the working tree file as long as they do not overlap and collide
>> with the incoming patch, make the same change with the patch to
>> arrive at different contents as the outcome. We explicitly forbid
>> that, but "apply to both" does not exactly tell it to the readers.
>
> Your have point that the original text muddies the preconditions a bit,
> but I still think that "what it does" must be the first thing to be
> mentioned, and the preconditions the second.
Yeah. I offhand do not think of a better phrasing within the
constraint that "apply to only identical state" must be said after
saying "to both the index and the working tree", though, so I'll
leave it up to the list's wisdom ;-)
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-23 20:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 3:49 [PATCH] apply: clarify description of --index Junio C Hamano
2020-10-23 5:31 ` Johannes Sixt
2020-10-23 14:38 ` Junio C Hamano
2020-10-23 18:08 ` Johannes Sixt
2020-10-23 20:18 ` 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).