{"thread":{"id":"21242","subject":"[PATCH] git add -e documentation: rephrase note","startedAt":"2009-10-14T22:26:28Z","lastAt":"2009-10-20T00:39:57Z","messageCount":10,"participants":["Miklos Vajna","Jeff King","Junio C Hamano","Wesley J. Landaker"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"125028","messageId":"20091014222628.GK6115@genesis.frugalware.org","threadId":"21242","inReplyTo":null,"subject":"[PATCH] git add -e documentation: rephrase note","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-10-14T22:26:28Z","receivedAt":"2009-10-14T22:26:28Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"The original note probably wanted to suggest \"edit the patch with care\",\nbut actually suggested just editing the first characters on certain\nlines, which is a pretty bad suggestion.\n\nSigned-off-by: Miklos Vajna \n---\n Documentation/git-add.txt | 5 ++---\n 1 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 45ebf87..daa1add 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -86,9 +86,8 @@ OPTIONS\n \tedit it. After the editor was closed, adjust the hunk headers\n \tand apply the patch to the index.\n +\n-*NOTE*: Obviously, if you change anything else than the first character\n-on lines beginning with a space or a minus, the patch will no longer\n-apply.\n+*NOTE*: Obviously, if you change the first characters of the lines or\n+lines beginning with a space or a minus, the patch will no longer apply.\n \n -u::\n --update::\n-- \n1.6.4\n"},{"id":"125033","messageId":"20091014230434.GB29664@coredump.intra.peff.net","threadId":"21242","inReplyTo":"20091014222628.GK6115@genesis.frugalware.org","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-14T23:04:34Z","receivedAt":"2009-10-14T23:04:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 15, 2009 at 12:26:28AM +0200, Miklos Vajna wrote:\n\n> The original note probably wanted to suggest \"edit the patch with care\",\n> but actually suggested just editing the first characters on certain\n> lines, which is a pretty bad suggestion.\n\nThe intent of \"-e\" is to pick and choose lines of the diff to apply, or\neven to munge the lines. So it is safe to:\n\n - remove lines with a '+' (don't stage the addition)\n\n - munge any lines with a '+' (stage modified contents)\n\n - add lines with a '+' (stage an addition)\n\n - convert lines with a ' ' to '-' (stage removal)\n\n - convert lines with a '-' to ' ' (don't stage removal)\n\nIt is a bad idea to:\n\n - delete '-' lines\n\n - delete ' ' lines\n\n - add ' ' or '-' line\n\n - munge the contents of ' ' or '-' lines\n\nBut that is just off the top of my head; perhaps others can come up with\ncorner cases where one of the safe things breaks.\n\n> -*NOTE*: Obviously, if you change anything else than the first character\n> -on lines beginning with a space or a minus, the patch will no longer\n> -apply.\n> +*NOTE*: Obviously, if you change the first characters of the lines or\n> +lines beginning with a space or a minus, the patch will no longer apply.\n\nSo this is warning about changing or deleting ' ' or '-' lines, which is\ngood. But you can change the first characters of those lines, but only\nby interchanging the two. So I think both the previous text and your\ntext don't cover all cases.\n\nMaybe it makes sense instead to do a quick overview of what is OK and\nwhat is not (like the list above).\n\n-Peff\n"},{"id":"125311","messageId":"20091019000900.GV6115@genesis.frugalware.org","threadId":"21242","inReplyTo":"20091014230434.GB29664@coredump.intra.peff.net","subject":"[PATCH] git add -e documentation: rephrase note","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-10-19T00:09:00Z","receivedAt":"2009-10-19T00:09:00Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Add a quick overview about what is OK and what is not to cover all\ncases.\n\nSigned-off-by: Miklos Vajna \nSigned-off-by: Jeff King \n---\n\nOn Wed, Oct 14, 2009 at 07:04:34PM -0400, Jeff King wrote:\n> Maybe it makes sense instead to do a quick overview of what is OK and\n> what is not (like the list above).\n\nHere it is.\n\n Documentation/git-add.txt | 18 +++++++++++++++---\n 1 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 45ebf87..1ea074c 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -86,9 +86,21 @@ OPTIONS\n \tedit it. After the editor was closed, adjust the hunk headers\n \tand apply the patch to the index.\n +\n-*NOTE*: Obviously, if you change anything else than the first character\n-on lines beginning with a space or a minus, the patch will no longer\n-apply.\n+The intent of this option is to pick and choose lines of the diff to\n+apply, or even to munge the lines. So it is safe to:\n++\n+* remove lines with a \"+\" (don't stage the addition)\n+* munge any lines with a \"+\" (stage modified contents)\n+* add lines with a \"+\" (stage an addition)\n+* convert lines with a \" \" to \"-\" (stage removal)\n+* convert lines with a \"-\" to \" \" (don't stage removal)\n++\n+It is a bad idea to:\n++\n+* delete \"-\" lines\n+* delete \" \" lines\n+* add \" \" or \"-\" lines\n+* munge the contents of \" \" or \"-\" lines\n \n -u::\n --update::\n-- \n1.6.5\n"},{"id":"125329","messageId":"20091019043418.GD7170@coredump.intra.peff.net","threadId":"21242","inReplyTo":"20091019000900.GV6115@genesis.frugalware.org","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-19T04:34:18Z","receivedAt":"2009-10-19T04:34:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 19, 2009 at 02:09:00AM +0200, Miklos Vajna wrote:\n\n> Add a quick overview about what is OK and what is not to cover all\n> cases.\n> \n> Signed-off-by: Miklos Vajna \n> Signed-off-by: Jeff King \n\nPlease be careful with sign-offs. While your patch text is an adaptation\nof what I said, and while it is true that I personally would never say\nanything on the list that was not free for use in git, I did not in fact\ndo such a sign-off, which is what that line is supposed to be\ndocumenting.\n\nIn this case, I think you probably just wanted to say \"text from Jeff\nKing\" or even \"From: Jeff King \".\n\nThat being said,\n\n Signed-off-by: Jeff King \n\n:)\n\nI still have a few comments, though.\n\n> +The intent of this option is to pick and choose lines of the diff to\n> +apply, or even to munge the lines. So it is safe to:\n> ++\n> +* remove lines with a \"+\" (don't stage the addition)\n> +* munge any lines with a \"+\" (stage modified contents)\n\nDo we want to use \"munge\" here? Maybe something a little more specific\nlike \"modify the content of\" is more appropriate for documentation.\n\n> +* add lines with a \"+\" (stage an addition)\n> +* convert lines with a \" \" to \"-\" (stage removal)\n> +* convert lines with a \"-\" to \" \" (don't stage removal)\n\nIs \" \" going to render in an obvious-to-read way? I checked the html\nversion and the manpage version in my terminal, and they looked OK, but\nI would not be surprised if somebody has a font setup that makes it a\nlittle hard to read.\n\n> ++\n> +It is a bad idea to:\n> ++\n\nAnnoyingly, this renders for me as:\n\n · convert lines with a \"-\" to \" \" (don’t stage removal)\n\n It is a bad idea to:\n\n · delete \"-\" lines\n\n(the spacing of which will hopefully not be destroyed in transit). In\nother words, the \"+\" list continuation makes the \"it is a bad idea\" text\npart of the \"good idea\" list, instead of continuing the definition of\nthe \"-e\" option. I think we can fix it with an open block marker. I'll\nsee what I can do.\n\n-Peff\n"},{"id":"125330","messageId":"20091019050456.GA15706@coredump.intra.peff.net","threadId":"21242","inReplyTo":"20091019043418.GD7170@coredump.intra.peff.net","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-19T05:04:56Z","receivedAt":"2009-10-19T05:04:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 19, 2009 at 12:34:18AM -0400, Jeff King wrote:\n\n> I still have a few comments, though.\n> [...]\n> (the spacing of which will hopefully not be destroyed in transit). In\n> other words, the \"+\" list continuation makes the \"it is a bad idea\" text\n> part of the \"good idea\" list, instead of continuing the definition of\n> the \"-e\" option. I think we can fix it with an open block marker. I'll\n> see what I can do.\n\nHmph. Here is my attempt. The text is (I hope) more clear, but I am\nstill having trouble with the formatting. It looks fine in the HTML\nversion, and if I am reading the XML right, the XML is correct. But\ndocbook seems to screw up converting the XML to roff, giving this:\n\n · convert removal lines to context lines (don’t stage removal)\n Similarly, your patch will likely not apply if you:\n\n · add context or removal lines\n\nAm I missing something, or is it really a docbook bug? Does it render\nbetter for anybody else?\n\n-- >8 --\nSubject: [PATCH] docs: give more hints about how \"add -e\" works\n\nThe original text was not very descriptive about what you\ncan and can't do; let's try to enumerate all cases.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-add.txt | 22 +++++++++++++++++++---\n 1 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 45ebf87..b0a8420 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -86,9 +86,25 @@ OPTIONS\n \tedit it. After the editor was closed, adjust the hunk headers\n \tand apply the patch to the index.\n +\n-*NOTE*: Obviously, if you change anything else than the first character\n-on lines beginning with a space or a minus, the patch will no longer\n-apply.\n+The intent of this option is to pick and choose lines of the diff to\n+apply, or even to modify the contents of lines to be staged. There are\n+three line types in a diff: addition lines (beginning with a plus),\n+removal lines (beginning with a minus), and context lines (beginning\n+with a space). In general, it should be safe to:\n++\n+--\n+* remove addition lines (don't stage the line)\n+* modify the content of any addition lines (stage modified contents)\n+* add new addition lines (stage the new line)\n+* convert context lines to removal lines (stage removal of line)\n+* convert removal lines to context lines (don't stage removal)\n+--\n++\n+Similarly, your patch will likely not apply if you:\n++\n+* add context or removal lines\n+* delete removal or context lines\n+* modify the contents of context or removal lines\n \n -u::\n --update::\n-- \n1.6.5.1.123.gcaaf\n"},{"id":"125332","messageId":"7vbpk4aqop.fsf@alter.siamese.dyndns.org","threadId":"21242","inReplyTo":"20091019050456.GA15706@coredump.intra.peff.net","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-19T05:38:46Z","receivedAt":"2009-10-19T05:38:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Hmph. Here is my attempt. The text is (I hope) more clear, but I am\n> still having trouble with the formatting. It looks fine in the HTML\n> version, and if I am reading the XML right, the XML is correct. But\n> docbook seems to screw up converting the XML to roff, giving this:\n>\n> · convert removal lines to context lines (don’t stage removal)\n> Similarly, your patch will likely not apply if you:\n>\n> · add context or removal lines\n>\n> Am I missing something, or is it really a docbook bug? Does it render\n> better for anybody else?\n\nNot for me, and I have seen manpages lacking a blank line like the above\nwhere I expect one in other places, so it is not very surprising.\n\n> -- >8 --\n> Subject: [PATCH] docs: give more hints about how \"add -e\" works\n>\n> The original text was not very descriptive about what you\n> can and can't do; let's try to enumerate all cases.\n>\n> Signed-off-by: Jeff King \n> ---\n> Documentation/git-add.txt | 22 +++++++++++++++++++---\n> 1 files changed, 19 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\n> index 45ebf87..b0a8420 100644\n> --- a/Documentation/git-add.txt\n> +++ b/Documentation/git-add.txt\n> @@ -86,9 +86,25 @@ OPTIONS\n> \tedit it. After the editor was closed, adjust the hunk headers\n> \tand apply the patch to the index.\n> +\n> -*NOTE*: Obviously, if you change anything else than the first character\n> -on lines beginning with a space or a minus, the patch will no longer\n> -apply.\n> +The intent of this option is to pick and choose lines of the diff to\n\nI'd slightly prefer \"patch\" over \"diff\" in this context.\n\n> +apply, or even to modify the contents of lines to be staged. There are\n> +three line types in a diff: addition lines (beginning with a plus),\n> +removal lines (beginning with a minus), and context lines (beginning\n> +with a space). In general, it should be safe to:\n> ++\n> +--\n> +* remove addition lines (don't stage the line)\n\nThis is more like \"don't add the line\", isn't it? Also if this \"+\" line\nhas corresponding \"-\" line (i.e. it is a \"rewrite to this\" line), removal\nof such a line would mean \"instead of rewriting, remove it\".\n\n> +* modify the content of any addition lines (stage modified contents)\n\nWould be \"add differently\".\n\n> +* add new addition lines (stage the new line)\n\n\"Add more than what you have in the work tree\"\n\nWhile kibitzing like the above, I noticed that there is another thing that\nmay deserve warning even more. Suppose you added lines and the patch\nbetween HEAD and the work tree looks like this:\n\n diff --git a/t-f b/t-f\n index e69de29..d68dd40 100644\n --- a/t-f\n +++ b/t-f\n @@ -0,0 +1,4 @@\n +a\n +b\n +c\n +d\n\nAnd you do \"add -e\" and edit the patch to:\n\n diff --git a/t-f b/t-f\n index e69de29..17c3f0b 100644\n --- a/t-f\n +++ b/t-f\n @@ -0,0 +1,3 @@\n +a\n +e\n +d\n\nObviously, the above patch is what \"diff --cached\" would show after such\nan \"add -e\", but this does _not_ touch anything in the work tree (which is\ncorrect; add is about moving changed contents to the index and it should\nfundamentally not affect work tree). As a result, \"diff\" between the\nindex and the work tree now would read like this:\n\n diff --git a/t-f b/t-f\n index 17c3f0b..d68dd40 100644\n --- a/t-f\n +++ b/t-f\n @@ -1,3 +1,4 @@\n a\n -e\n +b\n +c\n d\n\nIOW, your request to \"add -e\" was \"I do not want to put the addition of\n'c' in the index at this point (that is why you removed '+c')\" and \"I do\nnot want to put the addition of 'b' in the index; I want 'e' instead (that\nis why you changed '+b' to '+e')\". After \"add -e\" granted both requests,\nwhat is left in the work tree can confuse the user. The \"not at this\npoint\" part of the first request is clearly visible that the leftover diff\nhas an addition of '+c'. But the user may expect that the second request,\n\"I don't want 'b', I want 'e'\", would carry over to the work tree and not\nsee the apparent reversion of the wish (i.e. you changed it to add 'e'\ninstead of 'b' in \"add -e\" session, but it is reverted and \"commit -a\"\nwould record the version with 'b' instead).\n"},{"id":"125341","messageId":"20091019063446.GA1457@coredump.intra.peff.net","threadId":"21242","inReplyTo":"7vbpk4aqop.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-19T06:34:46Z","receivedAt":"2009-10-19T06:34:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 18, 2009 at 10:38:46PM -0700, Junio C Hamano wrote:\n\n> > -*NOTE*: Obviously, if you change anything else than the first character\n> > -on lines beginning with a space or a minus, the patch will no longer\n> > -apply.\n> > +The intent of this option is to pick and choose lines of the diff to\n> \n> I'd slightly prefer \"patch\" over \"diff\" in this context.\n\nSure, I have no problem with that.\n\n> > +apply, or even to modify the contents of lines to be staged. There are\n> > +three line types in a diff: addition lines (beginning with a plus),\n> > +removal lines (beginning with a minus), and context lines (beginning\n> > +with a space). In general, it should be safe to:\n> > ++\n> > +--\n> > +* remove addition lines (don't stage the line)\n> \n> This is more like \"don't add the line\", isn't it? Also if this \"+\" line\n> has corresponding \"-\" line (i.e. it is a \"rewrite to this\" line), removal\n> of such a line would mean \"instead of rewriting, remove it\".\n\nI was trying not to use \"add\" because we are already talking about\naddition and removal in the patch itself, and I thought it made sense to\nmap those actions to what the user is conceptually doing (staging or not\nstaging). Of course, I used \"remove\" which has a similar problem (you\nare removing the addition of the line to the index); probably \"delete\"\nwould have been better. So I am not sure I agree that using \"add\" is any\nbetter than \"stage\", but I don't feel that strongly about it.\n\nBut beyond that, yes, you are right that removing a \"+\" line may have a\ndifferent conceptual meaning to the user depending on the surrounding\ntext. I wonder if such a \"check-list\" document really makes much sense,\ngiven that using \"-e\" at all means you need to understand the patch\nformat and what makes sense (i.e., anybody who understands 'patch' knows\nthat you can't just delete context lines and expect it to apply).\n\n> Obviously, the above patch is what \"diff --cached\" would show after such\n> an \"add -e\", but this does _not_ touch anything in the work tree (which is\n> correct; add is about moving changed contents to the index and it should\n> fundamentally not affect work tree). As a result, \"diff\" between the\n> index and the work tree now would read like this:\n> \n> diff --git a/t-f b/t-f\n> index 17c3f0b..d68dd40 100644\n> --- a/t-f\n> +++ b/t-f\n> @@ -1,3 +1,4 @@\n> a\n> -e\n> +b\n> +c\n> d\n> \n> IOW, your request to \"add -e\" was \"I do not want to put the addition of\n> 'c' in the index at this point (that is why you removed '+c')\" and \"I do\n> not want to put the addition of 'b' in the index; I want 'e' instead (that\n> is why you changed '+b' to '+e')\". After \"add -e\" granted both requests,\n> what is left in the work tree can confuse the user. The \"not at this\n\nYeah, again, this comes down to understanding what you are doing:\napplying a patch to the index. So it really requires that the user\nunderstand what that means. Maybe instead of trying to enumerate cases\nand their effects, we should just say sometihng like \"this is a patch to\nthe index. While you can make arbitrary changes, keep in mind that (1)\nyour changes must apply to the content in the index, and (2) any new\nchanges you introduce in the patch will not be reflected in the working\ntree\".\n\n-Peff\n"},{"id":"125344","messageId":"7vaazn7tg4.fsf@alter.siamese.dyndns.org","threadId":"21242","inReplyTo":"20091019063446.GA1457@coredump.intra.peff.net","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-19T07:07:23Z","receivedAt":"2009-10-19T07:07:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> > +* remove addition lines (don't stage the line)\n>> \n>> This is more like \"don't add the line\", isn't it? Also if this \"+\" line\n>> has corresponding \"-\" line (i.e. it is a \"rewrite to this\" line), removal\n>> of such a line would mean \"instead of rewriting, remove it\".\n>\n> I was trying not to use \"add\" because we are already talking about\n> addition and removal in the patch itself,\n\nAh, I wasn't saying \"add\" is more kosher than \"stage\" by the above.\n\nBy \"don't add\", I meant that the user is saying \"I inserted a new line to\nthe file, but I actually did not want to add that line to the file for the\nnext commit.\" In other words, I was more focusing on the act of inserting\nthe line to the contents, not on staging the change to the index.\n\n> ... So I am not sure I agree that using \"add\" is any\n> better than \"stage\", but I don't feel that strongly about it.\n\nThat is Ok; the comment was not about stage vs add.\n\n> But beyond that, yes, you are right that removing a \"+\" line may have a\n> different conceptual meaning to the user depending on the surrounding\n> text. I wonder if such a \"check-list\" document really makes much sense,\n> given that using \"-e\" at all means you need to understand the patch\n> format and what makes sense (i.e., anybody who understands 'patch' knows\n> that you can't just delete context lines and expect it to apply).\n\nYeah, that is really what I wanted people who are in this discussion to\neventually realize ;-)\n\n> Yeah, again, this comes down to understanding what you are doing:\n"},{"id":"125400","messageId":"200910191142.03446.wjl@icecavern.net","threadId":"21242","inReplyTo":"7vaazn7tg4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Wesley J. Landaker","fromEmail":"wjl@icecavern.net","sentAt":"2009-10-19T17:42:03Z","receivedAt":"2009-10-19T17:42:03Z","isPatch":true,"sender":{"key":"wjl@icecavern.net","avatar":"https://avatars.githubusercontent.com/u/67229?v=4"},"body":"On Monday 19 October 2009 01:07:23 Junio C Hamano wrote:\n> That is Ok; the comment was not about stage vs add.\n>\n> > But beyond that, yes, you are right that removing a \"+\" line may have a\n> > different conceptual meaning to the user depending on the surrounding\n> > text. I wonder if such a \"check-list\" document really makes much sense,\n> > given that using \"-e\" at all means you need to understand the patch\n> > format and what makes sense (i.e., anybody who understands 'patch'\n> > knows that you can't just delete context lines and expect it to apply).\n>\n> Yeah, that is really what I wanted people who are in this discussion to\n> eventually realize ;-)\n\nComment from the peanut gallery:\n\nI still think a quick summary checklist is useful even for a seasoned \ndeveloper that is intimate with the 'patch' format, as it lets users know \nwhat git will do with your patch modifications.\n\nFor example, when I first tried \"add -e\", my first thought was: \"Awesome, \nbut, I wonder if git will do the right thing if I modify the patch in THIS \nway ...\". Fortunately, git did the right thing, but I wasn't really sure \nuntil I tried it.\n"},{"id":"125428","messageId":"7v3a5evqxu.fsf@alter.siamese.dyndns.org","threadId":"21242","inReplyTo":"200910191142.03446.wjl@icecavern.net","subject":"Re: [PATCH] git add -e documentation: rephrase note","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-20T00:39:57Z","receivedAt":"2009-10-20T00:39:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Wesley J. Landaker\" writes:\n\n> On Monday 19 October 2009 01:07:23 Junio C Hamano wrote:\n>> That is Ok; the comment was not about stage vs add.\n>>\n>> > But beyond that, yes, you are right that removing a \"+\" line may have a\n>> > different conceptual meaning to the user depending on the surrounding\n>> > text. I wonder if such a \"check-list\" document really makes much sense,\n>> > given that using \"-e\" at all means you need to understand the patch\n>> > format and what makes sense (i.e., anybody who understands 'patch'\n>> > knows that you can't just delete context lines and expect it to apply).\n>>\n>> Yeah, that is really what I wanted people who are in this discussion to\n>> eventually realize ;-)\n>\n> Comment from the peanut gallery:\n>\n> I still think a quick summary checklist is useful even for a seasoned \n> developer that is intimate with the 'patch' format, as it lets users know \n> what git will do with your patch modifications.\n\nOh, that is not something I am against at all. I (mis)took Peff's \"what\nmakes sense\" to cover more than what a patch format is, namely, \"what the\nuser by modifying the patch and 'add -e' by accepting a modified patch are\ndoing.\"\n\n> For example, when I first tried \"add -e\", my first thought was: \"Awesome, \n> but, I wonder if git will do the right thing if I modify the patch in THIS \n> way ...\". Fortunately, git did the right thing, but I wasn't really sure \n> until I tried it.\n\nActually, my \"change +b to +e\" example was meant to illustrate that git\nis not necessarily doing the right thing.\n\nAdmittedly, there is no way to always do the right thing unless git can\nread user's mind in this case, and realization of why it is impossible to\ndo is necessary before using \"add -e\"; otherwise you would end up getting\nutterly confused. I do not use \"add -e\" myself.\n\nIn one edit (i.e. \"remove +c\"), the intention is clear that the insertion\nis merely kept out of the index and it might even eventually be added in a\nlater commit. \"c\" could be a debugging \"printf()\" that the user may not\never want to commit. In either case, it is clear that the user wants to\nkeep that change in the work tree version.\n\nIn another edit (i.e. \"change +b to +e\"), the only thing that is clear is\nthat the user wants a version with \"e\" in the next commit.\n\nChanging from \"+b\" to \"+e\" in \"add -e\" might have been done, in order to\nchange \"+if (1 || debug)\" in the work tree version to \"+if (debug)\" in the\nversion to be committed (so that the user can keep debugging without\ngiving the --debug option to the program, for example). In this case, it\nis quite similar to the earlier \"remove +c\" (\"I know I inserted 'c' in my\nwork tree version, do not commit that change, but I do want to keep it in\nmy work tree\") case, and keeping the work tree intact is the right thing\nto do.\n\nBut it might have been done because the user spotted a typo in \"b\" and\nwanted to correct it to \"e\", in which case the user may wish the change\nbe reflected to the work tree.\n\nAs there is no way to distinguish the last two cases (and I do not think\nthe code even attempts to tell the difference); half of the time you would\n(perhaps mistakenly) think that \"add -e\" did a wrong thing to your typofix\nedit, after adding the updated contents correctly to the index: \"I told\ngit to fix the typo 'b' to 'e', and I committed the corrected version, but\nthen it discarded my fix---the typo is still there in my work tree\".\n"}]}