{"thread":{"id":"54076","subject":"[PATCH v2] git-apply.txt: update descriptions of --cached, --index","startedAt":"2020-08-20T23:11:20Z","lastAt":"2020-08-21T17:08:40Z","messageCount":4,"participants":["Raymond E. Pasco","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"404113","messageId":"20200820231051.85134-1-ray@ameretat.dev","threadId":"54076","inReplyTo":null,"subject":"[PATCH v2] git-apply.txt: update descriptions of --cached, --index","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2020-08-20T23:10:51Z","receivedAt":"2020-08-20T23:11:20Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"The blurb for \"--cached\" says it implies \"--index\", but in reality\n\"--cached\" and \"--index\" are distinct modes with different behavior.\n\nAdditionally, the descriptions of \"--index\" and \"--cached\" are somewhat\nunclear about what might be modified, and what \"--index\" looks for to\ndetermine that the index and working copy \"match\".\n\nRewrite the blurbs for both options for clarity and accuracy.\n\nSigned-off-by: Raymond E. Pasco <ray@ameretat.dev>\n---\nHow's this for an updated wording?\n\n Documentation/git-apply.txt | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex b9aa39000f..91d9a8601c 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -61,18 +61,18 @@ OPTIONS\n \tfile and detects errors.  Turns off \"apply\".\n \n --index::\n-\tWhen `--check` is in effect, or when applying the patch\n-\t(which is the default when none of the options that\n-\tdisables it is in effect), make sure the patch is\n-\tapplicable to what the current index file records.  If\n-\tthe file to be patched in the working tree is not\n-\tup to date, it is flagged as an error.  This flag also\n-\tcauses the index file to be updated.\n+\tApply the patch to both the index and the working tree (or\n+\tmerely check that it would apply cleanly to both if `--check` is\n+\tin effect). Note that `--index` expects index entries and\n+\tworking tree copies for relevant paths to be identical (their\n+\tcontents and metadata such as file mode must match), and will\n+\traise an error if they are not, even if the patch would apply\n+\tcleanly to both the index and the working tree in isolation.\n \n --cached::\n-\tApply a patch without touching the working tree. Instead take the\n-\tcached data, apply the patch, and store the result in the index\n-\twithout using the working tree. This implies `--index`.\n+\tApply the patch to just the index, without touching the working\n+\ttree. If `--check` is in effect, merely check that it would\n+\tapply cleanly to the index entry.\n \n --intent-to-add::\n \tWhen applying the patch only to the working tree, mark new\n-- \n2.28.0.1.gcf60d27c7c\n\n"},{"id":"404115","messageId":"xmqq4kowc1ls.fsf@gitster.c.googlers.com","threadId":"54076","inReplyTo":"20200820231051.85134-1-ray@ameretat.dev","subject":"Re: [PATCH v2] git-apply.txt: update descriptions of --cached, --index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-20T23:57:19Z","receivedAt":"2020-08-20T23:57:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n\n> The blurb for \"--cached\" says it implies \"--index\", but in reality\n> \"--cached\" and \"--index\" are distinct modes with different behavior.\n>\n> Additionally, the descriptions of \"--index\" and \"--cached\" are somewhat\n> unclear about what might be modified, and what \"--index\" looks for to\n> determine that the index and working copy \"match\".\n>\n> Rewrite the blurbs for both options for clarity and accuracy.\n>\n> Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>\n> ---\n> How's this for an updated wording?\n\ns/blurbs?/description/\n\n>  Documentation/git-apply.txt | 20 ++++++++++----------\n>  1 file changed, 10 insertions(+), 10 deletions(-)\n>\n> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\n> index b9aa39000f..91d9a8601c 100644\n> --- a/Documentation/git-apply.txt\n> +++ b/Documentation/git-apply.txt\n> @@ -61,18 +61,18 @@ OPTIONS\n>  \tfile and detects errors.  Turns off \"apply\".\n>  \n>  --index::\n> -\tWhen `--check` is in effect, or when applying the patch\n> -\t(which is the default when none of the options that\n> -\tdisables it is in effect), make sure the patch is\n> -\tapplicable to what the current index file records.  If\n> -\tthe file to be patched in the working tree is not\n> -\tup to date, it is flagged as an error.  This flag also\n> -\tcauses the index file to be updated.\n> +\tApply the patch to both the index and the working tree (or\n> +\tmerely check that it would apply cleanly to both if `--check` is\n> +\tin effect). Note that `--index` expects index entries and\n> +\tworking tree copies for relevant paths to be identical (their\n> +\tcontents and metadata such as file mode must match), and will\n> +\traise an error if they are not, even if the patch would apply\n> +\tcleanly to both the index and the working tree in isolation.\n\nI do not see why we want to stress the last part after \", even if\".\nThe safety mechanism insists on the working tree file and the index\nentry to be identical, and the location where in the file the\ndifference is, is irrelevant, whether it is outside the area the\nincoming patch touches, or it overlaps.\n\nI however am OK if your thrust is to stress the fact that the paths\nmust be up to date.  I think we can do so by making that the first\nthing readers would read about the option, e.g.\n\n\tAfter making sure the paths the patch touches in the working\n\ttree are up to date (i.e. have no modifications relative to\n\ttheir index entries), apply the patch both to the index\n\tentries and to the working tree files (or see if it applies\n\tcleanly, when `--check` is in effect).\n\n>  --cached::\n> -\tApply a patch without touching the working tree. Instead take the\n> -\tcached data, apply the patch, and store the result in the index\n> -\twithout using the working tree. This implies `--index`.\n> +\tApply the patch to just the index, without touching the working\n> +\ttree. If `--check` is in effect, merely check that it would\n> +\tapply cleanly to the index entry.\n\nThis side looks good.\n\nThanks.\n"},{"id":"404117","messageId":"C528Y3DXYRMW.22FBYW4FHMALJ@ziyou.local","threadId":"54076","inReplyTo":"xmqq4kowc1ls.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] git-apply.txt: update descriptions of --cached, --index","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2020-08-21T00:26:38Z","receivedAt":"2020-08-21T00:57:34Z","isPatch":true,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On Thu Aug 20, 2020 at 7:57 PM EDT, Junio C Hamano wrote:\n> I do not see why we want to stress the last part after \", even if\".\n> The safety mechanism insists on the working tree file and the index\n> entry to be identical, and the location where in the file the\n> difference is, is irrelevant, whether it is outside the area the\n> incoming patch touches, or it overlaps.\n\nIt's because this is the confusing part of the option - it's easy to\ngrasp \"apply the patch to both the working copy and the index\", but\nthat's not exactly what the option does, it applies only in the case of\nidentical preimages (and therefore, identical postimages). If you do\nwant to apply it to both the working copy and the index, which aren't\nidentical (e.g., you're a heavy worktree mangler and \"add -p\" user, like\nme), this points you towards invoking it twice, once with no option and\nonce with \"--cached\".\n"},{"id":"404163","messageId":"xmqqv9hc9do1.fsf@gitster.c.googlers.com","threadId":"54076","inReplyTo":"C528Y3DXYRMW.22FBYW4FHMALJ@ziyou.local","subject":"Re: [PATCH v2] git-apply.txt: update descriptions of --cached, --index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-21T16:17:18Z","receivedAt":"2020-08-21T17:08:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n\n> On Thu Aug 20, 2020 at 7:57 PM EDT, Junio C Hamano wrote:\n>> I do not see why we want to stress the last part after \", even if\".\n>> The safety mechanism insists on the working tree file and the index\n>> entry to be identical, and the location where in the file the\n>> difference is, is irrelevant, whether it is outside the area the\n>> incoming patch touches, or it overlaps.\n>\n> It's because this is the confusing part of the option - it's easy to\n> grasp \"apply the patch to both the working copy and the index\", but\n> that's not exactly what the option does, it applies only in the case of\n> identical preimages (and therefore, identical postimages).\n\nThat's fair.  \n\nBack when \"git apply\" was introduced, workflows to create partial\ncommits with \"edit file; git add file; edit file; git commit\" did\nexist, but the safety certainly far predates the more aggressive\nform of partial commits created by \"add -p\", \"checkout -p\",\netc. (which is natural, as \"add -p\" and friends have to use \"apply\"\nas their implementation detail).  As \"git apply --index\" was created\nprimarily for preparing the index immediately followed by \"git\ncommit\" (as an implementation detail for \"git applymbox\", which was\n\"git am\"'s precursor), it was one of the most obvious ways to avoid\nthe situation where _your_ work in progress in the working tree and\nin the index gets mixed in the resulting commit made by applying\nother's patch to insist that the index and the working tree contents\nto match.  As you suggest, of course, if the user deliberately wants\nto keep the index and the working tree to be different (e.g. changes\nin the working tree wrt the index are outside the block of text that\nis touched by any incoming patch), it is easy to bypass the safety\nfeature by applying to the index and to the working tree separately,\nor just apply to the working tree and run another \"add -p\".\n\nHaving said that, I still think the half-sentence after \"even if\"\nwas of little value.  If we want to give the reason why, \"even if\nthe patch may independently apply to the two, the two must be\nidentical\" doesn't at all.  It complains that the description does\nnot explain why the two must be identical without addressing the\ncomplaint the sentence itself raises.\n\nAnd if we are not going to give why that must be so, \"the index and\nthe working tree file must be identical\" is much clearer without the\n\"even if\" part.\n"}]}