{"thread":{"id":"13641","subject":"[PATCH] git-add--interactive: manual hunk editing mode","startedAt":"2008-05-23T20:21:43Z","lastAt":"2008-06-27T17:43:09Z","messageCount":62,"participants":["Thomas Rast","Ping Yin","Johannes Schindelin","Jeff King","Jakub Narebski","Junio C Hamano","Johan Herland","Andreas Ericsson","Miklos Vajna"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"77572","messageId":"200805232221.45406.trast@student.ethz.ch","threadId":"13641","inReplyTo":null,"subject":"[PATCH] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-05-23T20:21:43Z","receivedAt":"2008-05-23T20:21:43Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Adds a new option 'e' to the 'add -p' command loop that lets you\ndiscard or keep one hunk line at a time. This is useful if there are\nno unchanged lines in the middle of the hunk, so 's' will not work,\nbut you would still like to split it.\n\nSigned-off-by: Thomas Rast \n---\n\nThis is my first patch, and I had to dust off my Perl knowledge a bit,\nso I hope it is up to your standards.\n\nI could have used this a few times because I frequently hack around a\nbit, then after some time notice that this should really go in two\nseparate commits. Usually 'add -p' can separate the changes, but if\ntheir diff lines are immediately adjacent, one has to go back and\nundo some editing before the first commit, then redo later.\n\nI'm not quite happy with the scheme I use to handle colored diffs.\n'git diff' apparently does not always reset the color before newlines\n(is this a bug?), so I insert extra resets. However, I did not want\nto implement \"full\" diff coloring directly in git-add--interactive.\n\nAlso, I would be glad to hear your comments on the \"user interface\" of\nthe edit command loop. I haven't found a good way of saying \"an\noptional number followed by y or n\" that is consistent with the\n\"[y/n]\" format used in the rest of the patch loop. Similiarly, an\noption to undo an edit might be nice, but would complicate the code a\nfair bit.\n\nBest regards,\nThomas\n\n\n Documentation/git-add.txt | 1 +\n git-add--interactive.perl | 110 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 111 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex bb4abe2..8de4d4a 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -229,6 +229,7 @@ patch::\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+ e - manually edit the current hunk\n ? - print help\n +\n After deciding the fate for all hunks, if there is any hunk\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..8af841a 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -18,6 +18,10 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -682,6 +686,107 @@ sub split_hunk {\n \treturn @split;\n }\n \n+sub edit_hunk_manually {\n+\tmy ($in_text, $in_display) = @_;\n+\n+\tmy @text = @$in_text;\n+\tmy @display = @$in_display;\n+\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = parse_hunk_header($text[0]);\n+\tmy $num = scalar @text;\n+\n+\tmy $ix = 0;\n+\n+ OUTER:\n+\twhile (1) {\n+\t\t$text[0] = (\"@@ -$o_ofs\" .\n+\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n+\t\t\t \" +$n_ofs\" .\n+\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n+\t\t\t \" @@\\n\");\n+\t\tif ($diff_use_color) {\n+\t\t\t$display[0] = colored $fraginfo_color, $text[0];\n+\t\t}\n+\t\telse {\n+\t\t\t$display[0] = $text[0];\n+\t\t}\n+\n+\t\twhile ($text[$ix] !~ /^[+-]/) {\n+\t\t\t$ix++;\n+\t\t\tlast OUTER if $ix >= $num;\n+\t\t}\n+\t\tmy $lineno = 0;\n+\t\tfor (my $i = 0; $i < $num; $i++) {\n+\t\t\tif ($i >= $ix && $text[$i] =~ /^[+-]/) {\n+\t\t\t\t$lineno++;\n+\t\t\t\tif ($lineno == 1) {\n+\t\t\t\t\tprint $normal_color . colored($prompt_color, \">1 \");\n+\t\t\t\t}\n+\t\t\t\telsif ($lineno < 100) {\n+\t\t\t\t\tprint $normal_color . colored($prompt_color, sprintf(\"%2d \", $lineno));\n+\t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\tprint \" \";\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tprint \" \";\n+\t\t\t}\n+\t\t\tprint $display[$i];\n+\t\t}\n+\t\tprint colored $prompt_color, \"Use line(s) [y/n]? \";\n+\t\tmy $line = ;\n+\t\tif ($line) {\n+\t\t\tif ($line =~ /^(\\d*)y/) {\n+\t\t\t\tmy $repeat = $1 || 1;\n+\t\t\t\twhile (1) {\n+\t\t\t\t\tlast if ($repeat <= 0 || $ix >= $num);\n+\t\t\t\t\t$repeat-- if ($text[$ix] =~ /^[+-]/);\n+\t\t\t\t\t$ix++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\telsif ($line =~ /^(\\d*)n/) {\n+\t\t\t\t# This is the interesting case.\n+\t\t\t\t# - lines become context, + lines are dropped\n+\t\t\t\tmy $repeat = $1 || 1;\n+\t\t\t\twhile (1) {\n+\t\t\t\t\tlast if ($repeat <= 0 || $ix >= $num);\n+\t\t\t\t\tif ($text[$ix] =~ /^\\+/) {\n+\t\t\t\t\t\t$repeat--;\n+\t\t\t\t\t\tsplice(@text, $ix, 1);\n+\t\t\t\t\t\tsplice(@display, $ix, 1);\n+\t\t\t\t\t\t$n_cnt--;\n+\t\t\t\t\t\t$num--;\n+\t\t\t\t\t\t$ix--;\n+\t\t\t\t\t}\n+\t\t\t\t\telsif ($text[$ix] =~ /^-/) {\n+\t\t\t\t\t\t$repeat--;\n+\t\t\t\t\t\t$n_cnt++;\n+\t\t\t\t\t\t$text[$ix] =~ s/^-/ /;\n+\t\t\t\t\t\t# need a better way to do this\n+\t\t\t\t\t\t$display[$ix] = $normal_color . colored $diff_plain_color, $text[$ix];\n+\t\t\t\t\t}\n+\t\t\t\t\t$ix++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\twhile (1) {\n+\t\tfor (@display) {\n+\t\t\tprint;\n+\t\t}\n+\t\tprint colored $prompt_color, \"Accept and replace old hunk [y/n]? \";\n+\t\tmy $line = ;\n+\t\tif ($line =~ /^y/) {\n+\t\t\treturn (\\@text, \\@display);\n+\t\t}\n+\t\telsif ($line =~ /^n/) {\n+\t\t\treturn ($in_text, $in_display);\n+\t\t}\n+\t}\n+}\n+\n sub find_last_o_ctx {\n \tmy ($it) = @_;\n \tmy $text = $it->{TEXT};\n@@ -781,6 +886,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -885,6 +991,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1056,9 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\t($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY}) = edit_hunk_manually($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n-- \n1.5.4.5\n"},{"id":"77627","messageId":"20080524012458.GA15851@kooxoo235","threadId":"13641","inReplyTo":"200805232221.45406.trast@student.ethz.ch","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-24T01:24:58Z","receivedAt":"2008-05-24T01:24:58Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"* Thomas Rast [2008-05-23 22:21:43 +0200]:\n\n> Adds a new option 'e' to the 'add -p' command loop that lets you\n> discard or keep one hunk line at a time. This is useful if there are\n> no unchanged lines in the middle of the hunk, so 's' will not work,\n> but you would still like to split it.\n> \nWow, nice feature, although i havn't tried.\n"},{"id":"78053","messageId":"200805291737.53291.trast@student.ethz.ch","threadId":"13641","inReplyTo":"200805232221.45406.trast@student.ethz.ch","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-05-29T15:37:51Z","receivedAt":"2008-05-29T15:37:51Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"You wrote:\n> Adds a new option 'e' to the 'add -p' command loop that lets you\n> discard or keep one hunk line at a time. This is useful if there are\n> no unchanged lines in the middle of the hunk, so 's' will not work,\n> but you would still like to split it.\n\nAny news on this? I would greatly appreciate criticism if something\nis wrong or inadequate :-)\n\nThanks,\nThomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n"},{"id":"78056","messageId":"alpine.DEB.1.00.0805291708500.13507@racer.site.net","threadId":"13641","inReplyTo":"200805291737.53291.trast@student.ethz.ch","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-29T16:12:08Z","receivedAt":"2008-05-29T16:12:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 29 May 2008, Thomas Rast wrote:\n\n> You wrote:\n\nWho \"You\"? You did not say a name.\n\n> > Adds a new option 'e' to the 'add -p' command loop that lets you \n> > discard or keep one hunk line at a time. This is useful if there are \n> > no unchanged lines in the middle of the hunk, so 's' will not work, \n> > but you would still like to split it.\n> \n> Any news on this? I would greatly appreciate criticism if something is \n> wrong or inadequate :-)\n\nOh, probably was you, yourself. Hrm. Could have said that.\n\nThe splitting (even without common lines at the borders) is something I \nneeded myself, but the concept got rebuked in\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n\n(See the whole thread for more information)\n\nCiao,\nDscho\n"},{"id":"78079","messageId":"20080529185808.GA2140@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200805291737.53291.trast@student.ethz.ch","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-29T18:58:08Z","receivedAt":"2008-05-29T18:58:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 29, 2008 at 05:37:51PM +0200, Thomas Rast wrote:\n\n> You wrote:\n> > Adds a new option 'e' to the 'add -p' command loop that lets you\n> > discard or keep one hunk line at a time. This is useful if there are\n> > no unchanged lines in the middle of the hunk, so 's' will not work,\n> > but you would still like to split it.\n> \n> Any news on this? I would greatly appreciate criticism if something\n> is wrong or inadequate :-)\n\n[please just repost the patch when issuing such gentle reminders -- it\nsaves the rest of us from digging it out of the archive]\n\nI have often wanted to perform this operation, and your implementation\nseems to work from my minimal testing (though I haven't looked closely\nat the colorization implications).\n\nBut I find the interface a bit clunky. I would much rather get dumped in\nmy favorite editor, which happens to be quite fast at removing a subset\nof lines. After editing, any lines remaining would be staged.\n\nWe would have to figure out what happens if lines are added or edited,\nof course. It may be right to signal an error, or maybe there is some\nother useful functionality that can come of that. I think other systems\nhave some diff-editing functionality (IIRC, cogito did). It is probably\nworth looking at that for ideas.\n\n-Peff\n"},{"id":"78080","messageId":"200805292110.02151.trast@student.ethz.ch","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0805291708500.13507@racer.site.net","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-05-29T19:10:00Z","receivedAt":"2008-05-29T19:10:00Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Thu, 29 May 2008, Thomas Rast wrote:\n> \n> > You wrote:\n> \n> Who \"You\"? You did not say a name.\n> \n> > > Adds a new option 'e' to the 'add -p' command loop that lets you \n> > > discard or keep one hunk line at a time. This is useful if there are \n> > > no unchanged lines in the middle of the hunk, so 's' will not work, \n> > > but you would still like to split it.\n> > \n> \n> The splitting (even without common lines at the borders) is something I \n> needed myself, but the concept got rebuked in\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n\nAha, thanks for the pointer. I'm not sure I'm in a position to\nrestart that discussion, but let's try anyway:\n\nNote that what Junio said in\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n> That is, , if you want to do finer grained hunk splitting than what \"git\n> add -p\" lets you do, you do _not_ let user specify \"I want to split the\n> hunk into two, before this point and after this point\". Instead, let\n> the user pick zero or more '-' line and zero or more '+' line, and\n> adjust the context around it. An unpicked '-' line becomes the common\n> context, and an unpicked '+' line disappears. After that, you recount\n> the diff. That way, you do not have to do any \"unidiff zero\" cop-out.\n\nis precisely what my patch implements. I did not implement the\nstashing he recommends in the same message, but I believe the fact\nthat 'add -p' stages the end result should be enough (and works at\nleast for my use case).\n\nJohannes Sixt replied:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n> In this case I would expect two adjacent hunks: one that covers the selected\n> changes, another one with the remaining changes, but each against the original:\n\nI didn't do this because there is no way to ensure that the \"other\nhalf\" can be applied cleanly to the unpatched state, thus requiring\nextra tracking of \"hunk dependencies\", so to speak. It could be done\nof course, if anyone really needs it.\n\nYou also mention\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n> I thought about [the line-wise picking], but the UI is not trivial.\n\nThe y/n loop with count prefix works well for me. It would be nice to\nhave it in an actual GUI of course, but I don't know enough Tcl/Tk to\nimplement that.\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n"},{"id":"78081","messageId":"200805292116.31006.trast@student.ethz.ch","threadId":"13641","inReplyTo":"200805292110.02151.trast@student.ethz.ch","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-05-29T19:16:30Z","receivedAt":"2008-05-29T19:16:30Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> > http://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n> http://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n> http://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n> http://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68108\n\nBah, obviously I failed to notice that the links are all the same.\n\nThe correct links:\n\nJunio's remarks about +/- picking:\n http://article.gmane.org/gmane.comp.version-control.git/68131\nJohannes Sixt's reply:\n http://article.gmane.org/gmane.comp.version-control.git/68138\nJohannes Schindelin's reply:\n http://article.gmane.org/gmane.comp.version-control.git/68143\n\nSorry for the spam.\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n"},{"id":"78124","messageId":"alpine.DEB.1.00.0805301049000.13507@racer.site.net","threadId":"13641","inReplyTo":"20080529185808.GA2140@sigill.intra.peff.net","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-30T09:49:57Z","receivedAt":"2008-05-30T09:49:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 29 May 2008, Jeff King wrote:\n\n> I would much rather get dumped in my favorite editor, which happens to \n> be quite fast at removing a subset of lines. After editing, any lines \n> remaining would be staged.\n> \n> We would have to figure out what happens if lines are added or edited,\n> of course. It may be right to signal an error, or maybe there is some\n> other useful functionality that can come of that. I think other systems\n> have some diff-editing functionality (IIRC, cogito did). It is probably\n> worth looking at that for ideas.\n\nMaybe you want to start from this patch (only compile-tested):\n\n-- snipsnap --\n[PATCH] WIP: allow git-apply to fix up the line counts\n\nSometimes, the easiest way to fix up a patch is to edit it directly, even\nadding or deleting lines. Now, many people are not as divine as certain\nbenevolent dictators as to update the hunk headers correctly at the first\ntry.\n\nSo teach the tool to do it for us.\n---\n Documentation/git-apply.txt | 6 +++-\n builtin-apply.c | 66 ++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 64 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex 2dec2ec..8007d10 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git-apply' [--stat] [--numstat] [--summary] [--check] [--index]\n \t [--apply] [--no-add] [--build-fake-ancestor ] [-R | --reverse]\n \t [--allow-binary-replacement | --binary] [--reject] [-z]\n-\t [-pNUM] [-CNUM] [--inaccurate-eof] [--cached]\n+\t [-pNUM] [-CNUM] [--inaccurate-eof] [--fixup] [--cached]\n \t [--whitespace=]\n \t [--exclude=PATH] [--verbose] [...]\n \n@@ -169,6 +169,10 @@ behavior:\n \tcorrectly. This option adds support for applying such patches by\n \tworking around this bug.\n \n+--fixup-line-counts::\n+\tFix up the line counts (e.g. after editing the patch without\n+\tadjusting the hunk headers appropriately).\n+\n -v, --verbose::\n \tReport progress to stderr. By default, only a message about the\n \tcurrent patch being applied will be printed. This option will cause\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex c497889..c8f1cd9 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -153,6 +153,7 @@ struct patch {\n \tunsigned int is_binary:1;\n \tunsigned int is_copy:1;\n \tunsigned int is_rename:1;\n+\tunsigned int fixup:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -882,11 +883,49 @@ static int parse_range(const char *line, int len, int offset, const char *expect\n \treturn offset + ex;\n }\n \n+static int fixup_counts(char *line, int len, struct fragment *fragment)\n+{\n+\n+\tif (len < 1)\n+\t\treturn -1;\n+\n+\tfragment->oldlines = fragment->newlines = 0;\n+\n+\tfor (;;) {\n+\t\tchar *newline = memchr(line, '\\n', len);\n+\t\tif (!newline)\n+\t\t\tbreak;\n+\n+\t\tswitch (*line) {\n+\t\tcase ' ':\n+\t\t\tfragment->oldlines++;\n+\t\t\t/* fall through */\n+\t\tcase '+':\n+\t\t\tfragment->newlines++;\n+\t\t\tbreak;\n+\t\tcase '-':\n+\t\t\tfragment->oldlines++;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\treturn error(\"Unexpected first character: %c in line \"\n+\t\t\t\t\"'%.*s'\",\n+\t\t\t\t*line, newline ? newline - line : len, line);\n+\t\t}\n+\n+\t\tlen -= newline + 1 - line;\n+\t\tline = newline + 1;\n+\t\tif (len < 2 || !prefixcmp(line, \"@@\"))\n+\t\t\tbreak;\n+\t}\n+\treturn 0;\n+}\n+\n /*\n * Parse a unified diff fragment header of the\n * form \"@@ -a,b +c,d @@\"\n */\n-static int parse_fragment_header(char *line, int len, struct fragment *fragment)\n+static int parse_fragment_header(char *line, int len, struct fragment *fragment,\n+\tint fixup)\n {\n \tint offset;\n \n@@ -897,6 +936,10 @@ static int parse_fragment_header(char *line, int len, struct fragment *fragment)\n \toffset = parse_range(line, len, 4, \" +\", &fragment->oldpos, &fragment->oldlines);\n \toffset = parse_range(line, len, offset, \" @@\", &fragment->newpos, &fragment->newlines);\n \n+\tif (offset > 0 && fixup &&\n+\t\t\tfixup_counts(line + offset, len - offset, fragment))\n+\t\treturn -1;\n+\n \treturn offset;\n }\n \n@@ -927,7 +970,8 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc\n \t\t */\n \t\tif (!memcmp(\"@@ -\", line, 4)) {\n \t\t\tstruct fragment dummy;\n-\t\t\tif (parse_fragment_header(line, len, &dummy) < 0)\n+\t\t\tif (parse_fragment_header(line, len, &dummy,\n+\t\t\t\t\tpatch->fixup) < 0)\n \t\t\t\tcontinue;\n \t\t\tdie(\"patch fragment without header at line %d: %.*s\",\n \t\t\t linenr, (int)len-1, line);\n@@ -1010,7 +1054,7 @@ static int parse_fragment(char *line, unsigned long size,\n \tunsigned long oldlines, newlines;\n \tunsigned long leading, trailing;\n \n-\toffset = parse_fragment_header(line, len, fragment);\n+\toffset = parse_fragment_header(line, len, fragment, patch->fixup);\n \tif (offset < 0)\n \t\treturn -1;\n \toldlines = fragment->oldlines;\n@@ -2912,7 +2956,8 @@ static void prefix_patches(struct patch *p)\n \t}\n }\n \n-static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n+static int apply_patch(int fd, const char *filename, int inaccurate_eof,\n+\t\tint fixup)\n {\n \tsize_t offset;\n \tstruct strbuf buf;\n@@ -2929,6 +2974,7 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n \n \t\tpatch = xcalloc(1, sizeof(*patch));\n \t\tpatch->inaccurate_eof = inaccurate_eof;\n+\t\tpatch->fixup = fixup;\n \t\tnr = parse_chunk(buf.buf + offset, buf.len - offset, patch);\n \t\tif (nr < 0)\n \t\t\tbreak;\n@@ -2998,6 +3044,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \tint i;\n \tint read_stdin = 1;\n \tint inaccurate_eof = 0;\n+\tint fixup = 0;\n \tint errs = 0;\n \tint is_not_gitdir;\n \n@@ -3015,7 +3062,8 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\tint fd;\n \n \t\tif (!strcmp(arg, \"-\")) {\n-\t\t\terrs |= apply_patch(0, \"\", inaccurate_eof);\n+\t\t\terrs |= apply_patch(0, \"\", inaccurate_eof,\n+\t\t\t\t\tfixup);\n \t\t\tread_stdin = 0;\n \t\t\tcontinue;\n \t\t}\n@@ -3118,6 +3166,10 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\tinaccurate_eof = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(arg, \"--fixup-line-counts\")) {\n+\t\t\tfixup = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (0 < prefix_length)\n \t\t\targ = prefix_filename(prefix, prefix_length, arg);\n \n@@ -3126,12 +3178,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\tdie(\"can't open patch '%s': %s\", arg, strerror(errno));\n \t\tread_stdin = 0;\n \t\tset_default_whitespace_mode(whitespace_option);\n-\t\terrs |= apply_patch(fd, arg, inaccurate_eof);\n+\t\terrs |= apply_patch(fd, arg, inaccurate_eof, fixup);\n \t\tclose(fd);\n \t}\n \tset_default_whitespace_mode(whitespace_option);\n \tif (read_stdin)\n-\t\terrs |= apply_patch(0, \"\", inaccurate_eof);\n+\t\terrs |= apply_patch(0, \"\", inaccurate_eof, fixup);\n \tif (whitespace_error) {\n \t\tif (squelch_whitespace_errors &&\n \t\t squelch_whitespace_errors < whitespace_error) {\n-- \n1.5.6.rc0.206.g61199\n"},{"id":"78130","messageId":"m3prr4135z.fsf@localhost.localdomain","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0805301049000.13507@racer.site.net","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-30T10:46:26Z","receivedAt":"2008-05-30T10:46:26Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Johannes Schindelin writes:\n\n> -- snipsnap --\n> [PATCH] WIP: allow git-apply to fix up the line counts\n> \n> Sometimes, the easiest way to fix up a patch is to edit it directly, even\n> adding or deleting lines. Now, many people are not as divine as certain\n> benevolent dictators as to update the hunk headers correctly at the first\n> try.\n\nOr do not use editor with diff editing mode, such as unidiff mode for\nGNU Emacs (and derivatives)... :-)\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"78132","messageId":"200805301421.46162.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080529185808.GA2140@sigill.intra.peff.net","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-05-30T12:21:44Z","receivedAt":"2008-05-30T12:21:44Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> But I find the interface a bit clunky. I would much rather get dumped in\n> my favorite editor, which happens to be quite fast at removing a subset\n> of lines. After editing, any lines remaining would be staged.\n> \n> We would have to figure out what happens if lines are added or edited,\n> of course. It may be right to signal an error, or maybe there is some\n> other useful functionality that can come of that. I think other systems\n> have some diff-editing functionality (IIRC, cogito did). It is probably\n> worth looking at that for ideas.\n\nWe could just see if the hunk applies to the unchanged index (still\nassuming we're inside 'add -p'), and if not, reject the edit.\n\nUnfortunately git-apply does not seem to have a --dry-run option.\n(Even stranger, when given the option --dry-run it tries to open a\npatch of that name.) What is the recommended way to do such things?\nMake a backup copy of the index and apply --cached anyway?\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n"},{"id":"78182","messageId":"7v8wxriige.fsf@gitster.siamese.dyndns.org","threadId":"13641","inReplyTo":"200805301421.46162.trast@student.ethz.ch","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-30T21:35:13Z","receivedAt":"2008-05-30T21:35:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Unfortunately git-apply does not seem to have a --dry-run option.\n\n--check?\n"},{"id":"78260","messageId":"200806010241.51464.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080529185808.GA2140@sigill.intra.peff.net","subject":"[RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-01T00:41:48Z","receivedAt":"2008-06-01T00:41:48Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Adds a new option 'e' to the 'add -p' command loop that lets you\nedit the current hunk in your favourite editor.\n---\n\nThis is a draft of Jeff King's idea\n\nJeff King wrote:\n> But I find the interface a bit clunky. I would much rather get dumped in\n> my favorite editor, which happens to be quite fast at removing a subset\n> of lines. After editing, any lines remaining would be staged.\n>\n> We would have to figure out what happens if lines are added or edited,\n> of course. It may be right to signal an error, or maybe there is some\n> other useful functionality that can come of that.\n\nin the hope that I might get some more ideas and pointers on the details.\n\nThe current implementation rejects edits that break the (whole) patch\n(thanks Junio for pointing out --check...), but it does help the user\nin one aspect: @ lines are edited to reflect their hunk contents, even\ninferring the starting line numbers from the last hunk if they are\nmissing. This means you can insert a line consisting just of an @,\nand it will silently be fixed to start a new hunk at that point.\n\nSome things that might need improvement/fixing:\n\n- Does anyone need a way to force a broken diff past the git-apply\n check? I don't know what use such a diff might have, but who knows,\n perhaps it applies cleanly once you exclude a hunk or two...\n\n- Perhaps the instructions should go to the bottom, but then the odds\n are they would not be visible in many cases.\n\n- Should I try to come up with a unique filename to support concurrent\n edits? (git-commit doesn't...)\n\n- Perl's autovivification is out to get me. I've fixed a few, but\n there are probably still bugs.\n\nThomas\n\n---\n git-add--interactive.perl | 171 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 171 insertions(+), 0 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..c752e20 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -18,6 +18,18 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n+my ($diff_old_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.old', 'red'),\n+\t) : ();\n+my ($diff_new_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.new', 'green'),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -770,6 +782,158 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub edit_hunk_manually {\n+\tmy @oldtext;\n+\tfor (@_) {\n+\t\tpush @oldtext, @{$_->{TEXT}};\n+\t}\n+\n+\t# use a .diff file to help editors with highlighting\n+\tmy $editpath = $repo->repo_path() . \"/ADDP_HUNK_EDIT.diff\";\n+\tmy $fh;\n+\topen $fh, '>', $editpath\n+\t\tor die \"failed to open hunk edit file for writing: \" . $!;\n+\tprint $fh <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $editpath);\n+\n+\topen $fh, '<', $editpath\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext;\n+\twhile (<$fh>) {\n+\t\tpush (@newtext, $_) unless /^#/ || /^$/;\n+\t}\n+\tclose $fh;\n+\tmy @heads = ();\n+\tmy ($o_ofs, $n_ofs);\n+\tmy $o_cnt = 0;\n+\tmy $n_cnt = 0;\n+\tmy ($guess_o_ofs, undef, $guess_n_ofs, undef) = parse_hunk_header($oldtext[0]);\n+\tfor (my $i = 0; $i < @newtext; $i++) {\n+\t\tif ((scalar @heads) == 0 && $newtext[$i] =~ /^[ +-]/) {\n+\t\t\tsplice @newtext, $i, 0, $oldtext[0];\n+\t\t\tpush @heads, $i;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^ /) {\n+\t\t\t$o_cnt++;\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^-/) {\n+\t\t\t$o_cnt++;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^\\+/) {\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^@/) {\n+\t\t\tif (@heads > 0) {\n+\t\t\t\t# fix up the previous header first\n+\t\t\t\t($o_ofs, undef, $n_ofs, undef)\n+\t\t\t\t\t= parse_hunk_header($newtext[$heads[-1]]);\n+\t\t\t\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n+\t\t\t\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n+\t\t\t\t$newtext[$heads[-1]] = (\n+\t\t\t\t\t\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n+\t\t\t\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n+\t\t\t\t\t. \" @@\\n\");\n+\t\t\t\t$guess_o_ofs = $o_ofs + $o_cnt;\n+\t\t\t\t$guess_n_ofs = $n_ofs + $n_cnt;\n+\t\t\t}\n+\t\t\t$o_cnt = 0;\n+\t\t\t$n_cnt = 0;\n+\t\t\tpush @heads, $i;\n+\t\t}\n+\t}\n+\t($o_ofs, undef, $n_ofs, undef)\n+\t\t= parse_hunk_header($newtext[$heads[-1]]);\n+\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n+\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n+\t$newtext[$heads[-1]] = (\n+\t\t\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n+\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n+\t\t. \" @@\\n\");\n+\n+\tpush @heads, (scalar @newtext);\n+\tmy (@hunks) = ();\n+\tfor (my $i = 0; $i < @heads-1; $i++) {\n+\t\tmy @hunktext = @newtext[$heads[$i]..$heads[$i+1]-1];\n+\t\tmy @hunkdisplay = ();\n+\t\tfor (@hunktext) {\n+\t\t\tif (/^@/) {\n+\t\t\t\tpush @hunkdisplay, (colored $fraginfo_color, $_);\n+\t\t\t}\n+\t\t\telsif (/^\\+/) {\n+\t\t\t\tpush @hunkdisplay, (colored $diff_new_color, $_);\n+\t\t\t}\n+\t\t\telsif (/^-/) {\n+\t\t\t\tpush @hunkdisplay, (colored $diff_old_color, $_);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tpush @hunkdisplay, (colored $diff_plain_color, $_);\n+\t\t\t}\n+\t\t}\n+\t\tpush @hunks, {TEXT => \\@hunktext, DISPLAY => \\@hunkdisplay};\n+\t}\n+\n+\treturn @hunks;\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $hunks, $ix) = @_;\n+\n+\tmy @newhunks = ($hunks->[$ix]);\n+\n+ EDIT:\n+\twhile (1) {\n+\t\t@newhunks = edit_hunk_manually(@newhunks);\n+\t\tmy $fh;\n+\t\topen $fh, '| git apply --cached --check';\n+\t\tfor my $h ($head,\n+\t\t\t $ix > 0 ? @$hunks[0..$ix-1] : (),\n+\t\t\t @newhunks,\n+\t\t\t $ix < (scalar @$hunks)-2 ? @$hunks[$ix+1..@$hunks] : ()) {\n+\t\t\tfor (@{$h->{TEXT}}) {\n+\t\t\t\tprint $fh $_;\n+\t\t\t}\n+\t\t}\n+\t\tif (!close $fh) {\n+\t\t\t# didn't apply cleanly\n+\t\t\twhile (1) {\n+\t\t\t\tprint colored $prompt_color, \"Your edited hunk does not apply. Edit again (saying \\\"no\\\" discards!) [y/n]? \";\n+\t\t\t\tmy $line = ;\n+\t\t\t\tif ($line =~ /^y/) {\n+\t\t\t\t\tredo EDIT;\n+\t\t\t\t}\n+\t\t\t\telsif ($line =~ /^n/) {\n+\t\t\t\t\treturn $hunks->[$ix];\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tif (1 < @newhunks) {\n+\t\t\tprint colored $header_color, \"Manually edited into \",\n+\t\t\tscalar(@newhunks), \" hunks.\\n\";\n+\t\t}\n+\t\treturn @newhunks;\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +945,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -885,6 +1050,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1115,11 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tsplice @hunk, $ix, 1, edit_hunk_loop($head, \\@hunk, $ix);\n+\t\t\t\t$num = scalar @hunk;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n-- \n1.5.6.rc0.159.g710c6\n"},{"id":"78289","messageId":"200806011650.02245.trast@student.ethz.ch","threadId":"13641","inReplyTo":"200806010241.51464.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2.1","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-01T14:50:00Z","receivedAt":"2008-06-01T14:50:00Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Adds a new option 'e' to the 'add -p' command loop that lets you\nedit the current hunk in your favourite editor.\n---\n\nThomas Rast wrote:\n>\n> - Perl's autovivification is out to get me. I've fixed a few, but\n> there are probably still bugs.\n\nI also tripped over Perl's idea of array slicing. This fixes an\noff-by-one in the hunks forwared to apply --check. The diff between\nthis and the previous patch is\n\n# @@ -908,7 +908,7 @@ sub edit_hunk_loop {\n \t\tfor my $h ($head,\n \t\t\t $ix > 0 ? @$hunks[0..$ix-1] : (),\n \t\t\t @newhunks,\n-\t\t\t $ix < (scalar @$hunks)-2 ? @$hunks[$ix+1..@$hunks] : ()) {\n+\t\t\t $ix < (scalar @$hunks)-1 ? @$hunks[$ix+1..@$hunks-1] : ()) {\n \t\t\tfor (@{$h->{TEXT}}) {\n \t\t\t\tprint $fh $_;\n \t\t\t}\n\nAs a side note, what's the \"right\" way to deal with this situation of\npatch improvements? I made a normal commit chain on a side branch to\nkeep the history, but format-patch wants to make that into two mails,\nso I had to rebuild the message format from diff -p --stat.\n\nRegards\nThomas\n\n\n git-add--interactive.perl | 171 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 171 insertions(+), 0 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..a3b2c75 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -18,6 +18,18 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n+my ($diff_old_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.old', 'red'),\n+\t) : ();\n+my ($diff_new_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.new', 'green'),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -770,6 +782,158 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub edit_hunk_manually {\n+\tmy @oldtext;\n+\tfor (@_) {\n+\t\tpush @oldtext, @{$_->{TEXT}};\n+\t}\n+\n+\t# use a .diff file to help editors with highlighting\n+\tmy $editpath = $repo->repo_path() . \"/ADDP_HUNK_EDIT.diff\";\n+\tmy $fh;\n+\topen $fh, '>', $editpath\n+\t\tor die \"failed to open hunk edit file for writing: \" . $!;\n+\tprint $fh <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $editpath);\n+\n+\topen $fh, '<', $editpath\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext;\n+\twhile (<$fh>) {\n+\t\tpush (@newtext, $_) unless /^#/ || /^$/;\n+\t}\n+\tclose $fh;\n+\tmy @heads = ();\n+\tmy ($o_ofs, $n_ofs);\n+\tmy $o_cnt = 0;\n+\tmy $n_cnt = 0;\n+\tmy ($guess_o_ofs, undef, $guess_n_ofs, undef) = parse_hunk_header($oldtext[0]);\n+\tfor (my $i = 0; $i < @newtext; $i++) {\n+\t\tif ((scalar @heads) == 0 && $newtext[$i] =~ /^[ +-]/) {\n+\t\t\tsplice @newtext, $i, 0, $oldtext[0];\n+\t\t\tpush @heads, $i;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^ /) {\n+\t\t\t$o_cnt++;\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^-/) {\n+\t\t\t$o_cnt++;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^\\+/) {\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t\telsif ($newtext[$i] =~ /^@/) {\n+\t\t\tif (@heads > 0) {\n+\t\t\t\t# fix up the previous header first\n+\t\t\t\t($o_ofs, undef, $n_ofs, undef)\n+\t\t\t\t\t= parse_hunk_header($newtext[$heads[-1]]);\n+\t\t\t\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n+\t\t\t\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n+\t\t\t\t$newtext[$heads[-1]] = (\n+\t\t\t\t\t\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n+\t\t\t\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n+\t\t\t\t\t. \" @@\\n\");\n+\t\t\t\t$guess_o_ofs = $o_ofs + $o_cnt;\n+\t\t\t\t$guess_n_ofs = $n_ofs + $n_cnt;\n+\t\t\t}\n+\t\t\t$o_cnt = 0;\n+\t\t\t$n_cnt = 0;\n+\t\t\tpush @heads, $i;\n+\t\t}\n+\t}\n+\t($o_ofs, undef, $n_ofs, undef)\n+\t\t= parse_hunk_header($newtext[$heads[-1]]);\n+\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n+\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n+\t$newtext[$heads[-1]] = (\n+\t\t\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n+\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n+\t\t. \" @@\\n\");\n+\n+\tpush @heads, (scalar @newtext);\n+\tmy (@hunks) = ();\n+\tfor (my $i = 0; $i < @heads-1; $i++) {\n+\t\tmy @hunktext = @newtext[$heads[$i]..$heads[$i+1]-1];\n+\t\tmy @hunkdisplay = ();\n+\t\tfor (@hunktext) {\n+\t\t\tif (/^@/) {\n+\t\t\t\tpush @hunkdisplay, (colored $fraginfo_color, $_);\n+\t\t\t}\n+\t\t\telsif (/^\\+/) {\n+\t\t\t\tpush @hunkdisplay, (colored $diff_new_color, $_);\n+\t\t\t}\n+\t\t\telsif (/^-/) {\n+\t\t\t\tpush @hunkdisplay, (colored $diff_old_color, $_);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tpush @hunkdisplay, (colored $diff_plain_color, $_);\n+\t\t\t}\n+\t\t}\n+\t\tpush @hunks, {TEXT => \\@hunktext, DISPLAY => \\@hunkdisplay};\n+\t}\n+\n+\treturn @hunks;\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $hunks, $ix) = @_;\n+\n+\tmy @newhunks = ($hunks->[$ix]);\n+\n+ EDIT:\n+\twhile (1) {\n+\t\t@newhunks = edit_hunk_manually(@newhunks);\n+\t\tmy $fh;\n+\t\topen $fh, '| git apply --cached --check';\n+\t\tfor my $h ($head,\n+\t\t\t $ix > 0 ? @$hunks[0..$ix-1] : (),\n+\t\t\t @newhunks,\n+\t\t\t $ix < (scalar @$hunks)-1 ? @$hunks[$ix+1..@$hunks-1] : ()) {\n+\t\t\tfor (@{$h->{TEXT}}) {\n+\t\t\t\tprint $fh $_;\n+\t\t\t}\n+\t\t}\n+\t\tif (!close $fh) {\n+\t\t\t# didn't apply cleanly\n+\t\t\twhile (1) {\n+\t\t\t\tprint colored $prompt_color, \"Your edited hunk does not apply. Edit again (saying \\\"no\\\" discards!) [y/n]? \";\n+\t\t\t\tmy $line = ;\n+\t\t\t\tif ($line =~ /^y/) {\n+\t\t\t\t\tredo EDIT;\n+\t\t\t\t}\n+\t\t\t\telsif ($line =~ /^n/) {\n+\t\t\t\t\treturn $hunks->[$ix];\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tif (1 < @newhunks) {\n+\t\t\tprint colored $header_color, \"Manually edited into \",\n+\t\t\tscalar(@newhunks), \" hunks.\\n\";\n+\t\t}\n+\t\treturn @newhunks;\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +945,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -885,6 +1050,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1115,11 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tsplice @hunk, $ix, 1, edit_hunk_loop($head, \\@hunk, $ix);\n+\t\t\t\t$num = scalar @hunk;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n-- \n1.5.6.rc0.159.g710c6.dirty\n"},{"id":"78290","messageId":"20080601151409.GA31769@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806011650.02245.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2.1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-01T15:14:10Z","receivedAt":"2008-06-01T15:14:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 01, 2008 at 04:50:00PM +0200, Thomas Rast wrote:\n\n> Adds a new option 'e' to the 'add -p' command loop that lets you\n> edit the current hunk in your favourite editor.\n\nI haven't had time to review your patch yet, but I'll hopefully get to\nit in the next day or so. In the meantime, a quick comment on this\nchange:\n\n> -\t\t\t $ix < (scalar @$hunks)-2 ? @$hunks[$ix+1..@$hunks] : ()) {\n> +\t\t\t $ix < (scalar @$hunks)-1 ? @$hunks[$ix+1..@$hunks-1] : ()) {\n\nAn easier and less error prone way to write \"@$hunks-1\" is \"$#{$hunks}\".\n\n> As a side note, what's the \"right\" way to deal with this situation of\n> patch improvements? I made a normal commit chain on a side branch to\n> keep the history, but format-patch wants to make that into two mails,\n> so I had to rebuild the message format from diff -p --stat.\n\nUsually you would squash your changes together into a single commit\nlocally using \"commit --amend\" or \"rebase -i\".\n\n-Peff\n"},{"id":"78685","messageId":"20080605014618.GA27381@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806010241.51464.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-05T01:46:18Z","receivedAt":"2008-06-05T01:46:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 01, 2008 at 02:41:48AM +0200, Thomas Rast wrote:\n\n> The current implementation rejects edits that break the (whole) patch\n> (thanks Junio for pointing out --check...), but it does help the user\n> in one aspect: @ lines are edited to reflect their hunk contents, even\n> inferring the starting line numbers from the last hunk if they are\n> missing. This means you can insert a line consisting just of an @,\n> and it will silently be fixed to start a new hunk at that point.\n\nOverall, I think this patch is reasonable. I have a few complaints, but\nI'm not sure that there are better alternatives. See below.\n\n> - Perhaps the instructions should go to the bottom, but then the odds\n> are they would not be visible in many cases.\n\nThey are at the bottom in \"rebase -i\", and I think there was much\ndiscussion that led to that.\n\n> - Should I try to come up with a unique filename to support concurrent\n> edits? (git-commit doesn't...)\n\nI think it makes sense to; there isn't really a downside.\n\n> +my ($diff_plain_color) =\n> +\t$diff_use_color ? (\n> +\t\t$repo->get_color('color.diff.plain', ''),\n> +\t) : ();\n> +my ($diff_old_color) =\n> +\t$diff_use_color ? (\n> +\t\t$repo->get_color('color.diff.old', 'red'),\n> +\t) : ();\n> +my ($diff_new_color) =\n> +\t$diff_use_color ? (\n> +\t\t$repo->get_color('color.diff.new', 'green'),\n> +\t) : ();\n\nYuck. I was hoping we could avoid doing anything with diff colors here.\nI had envisioned this as the user editing the hunk, and we would just\nmatch up the lines. That is, they would only be deleting lines, and we\nwould simply read back each line, comparing against the original hunk.\nAny lines which were removed by the user would become no-ops in the\nhunk.\n\nBut that being said, I think your approach is much more powerful, since\nyou are allowing arbitrary editing of the hunk (as much as that is\npossible). And in that case, I think the re-coloring is necessary, since\nwe can't just match the lines up to their previously colored versions\nanymore.\n\n> +sub edit_hunk_manually {\n> +\tmy @oldtext;\n> +\tfor (@_) {\n> +\t\tpush @oldtext, @{$_->{TEXT}};\n> +\t}\n\nI think this might be more readable as:\n\n my @oldtext = map { @{$_->{TEXT}} } @_;\n\nI like to declare and initialize values in the same expression, so you\nknow that they have that value in all code paths. But that is a nit.\n\n> +\t# use a .diff file to help editors with highlighting\n> +\tmy $editpath = $repo->repo_path() . \"/ADDP_HUNK_EDIT.diff\";\n> +\tmy $fh;\n> +\topen $fh, '>', $editpath\n> +\t\tor die \"failed to open hunk edit file for writing: \" . $!;\n> +\tprint $fh < +# MANUAL HUNK EDIT MODE\n> +#\n> +# You can change the hunk to your heart's content, but it will be\n> +# refused if the end result (the entire patch including your edited\n> +# hunk) does not apply cleanly.\n\nThis paragraph can probably be omitted if we want to make the\ninstructions smaller (but I think just putting it at the end will be\nfine).\n\n> +# To remove '-' lines, make them ' ' lines (context).\n> +# To remove '+' lines, delete them.\n\nThis makes perfect sense if you understand editing diffs, but is maybe a\nlittle scary for less clueful users. I wonder if we could detect removed\n'-' lines and munge them into context lines on behalf of the user.\nThough that requires comparing old and new, which kind of goes against\nthe feature's \"do whatever you want, and we will try to apply it\" model.\n\nSo perhaps manual hunk editing is simply something for advanced users\nwho are comfortable with the patch format.\n\n> +# Empty lines and lines starting with # will be removed.\n\nWhat about lines starting with characters besides -, +, space, or @?\nThey will normally be ignored by diff.\n\n> +\tmy $editor = $ENV{GIT_EDITOR} || $repo->config(\"core.editor\")\n> +\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n> +\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $editpath);\n\nI think there are a few other perl spots that figure out the editor.\nMaybe this should be refactored into Git.pm?\n\n> +\topen $fh, '<', $editpath\n> +\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n> +\tmy @newtext;\n> +\twhile (<$fh>) {\n> +\t\tpush (@newtext, $_) unless /^#/ || /^$/;\n> +\t}\n> +\tclose $fh;\n\nAgain, I might do:\n\n my @newtext = grep { !/^$/ && !/^#/ } <$fh>;\n\nThough I wonder if what you really want is:\n\n grep { /^[@ +-]/ }\n\nsince other lines are generally ignored in patches.\n\n> +\tmy @heads = ();\n> +\tmy ($o_ofs, $n_ofs);\n> +\tmy $o_cnt = 0;\n> +\tmy $n_cnt = 0;\n> +\tmy ($guess_o_ofs, undef, $guess_n_ofs, undef) = parse_hunk_header($oldtext[0]);\n> +\tfor (my $i = 0; $i < @newtext; $i++) {\n> +\t\tif ((scalar @heads) == 0 && $newtext[$i] =~ /^[ +-]/) {\n> +\t\t\tsplice @newtext, $i, 0, $oldtext[0];\n> +\t\t\tpush @heads, $i;\n> +\t\t}\n\nYou can just say \"@heads == 0\" (or !@heads) here; the \"==\" implies\nscalar context.\n\nSo it looks like we are restoring the old hunk header if they deleted it\nfor some reason. I guess this is a reasonable precaution. However, it\ntook me a minute to figure out what was going on. Maybe a comment is in\norder.\n\n> +\t\telsif ($newtext[$i] =~ /^@/) {\n> +\t\t\tif (@heads > 0) {\n> +\t\t\t\t# fix up the previous header first\n> +\t\t\t\t($o_ofs, undef, $n_ofs, undef)\n> +\t\t\t\t\t= parse_hunk_header($newtext[$heads[-1]]);\n> +\t\t\t\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n> +\t\t\t\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n> +\t\t\t\t$newtext[$heads[-1]] = (\n> +\t\t\t\t\t\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n> +\t\t\t\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n> +\t\t\t\t\t. \" @@\\n\");\n> +\t\t\t\t$guess_o_ofs = $o_ofs + $o_cnt;\n> +\t\t\t\t$guess_n_ofs = $n_ofs + $n_cnt;\n> +\t\t\t}\n> +\t\t\t$o_cnt = 0;\n> +\t\t\t$n_cnt = 0;\n> +\t\t\tpush @heads, $i;\n> +\t\t}\n> +\t}\n\nI think this is right, but it would be nice to have a few testcases in\nt3701 with some basic sanity checks and covering the border cases.\n\n> +\t($o_ofs, undef, $n_ofs, undef)\n> +\t\t= parse_hunk_header($newtext[$heads[-1]]);\n> +\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n> +\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n> +\t$newtext[$heads[-1]] = (\n> +\t\t\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n> +\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n> +\t\t. \" @@\\n\");\n\nHmm. Repeating this chunk is a bit ugly. I wonder if the whole loop\nmight be a bit more readable in two sections: first parse into a list of\nhunks, and then fixup each hunk.\n\n> +\tpush @heads, (scalar @newtext);\n\nIs that right? Before, heads referred to the \"$i\" of our loop; but\n\"scalar @newtext\" is one _past_ the final element.\n\n> +\tmy (@hunks) = ();\n> +\tfor (my $i = 0; $i < @heads-1; $i++) {\n\nA nicer way of spelling this is:\n\n $i < $#heads\n\n> +\t\tmy @hunktext = @newtext[$heads[$i]..$heads[$i+1]-1];\n\nAgain, I think this might be more clear if you munged newtext into its\nconstituent hunks.\n\n> +\t\tmy @hunkdisplay = ();\n> +\t\tfor (@hunktext) {\n> +\t\t\tif (/^@/) {\n> +\t\t\t\tpush @hunkdisplay, (colored $fraginfo_color, $_);\n> +\t\t\t}\n> +\t\t\telsif (/^\\+/) {\n> +\t\t\t\tpush @hunkdisplay, (colored $diff_new_color, $_);\n> +\t\t\t}\n> +\t\t\telsif (/^-/) {\n> +\t\t\t\tpush @hunkdisplay, (colored $diff_old_color, $_);\n> +\t\t\t}\n> +\t\t\telse {\n> +\t\t\t\tpush @hunkdisplay, (colored $diff_plain_color, $_);\n> +\t\t\t}\n> +\t\t}\n\nAgain, perhaps map instead of a series of pushes is more readable:\n\n my @hunkdisplay = map {\n colored\n /^@/ ? $fraginfo_color :\n /^\\+/ ? $diff_new_color :\n /^-/ ? $diff_old_color :\n $diff_plain_color,\n $_;\n } @hunktext;\n\n> +\t\tmy $fh;\n> +\t\topen $fh, '| git apply --cached --check';\n> +\t\tfor my $h ($head,\n> +\t\t\t $ix > 0 ? @$hunks[0..$ix-1] : (),\n> +\t\t\t @newhunks,\n> +\t\t\t $ix < (scalar @$hunks)-2 ? @$hunks[$ix+1..@$hunks] : ()) {\n> +\t\t\tfor (@{$h->{TEXT}}) {\n> +\t\t\t\tprint $fh $_;\n> +\t\t\t}\n> +\t\t}\n> +\t\tif (!close $fh) {\n> +\t\t\t# didn't apply cleanly\n\nAnother style nit: I think the edit_hunk_loop function would be much\nmore readable if this chunk became 'sub diff_applies'.\n\n> +\t\t\twhile (1) {\n> +\t\t\t\tprint colored $prompt_color, \"Your edited hunk does not apply. Edit again (saying \\\"no\\\" discards!) [y/n]? \";\n> +\t\t\t\tmy $line = ;\n> +\t\t\t\tif ($line =~ /^y/) {\n\nProbably should be \"/^y/i\".\n\n> +\t\t\t\telsif ($line =~ /^n/) {\n\nDitto.\n\nOverall, I think it looks reasonable. My complaints are:\n\n - Minor fixups and style comments. All of my style comments are \"I\n would have done it as...\" and not \"Oh God, this is horrible\" so I\n don't think any block acceptance.\n\n - The interface is a little less user-friendly, but a little more\n powerful than I was envisioning. However, nobody has said much, so I\n think the best thing might be to cook in next for a while and see if\n people find it useful.\n\n - I'd really like to see a few testcases before that, though.\n\n-Peff\n"},{"id":"78715","messageId":"200806050954.13244.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080605014618.GA27381@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-05T07:53:54Z","receivedAt":"2008-06-05T07:53:54Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> \n> But that being said, I think your approach is much more powerful, since\n> you are allowing arbitrary editing of the hunk (as much as that is\n> possible).\n[...]\n> So perhaps manual hunk editing is simply something for advanced users\n> who are comfortable with the patch format.\n\nI'll think about this for a while. Somehow I don't like the idea of\nediting the actual patch _contents_ for the user, meaning that what\nthe user needs to edit his patch into is not what we are going to\napply.\n\nOn the other hand it would be just as powerful. Manually splitting a\nhunk is, in the general case, only possible in \"my\" scheme. However,\nto make any difference, you later have to answer 'n' to some of the\nsub-hunks. So in \"your\" scheme, you could just have deleted the lines\nin question.\n\nOf course, manually editing the '+' lines or even introducing new\nstuff into the patch is not possible, but then you should probably\nhave edited the actual file contents, not the patch. (The working\ndirectory will still have the old version, so if you commit after add\n-p, it will appear to undo part of the last commit.)\n\nThen again, if this is just about editor convenience, maybe make a\nmacro that toggles between '-'/'+' and ' '/'#', respectively? (Which\nmakes me wonder if it would be useful to keep the '#' lines, minus the\nhelp message, around until the final git-apply in case you change your\nmind and re-edit.)\n\n> > +# Empty lines and lines starting with # will be removed.\n> \n> What about lines starting with characters besides -, +, space, or @?\n> They will normally be ignored by diff.\n\nDiff doesn't really have a say in this, does it? And looking in\nbuiltin-apply.c:\n\n\tswitch (first) { // line 1858 as of v1.5.6-rc1-122-g3859f95\n\tcase '\\n':\n\t\t/* Newer GNU diff, empty context line */\n\t\t// actual work snipped\n\t\tbreak;\n\t// cases ' ', '+', '-' also covered\n\tdefault:\n\t\tif (apply_verbosely)\n\t\t\terror(\"invalid start of line: '%c'\", first);\n\t\treturn -1;\n\nso it appears invalid lines are actually not ignored, but abort hunk\nprocessing. While the error checking will be handled by apply\n--check, I don't think it would be a good idea to silently drop all\nother lines from the edit, as they probably indicate user error.\n\nOn the other hand, this also shows that dropping empty lines is\nwrong...\n\n> - Minor fixups and style comments. All of my style comments are \"I\n> would have done it as...\" and not \"Oh God, this is horrible\" so I\n> don't think any block acceptance.\n\nFrom a Perl POV, they probably _are_ horrible. I'm just not used to\nthe idioms, and tend to fall for semantic differences to Python as\nwell.\n\nThank you for the very thorough review! I'll improve the patch\naccordingly.\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n\n\n"},{"id":"78718","messageId":"20080605081126.GA16416@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806050954.13244.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-05T08:11:28Z","receivedAt":"2008-06-05T08:11:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2008 at 09:53:54AM +0200, Thomas Rast wrote:\n\n> On the other hand it would be just as powerful. Manually splitting a\n> hunk is, in the general case, only possible in \"my\" scheme. However,\n> to make any difference, you later have to answer 'n' to some of the\n> sub-hunks. So in \"your\" scheme, you could just have deleted the lines\n> in question.\n\nSorry, I've gotten lost in which is mine and which is yours. Yours is\nthe original patch or this patch? Mine is the proposal that spawned this\npatch, or my comments on this patch?\n\n> > What about lines starting with characters besides -, +, space, or @?\n> > They will normally be ignored by diff.\n> \n> Diff doesn't really have a say in this, does it? And looking in\n> builtin-apply.c:\n\nSorry, I meant to say \"patch\" here. But even so, I'm still wrong.\nArbitrary text is OK between _diffs_, but not between _hunks_. And by\ndefinition, this format is hunks inside a single diff.\n\n> \tdefault:\n> \t\tif (apply_verbosely)\n> \t\t\terror(\"invalid start of line: '%c'\", first);\n> \t\treturn -1;\n\nRight, so we signal the end of diff, and any hunks afterwards are \"patch\nfragment without header\".\n\n> so it appears invalid lines are actually not ignored, but abort hunk\n> processing. While the error checking will be handled by apply\n> --check, I don't think it would be a good idea to silently drop all\n> other lines from the edit, as they probably indicate user error.\n\nGood point. There's really no reason for such lines to occur, so the\nbest thing is probably to notice the situation and let the user re-edit.\n\n> On the other hand, this also shows that dropping empty lines is\n> wrong...\n\nHmm. Yes, it looks like they are significant. Given that they are\ngenerated by a particular version of GNU diff, and that these should be\n\"git diff\"-generated patches, we aren't likely to encounter them. But\nprobably we should just leave them as-is, and let \"git apply --check\" do\nwhat it will with them.\n\n> From a Perl POV, they probably _are_ horrible. I'm just not used to\n> the idioms, and tend to fall for semantic differences to Python as\n> well.\n\nHeh. The Perl style in git is a bit tricky; it is such a small portion\nof the code base, and there is not a real sense of ownership, so we seem\nto have several different styles.\n\n> Thank you for the very thorough review! I'll improve the patch\n> accordingly.\n\nGreat. I think this is a worthwhile feature, so please keep at it.\n\n-Peff\n"},{"id":"78719","messageId":"7vprqw2t64.fsf@gitster.siamese.dyndns.org","threadId":"13641","inReplyTo":"20080605014618.GA27381@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-05T08:16:19Z","receivedAt":"2008-06-05T08:16:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So perhaps manual hunk editing is simply something for advanced users\n> who are comfortable with the patch format.\n\nExactly. To them, \"git diff >patch && vi patch && git apply --cached > +# Empty lines and lines starting with # will be removed.\n>\n> What about lines starting with characters besides -, +, space, or @?\n> They will normally be ignored by diff.\n\nBeware that a totally empty line is the same as an empty context line \"SP LF\".\n\nFor the rest of your comments, I agree with the Perl style (use of map and\ngrep instead of repeated push in loops). The end user input, what the\ncode needs to parse and accept, can screw you up royally and your parsing\nneeds to be careful, and the code looks fragile.\n"},{"id":"78724","messageId":"20080605085605.GA16624@sigill.intra.peff.net","threadId":"13641","inReplyTo":"7vprqw2t64.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-05T08:56:06Z","receivedAt":"2008-06-05T08:56:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2008 at 01:16:19AM -0700, Junio C Hamano wrote:\n\n> > So perhaps manual hunk editing is simply something for advanced users\n> > who are comfortable with the patch format.\n> \n> Exactly. To them, \"git diff >patch && vi patch && git apply --cached would likely to be much handier, quicker and a more familiar way. That is\n> one of the reasons I somewhat doubt that we would want to have this patch.\n\nI'm not so sure. I think this patch buys you two things:\n\n 1. It munges the hunk headers as appropriate.\n\n 2. It fits more naturally into the \"git add -p\" workflow. That is, I\n don't start by saying \"I need to edit this patch.\" Instead, I am\n staging bits of changes, and looking at a hunk I say \"Oh, this needs\n to be tweaked\"; this patch provides an easy route from looking at\n the hunk to editing the hunk.\n\n Granted, 99% of the time my tweak is \"these adjacent lines are\n actually unrelated, so split them into two hunks.\" So maybe that is\n an argument for something like Thomas' original patch.\n\n> For the rest of your comments, I agree with the Perl style (use of map and\n> grep instead of repeated push in loops). The end user input, what the\n> code needs to parse and accept, can screw you up royally and your parsing\n> needs to be careful, and the code looks fragile.\n\nI think there are two extremes here:\n\n 1. We provide a very limited interface. E.g., you pick a line to\n arbitrarily split the hunk.\n\n 2. We provide a very flexible interface. You get dumped in your editor\n with the hunk text, and we try to apply the result. Powerful, but\n easy to shoot yourself in the foot.\n\nAnd there is probably room for both. But I think it is important not to\nfall into the middle, where we have a hard-to-use but featureless\ninterface. That is, if we dump the user in an editor, doing anything\nbesides (2) doesn't make sense; trying to infer what happened in the\nuser's editing session is a waste of time.\n\nSo I think we need to be not so much careful in parsing, as accepting.\nOr we need to dump the idea of letting the user make arbitrary edits.\n\n-Peff\n"},{"id":"78726","messageId":"200806051102.16965.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080529185808.GA2140@sigill.intra.peff.net","subject":"Re: [PATCH] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-05T09:02:05Z","receivedAt":"2008-06-05T09:02:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"You wrote:\n> On Thu, Jun 05, 2008 at 09:53:54AM +0200, Thomas Rast wrote:\n> \n> > On the other hand it would be just as powerful. Manually splitting a\n> > hunk is, in the general case, only possible in \"my\" scheme. However,\n> > to make any difference, you later have to answer 'n' to some of the\n> > sub-hunks. So in \"your\" scheme, you could just have deleted the lines\n> > in question.\n> \n> Sorry, I've gotten lost in which is mine and which is yours. Yours is\n> the original patch or this patch? Mine is the proposal that spawned this\n> patch, or my comments on this patch?\n\nI should have provided a definition, sorry. By \"your\" scheme, I meant\nthe one where one is restricted to deleting existing lines, and thus\ndeletes '-' lines to disable them; by \"my\" scheme, where one is free\nto edit and changes '-' lines to context to disable them.\n\nAt least it's my current understanding that, under the above\ndefinition, \"your\" scheme is what you actually proposed in\n http://www.spinics.net/lists/git/msg67478.html\n [why doesn't Google find gmane?]\nbut I misunderstood and implemented \"my\" scheme.\n\n[Let's just forget about the original patch at the top of the thread;\nin retrospect, it _is_ clunky, and the UI didn't get any replies in\nfavour.]\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n"},{"id":"78727","messageId":"200806051105.02802.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080605081126.GA16416@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-05T09:04:58Z","receivedAt":"2008-06-05T09:04:58Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Sorry, I sent it in reply to the wrong message accidentally.\n\n(Somehow it seems rather ironic that I'm trying to implement a feature\nthat provides plenty of live bullets, yet keep shooting myself with\nthe _MUA_.)\n\nYou wrote:\n> On Thu, Jun 05, 2008 at 09:53:54AM +0200, Thomas Rast wrote:\n> \n> > On the other hand it would be just as powerful. Manually splitting a\n> > hunk is, in the general case, only possible in \"my\" scheme. However,\n> > to make any difference, you later have to answer 'n' to some of the\n> > sub-hunks. So in \"your\" scheme, you could just have deleted the lines\n> > in question.\n> \n> Sorry, I've gotten lost in which is mine and which is yours. Yours is\n> the original patch or this patch? Mine is the proposal that spawned this\n> patch, or my comments on this patch?\n\nI should have provided a definition, sorry. By \"your\" scheme, I meant\nthe one where one is restricted to deleting existing lines, and thus\ndeletes '-' lines to disable them; by \"my\" scheme, where one is free\nto edit and changes '-' lines to context to disable them.\n\nAt least it's my current understanding that, under the above\ndefinition, \"your\" scheme is what you actually proposed in\n http://www.spinics.net/lists/git/msg67478.html\n [why doesn't Google find gmane?]\nbut I misunderstood and implemented \"my\" scheme.\n\n[Let's just forget about the original patch at the top of the thread;\nin retrospect, it _is_ clunky, and the UI didn't get any replies in\nfavour.]\n\n- Thomas\n\n"},{"id":"78728","messageId":"20080605092016.GA16748@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806051105.02802.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-05T09:20:16Z","receivedAt":"2008-06-05T09:20:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2008 at 11:04:58AM +0200, Thomas Rast wrote:\n\n> (Somehow it seems rather ironic that I'm trying to implement a feature\n> that provides plenty of live bullets, yet keep shooting myself with\n> the _MUA_.)\n\nHeh.\n\n> I should have provided a definition, sorry. By \"your\" scheme, I meant\n> the one where one is restricted to deleting existing lines, and thus\n> deletes '-' lines to disable them; by \"my\" scheme, where one is free\n> to edit and changes '-' lines to context to disable them.\n\nOK, thanks for the clarification.\n\n> At least it's my current understanding that, under the above\n> definition, \"your\" scheme is what you actually proposed in\n> http://www.spinics.net/lists/git/msg67478.html\n> [why doesn't Google find gmane?]\n> but I misunderstood and implemented \"my\" scheme.\n\nFor the record, \"my\" scheme was only half thought-through, and I think I\nactually like \"your\" scheme better. Once we give the user an editor,\nrestricting them to a tiny subset of editor operations seems error-prone\nand annoying, since we have no way of enforcing those operations except\nto wait until after they edit and say \"oops, you did something bad.\"\n\n> [Let's just forget about the original patch at the top of the thread;\n> in retrospect, it _is_ clunky, and the UI didn't get any replies in\n> favour.]\n\nIt seems like Junio isn't all that keen on the raw patch-editing\ninterface. And even if we do like it, I think there is still room for a\nless error-prone but more restrictive feature that mere mortals can use.\nSo maybe there is a better interface yet.\n\nWhat about 'S' to do a \"line split\"; that is, take the current hunk, and\nanywhere there are adjacent changed lines, split them into their own\nhunks. I.e. the hunk,\n\n line 1\n +line 2\n -line 3\n line 4\n -line 5\n\nbecomes three hunks:\n\n line 1\n +line 2\n\n -line 3\n line 4\n\n -line 5\n\nand then we proceed as usual, staging or not each split hunk. It would\nbe clunky to separate one or two lines from a huge chunk (since you\nwould inadvertently split the huge chunk and have to stage each\nindividually). But in many cases you can split into smaller hunks first\nwith 's'.\n\n-Peff\n"},{"id":"78731","messageId":"200806051138.28889.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080605092016.GA16748@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-05T09:38:15Z","receivedAt":"2008-06-05T09:38:15Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> > [Let's just forget about the original patch at the top of the thread;\n> > in retrospect, it _is_ clunky, and the UI didn't get any replies in\n> > favour.]\n> \n> It seems like Junio isn't all that keen on the raw patch-editing\n> interface. And even if we do like it, I think there is still room for a\n> less error-prone but more restrictive feature that mere mortals can use.\n> So maybe there is a better interface yet.\n\nMaybe git gui could do a sort of \"toggle the lines I click\" interface.\nBut I don't know anything about Tk, or Tcl, or git gui :-(\n\n> What about 'S' to do a \"line split\"; that is, take the current hunk, and\n> anywhere there are adjacent changed lines, split them into their own\n> hunks.\n[...]\n> and then we proceed as usual, staging or not each split hunk. It would\n> be clunky to separate one or two lines from a huge chunk (since you\n> would inadvertently split the huge chunk and have to stage each\n> individually). But in many cases you can split into smaller hunks first\n> with 's'.\n\nNow I'm slightly confused.\n\nDoing it that way would be almost like my original patch\n\n http://www.spinics.net/lists/git/msg66971.html\n\nminus the numeric prefixes -- meaning that you have to say y/n to\n_every_ line in the patch, at least until all remaining hunks are the\nsame and you can answer the rest with a/d.\n\nExcept that it wouldn't work anyway, because git-apply refuses hunks\nthat have no context (even if just on one end). Unless given\n--unidiff-zero, but that apparently was one of the points of refusal\nin the thread Dscho linked earlier:\n\n http://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68127\n\nGranted, we could insert extra context and/or make sure the mentioned\ndata loss can never happen (it's probably prevented by 'add -p's own\nrecounting before the final apply), but the first would make the UI\neven more confusing and the second is potentially dangerous.\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n\n"},{"id":"78733","messageId":"20080605094613.GA17089@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806051138.28889.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-05T09:46:13Z","receivedAt":"2008-06-05T09:46:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2008 at 11:38:15AM +0200, Thomas Rast wrote:\n\n> Maybe git gui could do a sort of \"toggle the lines I click\" interface.\n> But I don't know anything about Tk, or Tcl, or git gui :-(\n\nThat would probably be a reasonable interface. But I don't use git-gui,\nso I personally am not interested in that. ;)\n\n> Now I'm slightly confused.\n> \n> Doing it that way would be almost like my original patch\n> \n> http://www.spinics.net/lists/git/msg66971.html\n> \n> minus the numeric prefixes -- meaning that you have to say y/n to\n> _every_ line in the patch, at least until all remaining hunks are the\n> same and you can answer the rest with a/d.\n\nHmm, I suppose it is. Sorry, I'm just trying to brainstorm a bit about\nother options, but I think that my proposal really isn't that good an\nidea.\n\n-Peff\n"},{"id":"78737","messageId":"alpine.DEB.1.00.0806051126060.21190@racer","threadId":"13641","inReplyTo":"20080605085605.GA16624@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T10:28:53Z","receivedAt":"2008-06-05T10:28:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 5 Jun 2008, Jeff King wrote:\n\n> I think there are two extremes here:\n> \n> 1. We provide a very limited interface. E.g., you pick a line to \n> arbitrarily split the hunk.\n> \n> 2. We provide a very flexible interface. You get dumped in your editor \n> with the hunk text, and we try to apply the result. Powerful, but \n> easy to shoot yourself in the foot.\n\nWe have a tradition of giving the users plenty of rope.\n\nAnd I actually like having the power at my finger tips. You would not \nbelieve how I enjoyed using \"git add -e\" to commit the final version of \nthat very patch.\n\nI, for one, will probably use this feature almost exclusively, \n_especially_ since I look at the diffs before committing _anyway_, and I \ncannot count the times when I had to go to another window to fix up a \nstupid debug message or typo. Now I will do that right away, and be done \nwith it. I can even stash the debug messages for future use!\n\nCiao,\nDscho\n"},{"id":"78755","messageId":"200806051438.12758.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080605014618.GA27381@sigill.intra.peff.net","subject":"[WIP PATCH v2] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-05T12:38:06Z","receivedAt":"2008-06-05T12:38:06Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Adds a new option 'e' to the 'add -p' command loop that lets you\nedit the current hunk in your favourite editor.\n\n---\n\nJeff King wrote:\n> Hmm. Repeating this chunk is a bit ugly. I wonder if the whole loop\n> might be a bit more readable in two sections: first parse into a list of\n> hunks, and then fixup each hunk.\n[...]\n> - Minor fixups and style comments. All of my style comments are \"I\n> would have done it as...\" and not \"Oh God, this is horrible\" so I\n> don't think any block acceptance.\n\nThis is the squashed patch, changed according to your comments, and\nwith the loop in question rewritten as you suggested. (It's indeed\nfar less obscure.)\n\nNot ready for commit yet because\n\n> - I'd really like to see a few testcases before that, though.\n\nis still missing.\n\n- Thomas\n\n\n git-add--interactive.perl | 164 ++++++++++++++++++++++++++++++++++++++++-----\n 1 files changed, 147 insertions(+), 17 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..5fb8402 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,7 @@\n \n use strict;\n use Git;\n+use File::Temp qw/tempfile/;\n \n my $repo = Git->repository();\n \n@@ -18,6 +19,18 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n+my ($diff_old_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.old', 'red'),\n+\t) : ();\n+my ($diff_new_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.new', 'green'),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -581,6 +594,13 @@ sub parse_hunk_header {\n \treturn ($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n }\n \n+sub format_hunk_header {\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;\n+\treturn (\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n+\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n+\t\t. \" @@\\n\");\n+}\n+\n sub split_hunk {\n \tmy ($text, $display) = @_;\n \tmy @split = ();\n@@ -667,11 +687,7 @@ sub split_hunk {\n \t\tmy $o_cnt = $hunk->{OCNT};\n \t\tmy $n_cnt = $hunk->{NCNT};\n \n-\t\tmy $head = (\"@@ -$o_ofs\" .\n-\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t\t \" +$n_ofs\" .\n-\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t\t \" @@\\n\");\n+\t\tmy $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n \t\tmy $display_head = $head;\n \t\tunshift @{$hunk->{TEXT}}, $head;\n \t\tif ($diff_use_color) {\n@@ -741,11 +757,7 @@ sub merge_hunk {\n \t\t}\n \t\tpush @line, $line;\n \t}\n-\tmy $head = (\"@@ -$o0_ofs\" .\n-\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t \" +$n0_ofs\" .\n-\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t \" @@\\n\");\n+\tmy $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);\n \t@{$prev->{TEXT}} = ($head, @line);\n }\n \n@@ -770,6 +782,122 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub edit_hunk_manually {\n+\tmy @oldtext = map { @{$_->{TEXT}} } @_;\n+\n+\tmy ($fh, $editpath) = tempfile($repo->repo_path() . \"/git-hunk-edit.XXXXXX\",\n+\t\t\t\t SUFFIX => \".diff\", UNLINK => 0);\n+\tprint $fh \"# Manual hunk edit mode -- see bottom for a quick guide\\n\";\n+\tprint $fh @oldtext;\n+\tprint $fh <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $editpath);\n+\n+\topen $fh, '<', $editpath\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext = grep { !/^#/ } <$fh>;\n+\tclose $fh;\n+\tunlink(glob($editpath . \"*\"));\n+\t# Reinsert the first hunk header if the user accidentally deleted it\n+\tif ($newtext[0] !~ /^@/) {\n+\t\tsplice @newtext, 0, 0, $oldtext[0];\n+\t}\n+\t# Split into hunks\n+\tmy @hunktexts = ();\n+\tmy $curhunk = [];\n+\tfor (@newtext) {\n+\t\tif (/^@/ && @{$curhunk} > 0) {\n+\t\t\tpush @hunktexts, $curhunk;\n+\t\t\t$curhunk = [];\n+\t\t}\n+\t\tpush @{$curhunk}, $_;\n+\t}\n+\tpush @hunktexts, $curhunk;\n+\t# Fix the hunk headers\n+\tmy ($guess_o_ofs, undef, $guess_n_ofs, undef) = parse_hunk_header($oldtext[0]);\n+\tfor my $hunk (@hunktexts) {\n+\t\tmy ($o_ofs, undef, $n_ofs, undef) = parse_hunk_header($hunk->[0]);\n+\t\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n+\t\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n+\t\tmy $plus_cnt = grep /^\\+/, @{$hunk};\n+\t\tmy $minus_cnt = grep /^-/, @{$hunk};\n+\t\tmy $context_cnt = grep { /^ / || /^$/ } @{$hunk};\n+\t\tmy $o_cnt = $context_cnt + $minus_cnt;\n+\t\tmy $n_cnt = $context_cnt + $plus_cnt;\n+\t\t$hunk->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\t\t$guess_o_ofs = $o_ofs + $o_cnt;\n+\t\t$guess_n_ofs = $n_ofs + $n_cnt;\n+\t}\n+\t# Recolor the hunks\n+\tmy (@hunks) = ();\n+\tfor my $hunk (@hunktexts) {\n+\t\tmy @hunkdisplay = map {\n+\t\t\tcolored((/^@/ ? $fraginfo_color :\n+\t\t\t\t /^\\+/ ? $diff_new_color :\n+\t\t\t\t /^-/ ? $diff_old_color :\n+\t\t\t\t $diff_plain_color),\n+\t\t\t\t$_);\n+\t\t} @{$hunk};\n+\t\tpush @hunks, {TEXT => $hunk, DISPLAY => \\@hunkdisplay};\n+\t}\n+\n+\treturn @hunks;\n+}\n+\n+sub diff_applies {\n+\tmy $fh;\n+\topen $fh, '| git apply --cached --check';\n+\tfor my $h (@_) {\n+\t\tprint $fh @{$h->{TEXT}};\n+\t}\n+\treturn close $fh;\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $hunks, $ix) = @_;\n+\n+\tmy @newhunks = ($hunks->[$ix]);\n+\n+ EDIT:\n+\twhile (1) {\n+\t\t@newhunks = edit_hunk_manually(@newhunks);\n+\t\tif (!diff_applies($head, @$hunks[0..$ix-1], @newhunks,\n+\t\t\t\t @$hunks[$ix+1..$#{$hunks}])) {\n+\t\t\twhile (1) {\n+\t\t\t\tprint colored $prompt_color, 'Your edited hunk does not apply. Edit again (saying \"no\" discards!) [y/n]? ';\n+\t\t\t\tmy $line = ;\n+\t\t\t\tif ($line =~ /^y/i) {\n+\t\t\t\t\tredo EDIT;\n+\t\t\t\t}\n+\t\t\t\telsif ($line =~ /^n/i) {\n+\t\t\t\t\treturn $hunks->[$ix];\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tif (1 < @newhunks) {\n+\t\t\tprint colored $header_color, \"Manually edited into \",\n+\t\t\tscalar(@newhunks), \" hunks.\\n\";\n+\t\t}\n+\t\treturn @newhunks;\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +909,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -885,6 +1014,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1079,11 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tsplice @hunk, $ix, 1, edit_hunk_loop($head, \\@hunk, $ix);\n+\t\t\t\t$num = scalar @hunk;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n@@ -985,13 +1120,8 @@ sub patch_update_file {\n \t\telse {\n \t\t\tif ($n_lofs) {\n \t\t\t\t$n_ofs += $n_lofs;\n-\t\t\t\t$text->[0] = (\"@@ -$o_ofs\" .\n-\t\t\t\t\t (($o_cnt != 1)\n-\t\t\t\t\t ? \",$o_cnt\" : '') .\n-\t\t\t\t\t \" +$n_ofs\" .\n-\t\t\t\t\t (($n_cnt != 1)\n-\t\t\t\t\t ? \",$n_cnt\" : '') .\n-\t\t\t\t\t \" @@\\n\");\n+\t\t\t\t$text->[0] = format_hunk_header($o_ofs, $o_cnt,\n+\t\t\t\t\t\t\t\t$n_ofs, $n_cnt);\n \t\t\t}\n \t\t\tfor (@$text) {\n \t\t\t\tpush @result, $_;\n--\n1.5.6.rc1.137.g537d1\n\n"},{"id":"78862","messageId":"20080606051026.GA18257@sigill.intra.peff.net","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0806051126060.21190@racer","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-06T05:10:26Z","receivedAt":"2008-06-06T05:10:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2008 at 11:28:53AM +0100, Johannes Schindelin wrote:\n\n> We have a tradition of giving the users plenty of rope.\n> \n> And I actually like having the power at my finger tips. You would not \n> believe how I enjoyed using \"git add -e\" to commit the final version of \n> that very patch.\n\nI looked at your patch, and here are my complaints (versus what Thomas\nhas been working on):\n\n 1. You edit the whole diff, not just a hunk. Actually, I think this is\n probably not a big deal, as any decent editor lets you find the\n spot you're looking for pretty trivially.\n\n 2. It's not integrated into the git-add--interactive loop at all. That\n is, I don't start out saying \"I want to edit this diff.\" I look at\n the diff while staging with \"git add -p\" and say \"Oops, I need to\n edit this hunk.\" So I think it is better implemented as an \"e\"\n option in the hunk adding loop, with \"git add -e\" as a shortcut.\n Or maybe there is simply room for both (and \"git add -e\", rather\n than being a shortcut, just means \"do this on the _whole_ file\").\n\n-Peff\n"},{"id":"78869","messageId":"20080606060318.GE18257@sigill.intra.peff.net","threadId":"13641","inReplyTo":"20080606051026.GA18257@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-06T06:03:18Z","receivedAt":"2008-06-06T06:03:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 06, 2008 at 01:10:26AM -0400, Jeff King wrote:\n\n> 2. It's not integrated into the git-add--interactive loop at all. That\n> is, I don't start out saying \"I want to edit this diff.\" I look at\n> the diff while staging with \"git add -p\" and say \"Oops, I need to\n> edit this hunk.\" So I think it is better implemented as an \"e\"\n> option in the hunk adding loop, with \"git add -e\" as a shortcut.\n> Or maybe there is simply room for both (and \"git add -e\", rather\n> than being a shortcut, just means \"do this on the _whole_ file\").\n\nI wrote this after reading just your first patch, and it looks like\nyou've made much progress since. It seems like Thomas' patch could just\nget rid of all the recounting entirely, and just pass off the edited\nhunks to \"git apply --recount\". Which should make his patch much smaller\nand more straightforward.\n\n-Peff\n"},{"id":"78917","messageId":"alpine.DEB.1.00.0806061528270.1783@racer","threadId":"13641","inReplyTo":"20080606051026.GA18257@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T14:31:38Z","receivedAt":"2008-06-06T14:31:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Jun 2008, Jeff King wrote:\n\n> On Thu, Jun 05, 2008 at 11:28:53AM +0100, Johannes Schindelin wrote:\n> \n> > We have a tradition of giving the users plenty of rope.\n> > \n> > And I actually like having the power at my finger tips. You would not \n> > believe how I enjoyed using \"git add -e\" to commit the final version of \n> > that very patch.\n> \n> I looked at your patch, and here are my complaints (versus what Thomas\n> has been working on):\n> \n> 1. You edit the whole diff, not just a hunk. Actually, I think this is\n> probably not a big deal, as any decent editor lets you find the\n> spot you're looking for pretty trivially.\n\nActually, this is exactly what I wanted. I positively _hate_ all the key \npresses I have to go through until I am finally where I want to be. I \ncannot just search for a term and be done with it. Reminds me of \nWindows-like dialog based configurations.\n\n> 2. It's not integrated into the git-add--interactive loop at all. That\n> is, I don't start out saying \"I want to edit this diff.\" I look at\n> the diff while staging with \"git add -p\" and say \"Oops, I need to\n> edit this hunk.\" So I think it is better implemented as an \"e\"\n> option in the hunk adding loop, with \"git add -e\" as a shortcut.\n> Or maybe there is simply room for both (and \"git add -e\", rather\n> than being a shortcut, just means \"do this on the _whole_ file\").\n\nThis is very much on purpose. I do not like \"git add -i\" at all. It \nlimits my work unduly. That's why I tried to change the hunk editing in \ngit-gui once upon a time, but I never got round to fix that, and it does \nnot work well with ssh either.\n\nSo no, I do not want to use that perl script with that menu. I want to \nhave the raw diff in a raw editor, where I can change the things I need to \nchange.\n\nCiao,\nDscho\n"},{"id":"79178","messageId":"200806090018.48784.trast@student.ethz.ch","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0806061528270.1783@racer","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-08T22:18:45Z","receivedAt":"2008-06-08T22:18:45Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Schindelin wrote:\n> \n> On Fri, 6 Jun 2008, Jeff King wrote:\n> > 2. It's not integrated into the git-add--interactive loop at all. That\n> > is, I don't start out saying \"I want to edit this diff.\" I look at\n> > the diff while staging with \"git add -p\" and say \"Oops, I need to\n> > edit this hunk.\" So I think it is better implemented as an \"e\"\n> > option in the hunk adding loop, with \"git add -e\" as a shortcut.\n> > Or maybe there is simply room for both (and \"git add -e\", rather\n> > than being a shortcut, just means \"do this on the _whole_ file\").\n> \n> This is very much on purpose. I do not like \"git add -i\" at all. It \n> limits my work unduly. That's why I tried to change the hunk editing in \n> git-gui once upon a time, but I never got round to fix that, and it does \n> not work well with ssh either.\n> \n> So no, I do not want to use that perl script with that menu. I want to \n> have the raw diff in a raw editor, where I can change the things I need to \n> change.\n\nWhile there is obviously little point in trying to convince you, let\nme briefly explain why I still think it is useful:\n\nI usually run 'add -p' instead of adding specific files. \"Nodding\noff\" each hunk means that I get a last chance to review my changes,\nand perhaps skip some of them (possibly for a later commit). With my\nproposed editing feature, I can split hunks in the middle too. You\ncould of course argue that the right way to do this would be staring\nat 'git diff', or perhaps scrolling through the entire patch in your\n'add -e'. I just got used to doing the reviews during 'add -p'. As\nJeff said, by the time I decide to change a hunk, I might already be\nhalfway through the decisions and _definitely_ don't want to bail out\nand restart with 'add -e'.\n\nThat, and my Maple worksheets have a tendency to show up when I didn't\nwant to commit them, so I can just say 'd' there.\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n"},{"id":"79176","messageId":"200806090032.27516.trast@student.ethz.ch","threadId":"13641","inReplyTo":"200806051438.12758.trast@student.ethz.ch","subject":"[PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-08T22:32:26Z","receivedAt":"2008-06-08T22:32:26Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Subject: [PATCH] git-add--interactive: manual hunk editing mode\n\nAdds a new option 'e' to the 'add -p' command loop that lets you\nedit the current hunk in your favourite editor.\n\nSigned-off-by: Thomas Rast \n---\n\nOn top of the previous patch, this adds basic testing.\n\nCompared to the competing bikeshed\n http://article.gmane.org/gmane.comp.version-control.git/83894 \nthis integrates into the 'add -p' loop. I (still) prefer it because I\nuse 'add -p' a lot, thus having the editing feature at my fingertips.\n\n- Thomas\n\n\n git-add--interactive.perl | 164 +++++++++++++++++++++++++++++++++++++++-----\n t/t3701-add-interactive.sh | 57 +++++++++++++++\n 2 files changed, 204 insertions(+), 17 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..5fb8402 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,7 @@\n \n use strict;\n use Git;\n+use File::Temp qw/tempfile/;\n \n my $repo = Git->repository();\n \n@@ -18,6 +19,18 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n+my ($diff_old_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.old', 'red'),\n+\t) : ();\n+my ($diff_new_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.new', 'green'),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -581,6 +594,13 @@ sub parse_hunk_header {\n \treturn ($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n }\n \n+sub format_hunk_header {\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;\n+\treturn (\"@@ -$o_ofs\" . (($o_cnt != 1) ? \",$o_cnt\" : '')\n+\t\t. \" +$n_ofs\" . (($n_cnt != 1) ? \",$n_cnt\" : '')\n+\t\t. \" @@\\n\");\n+}\n+\n sub split_hunk {\n \tmy ($text, $display) = @_;\n \tmy @split = ();\n@@ -667,11 +687,7 @@ sub split_hunk {\n \t\tmy $o_cnt = $hunk->{OCNT};\n \t\tmy $n_cnt = $hunk->{NCNT};\n \n-\t\tmy $head = (\"@@ -$o_ofs\" .\n-\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t\t \" +$n_ofs\" .\n-\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t\t \" @@\\n\");\n+\t\tmy $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n \t\tmy $display_head = $head;\n \t\tunshift @{$hunk->{TEXT}}, $head;\n \t\tif ($diff_use_color) {\n@@ -741,11 +757,7 @@ sub merge_hunk {\n \t\t}\n \t\tpush @line, $line;\n \t}\n-\tmy $head = (\"@@ -$o0_ofs\" .\n-\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t \" +$n0_ofs\" .\n-\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t \" @@\\n\");\n+\tmy $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);\n \t@{$prev->{TEXT}} = ($head, @line);\n }\n \n@@ -770,6 +782,122 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub edit_hunk_manually {\n+\tmy @oldtext = map { @{$_->{TEXT}} } @_;\n+\n+\tmy ($fh, $editpath) = tempfile($repo->repo_path() . \"/git-hunk-edit.XXXXXX\",\n+\t\t\t\t SUFFIX => \".diff\", UNLINK => 0);\n+\tprint $fh \"# Manual hunk edit mode -- see bottom for a quick guide\\n\";\n+\tprint $fh @oldtext;\n+\tprint $fh <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $editpath);\n+\n+\topen $fh, '<', $editpath\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext = grep { !/^#/ } <$fh>;\n+\tclose $fh;\n+\tunlink(glob($editpath . \"*\"));\n+\t# Reinsert the first hunk header if the user accidentally deleted it\n+\tif ($newtext[0] !~ /^@/) {\n+\t\tsplice @newtext, 0, 0, $oldtext[0];\n+\t}\n+\t# Split into hunks\n+\tmy @hunktexts = ();\n+\tmy $curhunk = [];\n+\tfor (@newtext) {\n+\t\tif (/^@/ && @{$curhunk} > 0) {\n+\t\t\tpush @hunktexts, $curhunk;\n+\t\t\t$curhunk = [];\n+\t\t}\n+\t\tpush @{$curhunk}, $_;\n+\t}\n+\tpush @hunktexts, $curhunk;\n+\t# Fix the hunk headers\n+\tmy ($guess_o_ofs, undef, $guess_n_ofs, undef) = parse_hunk_header($oldtext[0]);\n+\tfor my $hunk (@hunktexts) {\n+\t\tmy ($o_ofs, undef, $n_ofs, undef) = parse_hunk_header($hunk->[0]);\n+\t\t$o_ofs = $guess_o_ofs unless defined $o_ofs;\n+\t\t$n_ofs = $guess_n_ofs unless defined $n_ofs;\n+\t\tmy $plus_cnt = grep /^\\+/, @{$hunk};\n+\t\tmy $minus_cnt = grep /^-/, @{$hunk};\n+\t\tmy $context_cnt = grep { /^ / || /^$/ } @{$hunk};\n+\t\tmy $o_cnt = $context_cnt + $minus_cnt;\n+\t\tmy $n_cnt = $context_cnt + $plus_cnt;\n+\t\t$hunk->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\t\t$guess_o_ofs = $o_ofs + $o_cnt;\n+\t\t$guess_n_ofs = $n_ofs + $n_cnt;\n+\t}\n+\t# Recolor the hunks\n+\tmy (@hunks) = ();\n+\tfor my $hunk (@hunktexts) {\n+\t\tmy @hunkdisplay = map {\n+\t\t\tcolored((/^@/ ? $fraginfo_color :\n+\t\t\t\t /^\\+/ ? $diff_new_color :\n+\t\t\t\t /^-/ ? $diff_old_color :\n+\t\t\t\t $diff_plain_color),\n+\t\t\t\t$_);\n+\t\t} @{$hunk};\n+\t\tpush @hunks, {TEXT => $hunk, DISPLAY => \\@hunkdisplay};\n+\t}\n+\n+\treturn @hunks;\n+}\n+\n+sub diff_applies {\n+\tmy $fh;\n+\topen $fh, '| git apply --cached --check';\n+\tfor my $h (@_) {\n+\t\tprint $fh @{$h->{TEXT}};\n+\t}\n+\treturn close $fh;\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $hunks, $ix) = @_;\n+\n+\tmy @newhunks = ($hunks->[$ix]);\n+\n+ EDIT:\n+\twhile (1) {\n+\t\t@newhunks = edit_hunk_manually(@newhunks);\n+\t\tif (!diff_applies($head, @$hunks[0..$ix-1], @newhunks,\n+\t\t\t\t @$hunks[$ix+1..$#{$hunks}])) {\n+\t\t\twhile (1) {\n+\t\t\t\tprint colored $prompt_color, 'Your edited hunk does not apply. Edit again (saying \"no\" discards!) [y/n]? ';\n+\t\t\t\tmy $line = ;\n+\t\t\t\tif ($line =~ /^y/i) {\n+\t\t\t\t\tredo EDIT;\n+\t\t\t\t}\n+\t\t\t\telsif ($line =~ /^n/i) {\n+\t\t\t\t\treturn $hunks->[$ix];\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tif (1 < @newhunks) {\n+\t\t\tprint colored $header_color, \"Manually edited into \",\n+\t\t\tscalar(@newhunks), \" hunks.\\n\";\n+\t\t}\n+\t\treturn @newhunks;\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +909,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -885,6 +1014,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1079,11 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tsplice @hunk, $ix, 1, edit_hunk_loop($head, \\@hunk, $ix);\n+\t\t\t\t$num = scalar @hunk;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n@@ -985,13 +1120,8 @@ sub patch_update_file {\n \t\telse {\n \t\t\tif ($n_lofs) {\n \t\t\t\t$n_ofs += $n_lofs;\n-\t\t\t\t$text->[0] = (\"@@ -$o_ofs\" .\n-\t\t\t\t\t (($o_cnt != 1)\n-\t\t\t\t\t ? \",$o_cnt\" : '') .\n-\t\t\t\t\t \" +$n_ofs\" .\n-\t\t\t\t\t (($n_cnt != 1)\n-\t\t\t\t\t ? \",$n_cnt\" : '') .\n-\t\t\t\t\t \" @@\\n\");\n+\t\t\t\t$text->[0] = format_hunk_header($o_ofs, $o_cnt,\n+\t\t\t\t\t\t\t\t$n_ofs, $n_cnt);\n \t\t\t}\n \t\t\tfor (@$text) {\n \t\t\t\tpush @result, $_;\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex fae64ea..7c8d459 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -66,6 +66,63 @@ test_expect_success 'revert works (commit)' '\n \tgrep \"unchanged *+3/-0 file\" output\n '\n \n+cat >expected <fake_editor.sh < diff &&\n+\ttest_cmp expected diff\n+'\n+\n+cat >patch <fake_editor.sh\n+cat >>fake_editor.sh <<\\EOF\n+mv -f \"$1\" oldpatch &&\n+mv -f patch \"$1\"\n+EOF\n+chmod a+x fake_editor.sh\n+test_set_editor \"$(pwd)/fake_editor.sh\"\n+test_expect_success 'garbage edit rejected' '\n+\tgit reset &&\n+\t(echo e; echo n; echo d) | git add -p >output &&\n+\tgrep \"hunk does not apply\" output\n+'\n+\n+cat >patch <expected <output &&\n+\ttest_cmp expected output\n+'\n+\n if test \"$(git config --bool core.filemode)\" = false\n then\n say 'skipping filemode tests (filesystem does not properly support modes)'\n-- \n1.5.6.rc2.129.gcd433\n"},{"id":"79177","messageId":"200806090033.51167.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080606060318.GE18257@sigill.intra.peff.net","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-08T22:33:47Z","receivedAt":"2008-06-08T22:33:47Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n>\n> I wrote this after reading just your first patch, and it looks like\n> you've made much progress since. It seems like Thomas' patch could just\n> get rid of all the recounting entirely, and just pass off the edited\n> hunks to \"git apply --recount\". Which should make his patch much smaller\n> and more straightforward.\n\nI think there's no way to split hunks, in the way I currently \"help\"\nwith @@ line guessing, using just the --recount feature. Unless the\neditor helps you with adding complete correct @@ lines in the middle\nof hunks (Emacs does that). I don't think it is at all possible to\nremove the middle part of a hunk in Johannes' scheme without somehow\nfiguring out the corresponding @@ line (or at least its old line\nnumber) or editing away every +/- line.\n\nThen again it's not always easy even with my patch: you may have to\nmanually insert extra context because of the rule against zero lines\nof context. We could guess the context too if we already guessed the\nline numbers, but that would mean even more complexity, which you\ndon't seem to like...\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n\n\n\n\n\n"},{"id":"79181","messageId":"alpine.DEB.1.00.0806090001090.1783@racer","threadId":"13641","inReplyTo":"200806090018.48784.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-08T23:02:30Z","receivedAt":"2008-06-08T23:02:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Thomas Rast wrote:\n\n> Johannes Schindelin wrote:\n> > \n> > On Fri, 6 Jun 2008, Jeff King wrote:\n> > > 2. It's not integrated into the git-add--interactive loop at all. \n> > > That is, I don't start out saying \"I want to edit this diff.\" I \n> > > look at the diff while staging with \"git add -p\" and say \"Oops, \n> > > I need to edit this hunk.\" So I think it is better implemented \n> > > as an \"e\" option in the hunk adding loop, with \"git add -e\" as \n> > > a shortcut. Or maybe there is simply room for both (and \"git \n> > > add -e\", rather than being a shortcut, just means \"do this on \n> > > the _whole_ file\").\n> > \n> > This is very much on purpose. I do not like \"git add -i\" at all. It \n> > limits my work unduly. That's why I tried to change the hunk editing \n> > in git-gui once upon a time, but I never got round to fix that, and it \n> > does not work well with ssh either.\n> > \n> > So no, I do not want to use that perl script with that menu. I want \n> > to have the raw diff in a raw editor, where I can change the things I \n> > need to change.\n> \n> While there is obviously little point in trying to convince you,\n\nOh, sorry, I really meant the \"I do not want\" literally. It is just me. \nThat does not mean that your hunk editing from within add -i has no merit. \nIt's just that this guy is not very interested in that feature.\n\nCiao,\nDscho\n"},{"id":"79183","messageId":"alpine.DEB.1.00.0806090003450.1783@racer","threadId":"13641","inReplyTo":"200806090033.51167.trast@student.ethz.ch","subject":"Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-08T23:06:03Z","receivedAt":"2008-06-08T23:06:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Thomas Rast wrote:\n\n> Jeff King wrote:\n> >\n> > I wrote this after reading just your first patch, and it looks like \n> > you've made much progress since. It seems like Thomas' patch could \n> > just get rid of all the recounting entirely, and just pass off the \n> > edited hunks to \"git apply --recount\". Which should make his patch \n> > much smaller and more straightforward.\n> \n> I think there's no way to split hunks, in the way I currently \"help\" \n> with @@ line guessing, using just the --recount feature. Unless the \n> editor helps you with adding complete correct @@ lines in the middle of \n> hunks (Emacs does that). I don't think it is at all possible to remove \n> the middle part of a hunk in Johannes' scheme without somehow figuring \n> out the corresponding @@ line (or at least its old line number) or \n> editing away every +/- line.\n> \n> Then again it's not always easy even with my patch: you may have to\n> manually insert extra context because of the rule against zero lines\n> of context.\n\nThat is actually where Junio convinced me that my approach is wrong: he \nsaid that you can _only_ reliably split a hunk at common lines.\n\nThe thing is: if you split _not_ at a common line, the context of the \nsecond part would _change_ depending if you want to apply the first part \nor not.\n\nCiao,\nDscho\n"},{"id":"79185","messageId":"alpine.DEB.1.00.0806090018350.1783@racer","threadId":"13641","inReplyTo":"200806090032.27516.trast@student.ethz.ch","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-08T23:19:07Z","receivedAt":"2008-06-08T23:19:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Thomas Rast wrote:\n\n> Compared to the competing bikeshed\n> http://article.gmane.org/gmane.comp.version-control.git/83894 \n\nIf you really think this is a bikeshed, I better spend my time elsewhere.\n\nHth,\nDscho\n"},{"id":"79203","messageId":"200806090746.22512.johan@herland.net","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0806090018350.1783@racer","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-09T05:46:22Z","receivedAt":"2008-06-09T05:46:22Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 09 June 2008, Johannes Schindelin wrote:\n> Hi,\n>\n> On Mon, 9 Jun 2008, Thomas Rast wrote:\n> > Compared to the competing bikeshed\n> > http://article.gmane.org/gmane.comp.version-control.git/83894\n>\n> If you really think this is a bikeshed, I better spend my time elsewhere.\n\nIs there a good reason against having *both*?\n\nAFAICS, there's nothing stopping us from having both a \"-e\"-option to \ngit-add, and an \"e\"-command inside git-add--interactive.\n\n...just like we have \"-p\" and \"p\" today...\n\n(\"git-add -e\" would open the entire diff in an editor, as would \"e\" from the \n*main* menu of git-add--interactive. However, \"e\" from the *single hunk* \nmenu would of course open only that single hunk within the editor. We could \neven have an \"E\" command to open all remaining/undecided hunks in an \neditor.)\n\n\nHave fun! :)\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"79220","messageId":"20080609122938.GA12210@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806090746.22512.johan@herland.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-09T12:29:38Z","receivedAt":"2008-06-09T12:29:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 09, 2008 at 07:46:22AM +0200, Johan Herland wrote:\n\n> Is there a good reason against having *both*?\n> \n> AFAICS, there's nothing stopping us from having both a \"-e\"-option to \n> git-add, and an \"e\"-command inside git-add--interactive.\n\nI agree (and I tried to make that point in an earlier mail).\n\nAnd I was hoping the right way to do it was to simply build the\ninteractive \"e\" command on top of Johannes' git-apply work. But I don't\nthink that quite makes sense. His work is about fixing up the hunk\nheader as we apply the patch, but a working \"e\" command in the hunk\nselection should probably not actually apply, but simply split into two\nhunks for the loop.\n\n> (\"git-add -e\" would open the entire diff in an editor, as would \"e\" from the \n> *main* menu of git-add--interactive. However, \"e\" from the *single hunk* \n> menu would of course open only that single hunk within the editor. We could \n> even have an \"E\" command to open all remaining/undecided hunks in an \n> editor.)\n\nI agree with all of this, though I think the big question is what\nhappens to the edited portion. In the interactive command, I think it\nbecomes a new hunk that can be staged or not. In \"git add -e\" it makes\nsense to simply stage the result.\n\n-Peff\n"},{"id":"79238","messageId":"alpine.DEB.1.00.0806091709350.1783@racer","threadId":"13641","inReplyTo":"20080609122938.GA12210@sigill.intra.peff.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-09T16:13:08Z","receivedAt":"2008-06-09T16:13:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jun 2008, Jeff King wrote:\n\n> On Mon, Jun 09, 2008 at 07:46:22AM +0200, Johan Herland wrote:\n> \n> And I was hoping the right way to do it was to simply build the \n> interactive \"e\" command on top of Johannes' git-apply work. But I don't \n> think that quite makes sense. His work is about fixing up the hunk \n> header as we apply the patch, but a working \"e\" command in the hunk \n> selection should probably not actually apply, but simply split into two \n> hunks for the loop.\n\nWell, maybe I am silly, but I thought that the idea with add -i (e) was \nto split off the first part, ask the user if that should be applied, and \nthen go on with the rest.\n\nBut like I said, Junio convinced me that it makes not much sense to split \nsomewhere else than common lines, and you have that already with the \"s\" \ncommand in add -i.\n\nIn any case, I am happy with \"add -e\", even more so than I though I would \nbe, and therefore I do not need add -i (e). Thus, I will just shut up and \nlet you guys work it out.\n\nCiao,\nDscho\n"},{"id":"79246","messageId":"200806091931.51221.johan@herland.net","threadId":"13641","inReplyTo":"20080609122938.GA12210@sigill.intra.peff.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-09T17:31:51Z","receivedAt":"2008-06-09T17:31:51Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 09 June 2008, Jeff King wrote:\n> On Mon, Jun 09, 2008 at 07:46:22AM +0200, Johan Herland wrote:\n> > Is there a good reason against having *both*?\n> >\n> > AFAICS, there's nothing stopping us from having both a \"-e\"-option to\n> > git-add, and an \"e\"-command inside git-add--interactive.\n>\n> I agree (and I tried to make that point in an earlier mail).\n>\n> And I was hoping the right way to do it was to simply build the\n> interactive \"e\" command on top of Johannes' git-apply work. But I don't\n> think that quite makes sense.\n\nYeah, the two approaches don't merge easily...\n\n> His work is about fixing up the hunk header as we apply the patch, but a\n> working \"e\" command in the hunk selection should probably not actually\n> apply, but simply split into two hunks for the loop.\n\nBy \"split into two hunks\", you mean splitting the original \"index -> \nworktree\" hunk (#0) into one hunk that represents \"index -> edited\" (#1), \nand another hunk that represents \"edited -> worktree\" (#2)?\n\n>From a technical POV this might make sense, but AFAICS, users would always \nwant to answer 'y' to #1, and 'n' to #2 (see [1]), so from a user POV, \ngit-add--interactive should simply stage #1, and drop #2.\n\n(Side note: AFAIR, some of the original rationale for this feature was to \nprovide a more fine-grained split than 's'. Looking at the problem from \nthis POV: What is the reason for splitting a hunk in the first place? It \nmust be because one part of the hunk should be staged while leaving the \nother unstaged. With 's', it just splits, and lets the user select which \nparts of the hunk to stage, using 'y'/'n'. But 'e' introduces a much more \npowerful notion of letting user split AND select in ONE operation (i.e. the \neditor). Therefore, when the user has already selected which parts of the \nhunk to stage (#1), it is not necessary to re-ask the user whether or not \n#1 should be staged (and certainly not #2).\n\n> > (\"git-add -e\" would open the entire diff in an editor, as would \"e\"\n> > from the *main* menu of git-add--interactive. However, \"e\" from the\n> > *single hunk* menu would of course open only that single hunk within\n> > the editor. We could even have an \"E\" command to open all\n> > remaining/undecided hunks in an editor.)\n>\n> I agree with all of this, though I think the big question is what\n> happens to the edited portion. In the interactive command, I think it\n> becomes a new hunk that can be staged or not. In \"git add -e\" it makes\n> sense to simply stage the result.\n\nSounds acceptable to me (although I would also be ok with automatically \nstaging the edited portion in the interactive command).\n\n\nHave fun! :)\n\n...Johan\n\n\n[1]: AIUI #1 represents the hunk that the user want to stage at this moment. \nConversely, #2 represents the changes that the user is NOT interested in \nstaging at this point. Therefore, the only answers that make sense is 'y' \n(i.e. \"stage this hunk\") for #1 and 'n' (i.e. \"do not stage this hunk\") for \n#2. The only problem with this is if the user screwed up the hunk edit and \nwants to revert to the original hunk (#0). I don't know if this is worth \nsupporting.\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"79256","messageId":"7vlk1egz19.fsf@gitster.siamese.dyndns.org","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0806091709350.1783@racer","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-09T19:59:30Z","receivedAt":"2008-06-09T19:59:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> But like I said, Junio convinced me that it makes not much sense to split \n> somewhere else than common lines, and you have that already with the \"s\" \n> command in add -i.\n\nThat needs clarifying. What I would have convinced you is not quite\n\"somewhere else than common lines\". It is more like \"don't split between\ntwo deleted lines that is followed by addition, or two added lines that\nimmediately follow deletion, without thinking\".\n\nSplitting the left into the right:\n\n 1 1\n -2 -2\n +two +two\n 3 3\n -4\n +four\n 5\n\nso that only the first change is applicable makes sense and can easily be\nexplained. This is \"common lines\" case.\n\nThe example I gave you long time ago about split that does not make much\nsense was to split this:\n\n 1 \n -2 \n -3 \n -4\n +two, three\n +four\n 5\n\nanywhere between \"-2 .. +four\" without giving the user any other ways to\ncontrol the result.\n\nBut you _can_ give users a meaningful split by reordering lines.\n\n 1 \n -2 \n -3 \n +two, three\n -4\n +four\n 5\n\nIt makes sense to then split this immediately before \"-4\" to apply \"only\nthe first change that is to remove two lines and rewrite it with one\nline\", i.e.:\n\n 1 \n -2 \n -3 \n +two, three\n 4\n\nwith the leftover part, whose lines differ if the above is applied or not,\npresented as the next hunk to the user:\n\n (if the above is unused) (if the above is used)\n\n 3 two, three \n -4 -4 \n +four +four \n 5 5 \n"},{"id":"79259","messageId":"20080609201734.GA15662@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806091931.51221.johan@herland.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-09T20:17:34Z","receivedAt":"2008-06-09T20:17:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 09, 2008 at 07:31:51PM +0200, Johan Herland wrote:\n\n> > His work is about fixing up the hunk header as we apply the patch, but a\n> > working \"e\" command in the hunk selection should probably not actually\n> > apply, but simply split into two hunks for the loop.\n> \n> By \"split into two hunks\", you mean splitting the original \"index -> \n> worktree\" hunk (#0) into one hunk that represents \"index -> edited\" (#1), \n> and another hunk that represents \"edited -> worktree\" (#2)?\n\nI mean splitting the original \"index -> worktree\" hunk into two other\nhunks, each of which is \"index -> worktree\" but which can be staged\nseparately. I.e., what the 's'plit command does, but with finer-grained\ncontrol.\n\nBut I think that is what you are trying to say...\n\n> From a technical POV this might make sense, but AFAICS, users would always \n> want to answer 'y' to #1, and 'n' to #2 (see [1]), so from a user POV, \n> git-add--interactive should simply stage #1, and drop #2.\n\nYes. I assumed we wanted to maintain the separate splitting operation,\nsince that parallels the existing split (so the interface is consistent)\nand it logically separates the two parts (you split, and then you choose\nthe part you want).\n\nBut honestly, I don't really see a use case that isn't covered by\n\"manually edit the diff and apply the hunk\". And the rationale in your\n\"side note\" indicates that you think the same way.\n\nSo now I wonder if we _can_ leverage Dscho's work here. I.e., can we\nsimply send the edited hunk to \"git apply --recount --cached\" (instead\nof doing a \"git apply --check\")?\n\nThe main problem I see at this point is that it screws up the line\nnumbering for _every other hunk_, so later hunks in that file might not\napply (IIRC, we usually save up all of the \"yes\" hunks and apply at the\nend). So it might be needed to do a --recount --check, and then actually\napply at the end.\n\nI'll try to play around with that.\n\n-Peff\n"},{"id":"79272","messageId":"200806092319.23906.johan@herland.net","threadId":"13641","inReplyTo":"20080609201734.GA15662@sigill.intra.peff.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-06-09T21:19:23Z","receivedAt":"2008-06-09T21:19:23Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 09 June 2008, Jeff King wrote:\n> But honestly, I don't really see a use case that isn't covered by\n> \"manually edit the diff and apply the hunk\". And the rationale in your\n> \"side note\" indicates that you think the same way.\n\nAgreed.\n\n> I'll try to play around with that.\n\nLooking forward to play with the result.\n\n\nHave fun! :)\n\n...Johan\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"79336","messageId":"20080610110553.GA13655@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806092319.23906.johan@herland.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-10T11:05:53Z","receivedAt":"2008-06-10T11:05:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 09, 2008 at 11:19:23PM +0200, Johan Herland wrote:\n\n> On Monday 09 June 2008, Jeff King wrote:\n> > But honestly, I don't really see a use case that isn't covered by\n> > \"manually edit the diff and apply the hunk\". And the rationale in your\n> > \"side note\" indicates that you think the same way.\n\nBelow is a simple patch to build interactive edit support on top of\nDscho's \"git apply --recount\" patch. Rather than create new hunks, the\naction is just \"edit and apply\": if the apply is successful, the hunk is\nremoved from further consideration. This is just for playing with, and\nnot for commit:\n\n - it's not very well tested :)\n\n - I think the recount stuff may have flaws; I didn't follow the thread\n too closely, but Junio seemed to have a lot of comments (I am\n working under the assumption that Dscho will get that part right,\n and we can just build on top of it)\n\n - I tried one or two simple tests where I edited and applied an early\n hunk, and then resumed the loop for later hunks. Everything seemed\n to work, but I am not convinced that it isn't possible to make the\n rest of the hunk selection loop useless by applying early and\n invalidating all of the other hunks. That might be a flaw with this\n approach. A more sane interface might be to simply jump from the\n hunk selection loop into manually editing _all_ hunks and then\n applying, ending the hunk selection loop.\n\nThis should be applied on top of \"allow git-apply to ignore the hunk\nheaders\" (v3) which can be found on the list. Obviously significant\nportions of the patch below are based on Thomas' version.\n\n-- >8 --\ngit-add--interactive: manual hunk editing mode\n\nAdds a new option 'e' to the 'add -p' command loop that lets you\nedit the current hunk in your favourite editor, applying the\nresult.\n---\n git-add--interactive.perl | 81 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 81 insertions(+), 0 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..62b5aed 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,7 @@\n \n use strict;\n use Git;\n+use File::Temp;\n \n my $repo = Git->repository();\n \n@@ -770,6 +771,76 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub edit_hunk_manually {\n+\tmy ($oldtext) = @_;\n+\n+\tmy $t = File::Temp->new(\n+\t\tTEMPLATE => $repo->repo_path . \"/git-hunk-edit.XXXXXX\",\n+\t\tSUFFIX => '.diff'\n+\t);\n+\tprint $t \"# Manual hunk edit mode -- see bottom for a quick guide\\n\";\n+\tprint $t @$oldtext;\n+\tprint $t <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $t);\n+\n+\topen my $fh, '<', $t\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext = grep { !/^#/ } <$fh>;\n+\tclose $fh;\n+\n+\t# Reinsert the first hunk header if the user accidentally deleted it\n+\tif ($newtext[0] !~ /^@/) {\n+\t\tunshift @newtext, $oldtext->[0];\n+\t}\n+\treturn \\@newtext;\n+}\n+\n+sub apply_diff {\n+\topen(my $fh, '|-', qw(git apply --cached --recount));\n+\tprint $fh map { @$_ } @_;\n+\treturn close $fh;\n+}\n+\n+sub prompt_yesno {\n+\tmy ($prompt) = @_;\n+\twhile (1) {\n+\t\tprint colored $prompt_color, $prompt;\n+\t\tmy $line = ;\n+\t\treturn 0 if $line =~ /^n/i;\n+\t\treturn 1 if $line =~ /^y/i;\n+\t}\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $text) = @_;\n+\n+\twhile (1) {\n+\t\t$text = edit_hunk_manually($text);\n+\t\tapply_diff($head, $text) and return 1;\n+\t\tprompt_yesno(\n+\t\t\t'Your edited hunk does not apply. Edit again '\n+\t\t\t. '(saying \"no\" discards!) [y/n]? '\n+\t\t) or return 0;\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +852,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -885,6 +957,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1022,14 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tif (edit_hunk_loop($head->{TEXT},\n+\t\t\t\t\t\t $hunk[$ix]{TEXT})) {\n+\t\t\t\t\tsplice @hunk, $ix, 1;\n+\t\t\t\t\t$num = @hunk;\n+\t\t\t\t}\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n-- \n1.5.6.rc2.26.gda075.dirty\n"},{"id":"79337","messageId":"484E632B.7050600@op5.se","threadId":"13641","inReplyTo":"20080609201734.GA15662@sigill.intra.peff.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-06-10T11:19:07Z","receivedAt":"2008-06-10T11:19:07Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Jeff King wrote:\n> \n> The main problem I see at this point is that it screws up the line\n> numbering for _every other hunk_, so later hunks in that file might not\n> apply (IIRC, we usually save up all of the \"yes\" hunks and apply at the\n> end). So it might be needed to do a --recount --check, and then actually\n> apply at the end.\n> \n\nI for one would really hate if the apply-at-the-end approach currently\nused would be swapped around to \"apply-as-we-go\", because I sometimes\nrealize I'm committing a feature on top of a bugfix I've forgotten to\ncommit (or some such). I know I'd forget quite a few times to revert\nthe added changes after Ctrl-C'ing out of such cases if the current\napproach is changed.\n\nI know that wasn't what was proposed but at least anyone who\nproposes it will at least think twice now before doing so, and\nhopefully providing a SIGHUP hook to restore the index+worktree\nto exactly what they were at the start of the interactive session.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"79442","messageId":"200806111102.31481.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080610110553.GA13655@sigill.intra.peff.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-11T09:02:29Z","receivedAt":"2008-06-11T09:02:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n>\n> Below is a simple patch to build interactive edit support on top of\n> Dscho's \"git apply --recount\" patch. Rather than create new hunks, the\n> action is just \"edit and apply\": if the apply is successful, the hunk is\n> removed from further consideration.\n\nOk, I finally see what you meant earlier. So how about this, which is\nbasically \"middle ground\":\n\nAs in the parent, editing is restricted to a single hunk, no splitting\nallowed (almost---you can still insert @ lines if you are good at\ncounting, but you're on your own).\n\nHowever, as in the earlier versions, the hunk is placed in the editing\nloop again. In this version, I made it so it will be marked for use\nas if you had entered 'y', so if it was the last (or only) hunk, the\nfiles will be patched immediately. But if you Ctrl-C out in the\nmiddle, nothing has been changed yet.\n\nThe following two remarks also apply:\n\n> - it's not very well tested :)\n[...]\n> This should be applied on top of \"allow git-apply to ignore the hunk\n> headers\" (v3) which can be found on the list.\n\nBy the way, current 'add -p' recounts the headers to account for all\nhunks that the user answered 'n'. I haven't given it enough thought\nto put it in the code yet, but it may be possible to rip that out as\nwell and unconditionally --recount. Only the preimage line numbers\nmatter, and those never change.\n\n-- 8< --\n git-add--interactive.perl | 128 ++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 127 insertions(+), 1 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..8c39149 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,7 @@\n \n use strict;\n use Git;\n+use File::Temp;\n \n my $repo = Git->repository();\n \n@@ -18,6 +19,18 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n+my ($diff_old_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.old', 'red'),\n+\t) : ();\n+my ($diff_new_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.new', 'green'),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -770,6 +783,108 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub color_diff {\n+\treturn map {\n+\t\tcolored((/^@/ ? $fraginfo_color :\n+\t\t\t /^\\+/ ? $diff_new_color :\n+\t\t\t /^-/ ? $diff_old_color :\n+\t\t\t $diff_plain_color),\n+\t\t\t$_);\n+\t} @_;\n+}\n+\n+sub edit_hunk_manually {\n+\tmy ($oldtext) = @_;\n+\n+\tmy $t = File::Temp->new(\n+\t\tTEMPLATE => $repo->repo_path . \"/git-hunk-edit.XXXXXX\",\n+\t\tSUFFIX => '.diff'\n+\t);\n+\tprint $t \"# Manual hunk edit mode -- see bottom for a quick guide\\n\";\n+\tprint $t @$oldtext;\n+\tprint $t <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $t);\n+\n+\topen my $fh, '<', $t\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext = grep { !/^#/ } <$fh>;\n+\tclose $fh;\n+\n+\t# Abort if nothing remains\n+\tif (@newtext == 0) {\n+\t\treturn undef;\n+\t}\n+\n+\t# Reinsert the first hunk header if the user accidentally deleted it\n+\tif ($newtext[0] !~ /^@/) {\n+\t\tunshift @newtext, $oldtext->[0];\n+\t}\n+\treturn \\@newtext;\n+}\n+\n+sub diff_applies {\n+\tmy $fh;\n+\topen $fh, '| git apply --recount --cached --check';\n+\tfor my $h (@_) {\n+\t\tprint $fh @{$h->{TEXT}};\n+\t}\n+\treturn close $fh;\n+}\n+\n+sub prompt_yesno {\n+\tmy ($prompt) = @_;\n+\twhile (1) {\n+\t\tprint colored $prompt_color, $prompt;\n+\t\tmy $line = ;\n+\t\treturn 0 if $line =~ /^n/i;\n+\t\treturn 1 if $line =~ /^y/i;\n+\t}\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $hunk, $ix) = @_;\n+\tmy $text = $hunk->[$ix]->{TEXT};\n+\n+\twhile (1) {\n+\t\t$text = edit_hunk_manually($text);\n+\t\tif (!defined $text) {\n+\t\t\treturn undef;\n+\t\t}\n+\t\tmy $newhunk = { TEXT => $text, USE => 1 };\n+\t\tif (diff_applies($head,\n+\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t $newhunk,\n+\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n+\t\t\tmy @display = color_diff(@{$text});\n+\t\t\t$newhunk->{DISPLAY} = \\@display;\n+\t\t\treturn $newhunk;\n+\t\t}\n+\t\telse {\n+\t\t\tprompt_yesno(\n+\t\t\t\t'Your edited hunk does not apply. Edit again '\n+\t\t\t\t. '(saying \"no\" discards!) [y/n]? '\n+\t\t\t\t) or return undef;\n+\t\t}\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +896,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -846,6 +962,7 @@ sub patch_update_file {\n \n \t$num = scalar @hunk;\n \t$ix = 0;\n+\tmy $need_recount = 0;\n \n \twhile (1) {\n \t\tmy ($prev, $next, $other, $undecided, $i);\n@@ -885,6 +1002,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1067,13 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tmy $newhunk = edit_hunk_loop($head, \\@hunk, $ix);\n+\t\t\t\tif (defined $newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t\t$need_recount = 1;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n@@ -1002,7 +1127,8 @@ sub patch_update_file {\n \tif (@result) {\n \t\tmy $fh;\n \n-\t\topen $fh, '| git apply --cached';\n+\t\topen $fh, '| git apply --cached'\n+\t\t\t. ($need_recount ? ' --recount' : '');\n \t\tfor (@{$head->{TEXT}}, @result) {\n \t\t\tprint $fh $_;\n \t\t}\n-- \n1.5.6.rc1.134.g8dfb.dirty\n"},{"id":"79539","messageId":"20080612044932.GA25992@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806111102.31481.trast@student.ethz.ch","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-12T04:49:33Z","receivedAt":"2008-06-12T04:49:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 11, 2008 at 11:02:29AM +0200, Thomas Rast wrote:\n\n> However, as in the earlier versions, the hunk is placed in the editing\n> loop again. In this version, I made it so it will be marked for use\n> as if you had entered 'y', so if it was the last (or only) hunk, the\n> files will be patched immediately. But if you Ctrl-C out in the\n> middle, nothing has been changed yet.\n\nOK, I really think this is the most sane behavior. It naturally follows\nwhat the user wants (if they edit, they then want to apply), but it\nstill waits until the end of the loop, so they can back out if they\nwant.\n\n> By the way, current 'add -p' recounts the headers to account for all\n> hunks that the user answered 'n'. I haven't given it enough thought\n> to put it in the code yet, but it may be possible to rip that out as\n> well and unconditionally --recount. Only the preimage line numbers\n> matter, and those never change.\n\nAh, I hadn't thought about that, but of course it makes sense that it\nwould need to recount (I guess I should have looked a little more\nclosely at the other code). I think replacing that with --recount would\nbe great, but it should be a separate patch, and we should wait for\n--recount to stabilize (all of this should be post 1.5.6, anyway).\n\n> +# If the patch applies cleanly, the hunk will immediately be marked\n> +# for staging as if you had answered 'y'. (However, if you remove\n> +# everything, nothing happens.) Otherwise, you will be asked to edit\n> +# again.\n\nThis \"however\" confused me the first time I read it. I assume you mean\nthat \"if the diff is empty, then staging it will do nothing\"? I wonder\nif that is even worth mentioning, since it seems obvious.\n\nAlthough I guess you do special-case \"no lines\" later on.\n\n> +# Do not add @ lines unless you know what you are doing. The original\n> +# @ line will be reinserted if you remove it.\n\nThis can probably be moved from the \"every time\" instructions to the\nmanual, if we want to keep the size of the instructions a bit smaller.\n\n> +\t\tif (diff_applies($head,\n> +\t\t\t\t @{$hunk}[0..$ix-1],\n> +\t\t\t\t $newhunk,\n> +\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n\nI'm confused here...we are feeding _all_ of the hunks to git-apply. In\nmy patch, I simply fed the hunk of interest. Since we are recounting,\nshouldn't the single hunk be enough? And if it isn't, then shouldn't we\nbe feeding only the hunks that are to be applied?\n\n> +\t\t\tmy @display = color_diff(@{$text});\n> +\t\t\t$newhunk->{DISPLAY} = \\@display;\n\nPerl nit: you can create an arrayref with brackets:\n\n $newhunk->{DISPLAY} = [color_diff(@$text)];\n\n> +\t\topen $fh, '| git apply --cached'\n> +\t\t\t. ($need_recount ? ' --recount' : '');\n\nNice. I think this is much cleaner.\n\n-Peff\n"},{"id":"79566","messageId":"200806120855.26847.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080612044932.GA25992@sigill.intra.peff.net","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-12T06:55:24Z","receivedAt":"2008-06-12T06:55:24Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> On Wed, Jun 11, 2008 at 11:02:29AM +0200, Thomas Rast wrote:\n> \n> > +# If the patch applies cleanly, the hunk will immediately be marked\n> > +# for staging as if you had answered 'y'. (However, if you remove\n> > +# everything, nothing happens.) Otherwise, you will be asked to edit\n> > +# again.\n> \n> This \"however\" confused me the first time I read it. I assume you mean\n> that \"if the diff is empty, then staging it will do nothing\"? I wonder\n> if that is even worth mentioning, since it seems obvious.\n\nI wanted to make the same special case that 'recount -i' has: Deleting\neverything does not \"do nothing\", but actually aborts the edit. So in\nrebase, deleting everything will not rebase anything (instead of\nrebasing \"no patches\" onto wherever you said). Along the same lines,\nin the above patch deleting everything does not patch \"no changes\",\nbut aborts editing the hunk and leaves it unchanged.\n\nDo you think that behaviour confuses users?\n\n> > +# Do not add @ lines unless you know what you are doing. The original\n> > +# @ line will be reinserted if you remove it.\n> \n> This can probably be moved from the \"every time\" instructions to the\n> manual, if we want to keep the size of the instructions a bit smaller.\n> \n> > +\t\tif (diff_applies($head,\n> > +\t\t\t\t @{$hunk}[0..$ix-1],\n> > +\t\t\t\t $newhunk,\n> > +\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n> \n> I'm confused here...we are feeding _all_ of the hunks to git-apply. In\n> my patch, I simply fed the hunk of interest. Since we are recounting,\n> shouldn't the single hunk be enough? And if it isn't, then shouldn't we\n> be feeding only the hunks that are to be applied?\n\nFeeding it the single hunk would be enough, but doing it this way\nensures that the edited hunk cannot step on another hunk's toes,\ni.e. produce a conflict that may prevent us from applying the patch at\nthe end of the hunk selection loop.\n\nIt's fairly unlikely of course, because the user would deliberately\nhave to extend the hunk beyond its current borders by adding extra\ncontext from the original file.\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n"},{"id":"79568","messageId":"20080612071311.GA32491@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806120855.26847.trast@student.ethz.ch","subject":"Re: [PATCH v3] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-12T07:13:11Z","receivedAt":"2008-06-12T07:13:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 12, 2008 at 08:55:24AM +0200, Thomas Rast wrote:\n\n> > > +# If the patch applies cleanly, the hunk will immediately be marked\n> > > +# for staging as if you had answered 'y'. (However, if you remove\n> > > +# everything, nothing happens.) Otherwise, you will be asked to edit\n> > > +# again.\n> [...]\n> I wanted to make the same special case that 'recount -i' has: Deleting\n> everything does not \"do nothing\", but actually aborts the edit. So in\n> rebase, deleting everything will not rebase anything (instead of\n> rebasing \"no patches\" onto wherever you said). Along the same lines,\n> in the above patch deleting everything does not patch \"no changes\",\n> but aborts editing the hunk and leaves it unchanged.\n> \n> Do you think that behaviour confuses users?\n\nAh, OK. That makes sense. I think it could be worded more explicitly,\nthough. Maybe:\n\n If the patch applies cleanly, the edited hunk will immediately be\n marked for staging. If it does not apply cleanly, you will be given an\n opportunity to edit again. If all lines of the hunk are removed, then\n the edit is aborted and the hunk is left unchanged.\n\n> Feeding it the single hunk would be enough, but doing it this way\n> ensures that the edited hunk cannot step on another hunk's toes,\n> i.e. produce a conflict that may prevent us from applying the patch at\n> the end of the hunk selection loop.\n> \n> It's fairly unlikely of course, because the user would deliberately\n> have to extend the hunk beyond its current borders by adding extra\n> context from the original file.\n\nRight, but you may get false positives if a later conflicting hunk is\nnot staged. Though as you say, I think it is unlikely in either case.\n\n-Peff\n"},{"id":"79734","messageId":"200806131748.44867.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080612071311.GA32491@sigill.intra.peff.net","subject":"[PATCH v4] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-13T15:48:43Z","receivedAt":"2008-06-13T15:48:43Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Adds a new option 'e' to the 'add -p' command loop that lets you edit\nthe current hunk in your favourite editor.\n\nIf the resulting patch applies cleanly, the edited hunk will\nimmediately be marked for staging. If it does not apply cleanly, you\nwill be given an opportunity to edit again. If all lines of the hunk\nare removed, then the edit is aborted and the hunk is left unchanged.\n\nApplying the changed hunk(s) relies on Johannes Schindelin's new\n--recount option for git-apply.\n\nNote that the \"real patch\" test intentionally uses\n (echo e; echo n; echo d) | git add -p\neven though the 'n' and 'd' are superfluous at first sight. They\nserve to get out of the interaction loop if git add -p wrongly\nconcludes the patch does not apply.\n\nMany thanks to Jeff King for lots of help and\nsuggestions.\n\nSigned-off-by: Thomas Rast \n---\n\nOnce again trying to shape this up as a complete patch.\n\nJeff King wrote:\n> On Thu, Jun 12, 2008 at 08:55:24AM +0200, Thomas Rast wrote:\n> \n> > I wanted to make the same special case that 'recount -i' has: Deleting\n> > everything does not \"do nothing\", but actually aborts the edit.\n[...]\n> Ah, OK. That makes sense. I think it could be worded more explicitly,\n> though. [...]\n\nI took the liberty of copying your wording to the help and commit\nmessages, as it is indeed much clearer that way.\n\n> Right, but you may get false positives if a later conflicting hunk is\n> not staged. Though as you say, I think it is unlikely in either case.\n\nI'd rather reject early and offer to re-edit, than notice a problem\nmuch later, so I left it the way it was.\n\n\nChanges since the last version:\n\n* The help message mentioned above\n\n* Use [f(...)] instead of @tmp = f(); \\@tmp\n\n* Re-added tests from v3, with an added test for a correctly formatted\n patch that does not apply\n\n* Re-added a line about 'e' to Documentation/git-add.txt (which was in\n the original patch but got lost)\n\nI was tempted to reintroduce globbed unlinking of the temporary file\nas in v3 (that is, removing TMP* instead of just TMP). In the absence\nof it, backup files made by the user's editor will remain in .git/.\nEventually I didn't because it seems vi doesn't make backups anyway,\nand while emacs does, enabling that for GIT_EDITOR seems a user error.\nBesides, how do we know the backups match that pattern anyway. Not\nsure though.\n\n- Thomas\n \n\n Documentation/git-add.txt | 1 +\n git-add--interactive.perl | 124 +++++++++++++++++++++++++++++++++++++++++++-\n t/t3701-add-interactive.sh | 67 ++++++++++++++++++++++++\n 3 files changed, 191 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 8620ae2..842430c 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -238,6 +238,7 @@ patch::\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+ e - manually edit the current hunk\n ? - print help\n +\n After deciding the fate for all hunks, if there is any hunk\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..8cc275d 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,7 @@\n \n use strict;\n use Git;\n+use File::Temp;\n \n my $repo = Git->repository();\n \n@@ -18,6 +19,18 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n+my ($diff_old_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.old', 'red'),\n+\t) : ();\n+my ($diff_new_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.new', 'green'),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -770,6 +783,104 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub color_diff {\n+\treturn map {\n+\t\tcolored((/^@/ ? $fraginfo_color :\n+\t\t\t /^\\+/ ? $diff_new_color :\n+\t\t\t /^-/ ? $diff_old_color :\n+\t\t\t $diff_plain_color),\n+\t\t\t$_);\n+\t} @_;\n+}\n+\n+sub edit_hunk_manually {\n+\tmy ($oldtext) = @_;\n+\n+\tmy $t = File::Temp->new(\n+\t\tTEMPLATE => $repo->repo_path . \"/git-hunk-edit.XXXXXX\",\n+\t\tSUFFIX => '.diff'\n+\t);\n+\tprint $t \"# Manual hunk edit mode -- see bottom for a quick guide\\n\";\n+\tprint $t @$oldtext;\n+\tprint $t <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $t);\n+\n+\topen my $fh, '<', $t\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext = grep { !/^#/ } <$fh>;\n+\tclose $fh;\n+\n+\t# Abort if nothing remains\n+\tif (@newtext == 0) {\n+\t\treturn undef;\n+\t}\n+\n+\t# Reinsert the first hunk header if the user accidentally deleted it\n+\tif ($newtext[0] !~ /^@/) {\n+\t\tunshift @newtext, $oldtext->[0];\n+\t}\n+\treturn \\@newtext;\n+}\n+\n+sub diff_applies {\n+\tmy $fh;\n+\topen $fh, '| git apply --recount --cached --check';\n+\tfor my $h (@_) {\n+\t\tprint $fh @{$h->{TEXT}};\n+\t}\n+\treturn close $fh;\n+}\n+\n+sub prompt_yesno {\n+\tmy ($prompt) = @_;\n+\twhile (1) {\n+\t\tprint colored $prompt_color, $prompt;\n+\t\tmy $line = ;\n+\t\treturn 0 if $line =~ /^n/i;\n+\t\treturn 1 if $line =~ /^y/i;\n+\t}\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $hunk, $ix) = @_;\n+\tmy $text = $hunk->[$ix]->{TEXT};\n+\n+\twhile (1) {\n+\t\t$text = edit_hunk_manually($text);\n+\t\tif (!defined $text) {\n+\t\t\treturn undef;\n+\t\t}\n+\t\tmy $newhunk = { TEXT => $text, USE => 1 };\n+\t\tif (diff_applies($head,\n+\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t $newhunk,\n+\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n+\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$text})];\n+\t\t\treturn $newhunk;\n+\t\t}\n+\t\telse {\n+\t\t\tprompt_yesno(\n+\t\t\t\t'Your edited hunk does not apply. Edit again '\n+\t\t\t\t. '(saying \"no\" discards!) [y/n]? '\n+\t\t\t\t) or return undef;\n+\t\t}\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +892,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -846,6 +958,7 @@ sub patch_update_file {\n \n \t$num = scalar @hunk;\n \t$ix = 0;\n+\tmy $need_recount = 0;\n \n \twhile (1) {\n \t\tmy ($prev, $next, $other, $undecided, $i);\n@@ -885,6 +998,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1063,13 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tmy $newhunk = edit_hunk_loop($head, \\@hunk, $ix);\n+\t\t\t\tif (defined $newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t\t$need_recount = 1;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n@@ -1002,7 +1123,8 @@ sub patch_update_file {\n \tif (@result) {\n \t\tmy $fh;\n \n-\t\topen $fh, '| git apply --cached';\n+\t\topen $fh, '| git apply --cached'\n+\t\t\t. ($need_recount ? ' --recount' : '');\n \t\tfor (@{$head->{TEXT}}, @result) {\n \t\t\tprint $fh $_;\n \t\t}\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex fae64ea..e95663d 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -66,6 +66,73 @@ test_expect_success 'revert works (commit)' '\n \tgrep \"unchanged *+3/-0 file\" output\n '\n \n+cat >expected <fake_editor.sh < diff &&\n+\ttest_cmp expected diff\n+'\n+\n+cat >patch <fake_editor.sh\n+cat >>fake_editor.sh <<\\EOF\n+mv -f \"$1\" oldpatch &&\n+mv -f patch \"$1\"\n+EOF\n+chmod a+x fake_editor.sh\n+test_set_editor \"$(pwd)/fake_editor.sh\"\n+test_expect_success 'bad edit rejected' '\n+\tgit reset &&\n+\t(echo e; echo n; echo d) | git add -p >output &&\n+\tgrep \"hunk does not apply\" output\n+'\n+\n+cat >patch <output &&\n+\tgrep \"hunk does not apply\" output\n+'\n+\n+cat >patch <expected <output &&\n+\ttest_cmp expected output\n+'\n+\n if test \"$(git config --bool core.filemode)\" = false\n then\n say 'skipping filemode tests (filesystem does not properly support modes)'\n--\n1.5.6.rc1.138.gb2840\n"},{"id":"80740","messageId":"20080623183840.GA28887@sigill.intra.peff.net","threadId":"13641","inReplyTo":"200806131748.44867.trast@student.ethz.ch","subject":"Re: [PATCH v4] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-23T18:38:40Z","receivedAt":"2008-06-23T18:38:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 13, 2008 at 05:48:43PM +0200, Thomas Rast wrote:\n\n> Adds a new option 'e' to the 'add -p' command loop that lets you edit\n> the current hunk in your favourite editor.\n> [...]\n> Applying the changed hunk(s) relies on Johannes Schindelin's new\n> --recount option for git-apply.\n\nThis version looks pretty good to me (though I do have a few comments\nbelow), and now that we are in the new release cycle, I think it is time\nto re-submit \"for real\".\n\nThe big question is what is happening with the recount work. Johannes,\nare you planning on re-submitting those patches?\n\n> > Right, but you may get false positives if a later conflicting hunk is\n> > not staged. Though as you say, I think it is unlikely in either case.\n> I'd rather reject early and offer to re-edit, than notice a problem\n> much later, so I left it the way it was.\n\nYeah, thinking about it more, that is the sanest choice.\n\n> I was tempted to reintroduce globbed unlinking of the temporary file\n> as in v3 (that is, removing TMP* instead of just TMP). In the absence\n> of it, backup files made by the user's editor will remain in .git/.\n> Eventually I didn't because it seems vi doesn't make backups anyway,\n> and while emacs does, enabling that for GIT_EDITOR seems a user error.\n> Besides, how do we know the backups match that pattern anyway. Not\n> sure though.\n\nI would leave it; it seems like the editor's responsibility to handle\nthis. We don't know what patterns are used, or when it is safe to remove\nsuch backups. And this is far from the only place where we invoke the\neditor on a temporary file, so any solution should probably be applied\nglobally.\n\n> +\tmy @newtext = grep { !/^#/ } <$fh>;\n> [...]\n> +\t# Abort if nothing remains\n> +\tif (@newtext == 0) {\n> +\t\treturn undef;\n> +\t}\n\nReading over this again, I wonder if it should be:\n\n # Abort if nothing remains\n if (!grep { /\\S/ } @newtext) {\n return undef;\n }\n\nThat is, we can't eliminate blank lines on input, since they might be\nmeaningful to the diff. But if the user removes every non-comment line\n_except_ a blank line or a line with only whitespace, we probably want\nto abort, too.\n\n-Peff\n"},{"id":"80744","messageId":"alpine.DEB.1.00.0806231952360.6440@racer","threadId":"13641","inReplyTo":"20080623183840.GA28887@sigill.intra.peff.net","subject":"Re: [PATCH v4] git-add--interactive: manual hunk editing mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-23T18:54:29Z","receivedAt":"2008-06-23T18:54:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 23 Jun 2008, Jeff King wrote:\n\n> The big question is what is happening with the recount work. Johannes, \n> are you planning on re-submitting those patches?\n\nOh, so much to do. I have 55 patches in my personal fork, on top of \n'next'. And a few of them need resubmitting, such as the initHook work, \nand, yes, add --edit.\n\nI am not sure when I will have time for that (particularly given that I \ngot sidetracked with the OPTION_OPTIONS patch, when I should have worked \non something completely different).\n\nIn the meantime, feel free to submit in my name.\n\nCiao,\nDscho\n"},{"id":"80751","messageId":"20080623195719.GB29569@sigill.intra.peff.net","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0806231952360.6440@racer","subject":"Re: [PATCH v4] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-23T19:57:19Z","receivedAt":"2008-06-23T19:57:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 23, 2008 at 07:54:29PM +0100, Johannes Schindelin wrote:\n\n> I am not sure when I will have time for that (particularly given that I \n> got sidetracked with the OPTION_OPTIONS patch, when I should have worked \n> on something completely different).\n\nHeh, I know the feeling. ;)\n\n> In the meantime, feel free to submit in my name.\n\nDo you remember off the top of your head whether more work was needed?\nThey looked good to me, but then I seem to recall you saying in another\nthread that Junio pointed out some flaw with your approach (but I never\nquite understood what the flaw was).\n\n-Peff\n"},{"id":"80769","messageId":"alpine.DEB.1.00.0806232204550.6440@racer","threadId":"13641","inReplyTo":"20080623195719.GB29569@sigill.intra.peff.net","subject":"apply --recount, was Re: [PATCH v4] git-add--interactive: manual hunk editing mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-23T21:16:08Z","receivedAt":"2008-06-23T21:16:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 23 Jun 2008, Jeff King wrote:\n\n> On Mon, Jun 23, 2008 at 07:54:29PM +0100, Johannes Schindelin wrote:\n> \n> > I am not sure when I will have time for that (particularly given that \n> > I got sidetracked with the OPTION_OPTIONS patch, when I should have \n> > worked on something completely different).\n> \n> Heh, I know the feeling. ;)\n\nYes, us mere mortals cannot hack on Linux and Git all the time, much as \nwe would like to (be able to).\n\n> > In the meantime, feel free to submit in my name.\n> \n> Do you remember off the top of your head whether more work was needed? \n> They looked good to me, but then I seem to recall you saying in another \n> thread that Junio pointed out some flaw with your approach (but I never \n> quite understood what the flaw was).\n\nI think it was not a flaw, but something to be worried about: \n\nhttp://mid.gmane.org/7v4p87zcv6.fsf@gitster.siamese.dyndns.org\n\nTo spare you following that link: Junio wanted to reuse \"git apply \n--recount\" to apply mboxes, where a separator \"^-- $\" to the signature is \nquite common, and could be mistaken for a \"-\" line of a hunk.\n\nI am not quite sure how to deal with this; \"--recount=nosig\" (assume there \nis no signature) and \"--recount=stripsig\" (assume that \"^-- $\" is a \nlead-in to a signature) come to mind, defaulting to \"nosig\" with a \nwarning.\n\nHowever, I think that this issue should not concern us _now_. As long as \n--recount is only to be used in \"add -i\" and \"add -e\", I think the patch \nis good as is:\n\nhttp://repo.or.cz/w/git/dscho.git?a=commitdiff;h=c95d4b3da2e5f595f600720b91b8e396b43fe0ae\n\nCiao,\nDscho\n"},{"id":"80829","messageId":"20080624050901.GA19224@sigill.intra.peff.net","threadId":"13641","inReplyTo":"alpine.DEB.1.00.0806232204550.6440@racer","subject":"Re: apply --recount, was Re: [PATCH v4] git-add--interactive: manual hunk editing mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-24T05:09:02Z","receivedAt":"2008-06-24T05:09:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 23, 2008 at 10:16:08PM +0100, Johannes Schindelin wrote:\n\n> I think it was not a flaw, but something to be worried about: \n> \n> http://mid.gmane.org/7v4p87zcv6.fsf@gitster.siamese.dyndns.org\n> \n> To spare you following that link: Junio wanted to reuse \"git apply \n> --recount\" to apply mboxes, where a separator \"^-- $\" to the signature is \n> quite common, and could be mistaken for a \"-\" line of a hunk.\n\nAh, OK. Thanks for the recap, that makes sense to me now.\n\n> However, I think that this issue should not concern us _now_. As long as \n> --recount is only to be used in \"add -i\" and \"add -e\", I think the patch \n> is good as is:\n\nI agree. We can worry about the other when and if somebody decides to\nuse it that way (but maybe Junio is already planning to do so...).\n\nThomas, do you want to just re-submit the \"--recount\" patches when you\nre-submit your patch?\n\n-Peff\n"},{"id":"80951","messageId":"200806242108.08654.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080624050901.GA19224@sigill.intra.peff.net","subject":"[PATCH 0/3] Manual editing for 'add' and 'add -p'","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-24T19:07:54Z","receivedAt":"2008-06-24T19:07:54Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> Thomas, do you want to just re-submit the \"--recount\" patches when you\n> re-submit your patch?\n\nIf that is the right thing to do :-)\n\nI rebased Johannes' patches to current 'next', which had some trivial\nmerge conflicts, and made one actual change: By analogy with one\nearlier patch that caused some of the conflicts, I split '-e' and\n'--edit' across two lines in the documentation.\n\nI also integrated your last suggestion:\n\n> # Abort if nothing remains\n> if (!grep { /\\S/ } @newtext) {\n> return undef;\n> }\n\nI'm not really convinced it is needed, but any such patch can\nobviously not change anything, so it can't hurt. Who knows, perhaps\nthere are editors brain-damaged enough to always save at least one\nnewline. (This is the only change to my patch compared to v4.)\n\nJohannes Schindelin wrote:\n>\n> To spare you following that link: Junio wanted to reuse \"git apply \n> --recount\" to apply mboxes, where a separator \"^-- $\" to the signature is \n> quite common, and could be mistaken for a \"-\" line of a hunk.\n[...]\n> However, I think that this issue should not concern us _now_. As long as \n> --recount is only to be used in \"add -i\" and \"add -e\", I think the patch \n> is good as is:\n\nI'm wondering if this should be turned into docs. After all, it's not\nsome DWIM option. It just says that instead of trusting the counts,\nlines starting with \"diff \" or \"@@ \" start hunks, implying that all\n/^[-+ ]/ lines in between are significant.\n\n\nI hope this series turns out right in mail; I'm struggling a bit to\nproperly import the mails to KMail. Most annoyingly, format-patch\napparently cannot turn other people's patches into mails by myself\nwith an appropriate \"From:\" line. :-(\n\n- Thomas\n\n\nJohannes Schindelin (2):\n Allow git-apply to ignore the hunk headers (AKA recountdiff)\n git-add: introduce --edit (to edit the diff vs. the index)\n\nThomas Rast (1):\n git-add--interactive: manual hunk editing mode\n\n Documentation/git-add.txt | 13 ++++-\n Documentation/git-apply.txt | 7 ++-\n builtin-add.c | 55 ++++++++++++++++++-\n builtin-apply.c | 64 ++++++++++++++++++++--\n git-add--interactive.perl | 124 +++++++++++++++++++++++++++++++++++++++++-\n t/t3701-add-interactive.sh | 67 +++++++++++++++++++++++\n t/t3702-add-edit.sh | 126 +++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 448 insertions(+), 8 deletions(-)\n create mode 100755 t/t3702-add-edit.sh\n"},{"id":"80950","messageId":"200806242108.16379.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080624050901.GA19224@sigill.intra.peff.net","subject":"[PATCH 1/3] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-24T19:08:15Z","receivedAt":"2008-06-24T19:08:15Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"From: Johannes Schindelin \n\nSometimes, the easiest way to fix up a patch is to edit it directly, even\nadding or deleting lines. Now, many people are not as divine as certain\nbenevolent dictators as to update the hunk headers correctly at the first\ntry.\n\nSo teach the tool to do it for us.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Thomas Rast \n---\n Documentation/git-apply.txt | 7 ++++-\n builtin-apply.c | 64 ++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 66 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex c834763..b8ab6ed 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git-apply' [--stat] [--numstat] [--summary] [--check] [--index]\n \t [--apply] [--no-add] [--build-fake-ancestor ] [-R | --reverse]\n \t [--allow-binary-replacement | --binary] [--reject] [-z]\n-\t [-pNUM] [-CNUM] [--inaccurate-eof] [--cached]\n+\t [-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]\n \t [--whitespace=]\n \t [--exclude=PATH] [--verbose] [...]\n \n@@ -171,6 +171,11 @@ behavior:\n \tcorrectly. This option adds support for applying such patches by\n \tworking around this bug.\n \n+--recount::\n+\tDo not trust the line counts in the hunk headers, but infer them\n+\tby inspecting the patch (e.g. after editing the patch without\n+\tadjusting the hunk headers appropriately).\n+\n -v::\n --verbose::\n \tReport progress to stderr. By default, only a message about the\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex c497889..34c220f 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -153,6 +153,7 @@ struct patch {\n \tunsigned int is_binary:1;\n \tunsigned int is_copy:1;\n \tunsigned int is_rename:1;\n+\tunsigned int recount:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -882,6 +883,50 @@ static int parse_range(const char *line, int len, int offset, const char *expect\n \treturn offset + ex;\n }\n \n+static int recount_diff(char *line, int size, struct fragment *fragment)\n+{\n+\tint line_nr = 0;\n+\n+\tif (size < 1)\n+\t\treturn -1;\n+\n+\tfragment->oldpos = 2;\n+\tfragment->oldlines = fragment->newlines = 0;\n+\n+\tfor (;;) {\n+\t\tint len = linelen(line, size);\n+\t\tsize -= len;\n+\t\tline += len;\n+\n+\t\tif (size < 1)\n+\t\t\treturn 0;\n+\n+\t\tswitch (*line) {\n+\t\tcase ' ': case '\\n':\n+\t\t\tfragment->newlines++;\n+\t\t\t/* fall through */\n+\t\tcase '-':\n+\t\t\tfragment->oldlines++;\n+\t\t\tbreak;\n+\t\tcase '+':\n+\t\t\tfragment->newlines++;\n+\t\t\tif (line_nr == 0) {\n+\t\t\t\tfragment->leading = 1;\n+\t\t\t\tfragment->oldpos = 1;\n+\t\t\t}\n+\t\t\tfragment->trailing = 1;\n+\t\t\tbreak;\n+\t\tcase '@':\n+\t\t\treturn size < 3 || prefixcmp(line, \"@@ \");\n+\t\tcase 'd':\n+\t\t\treturn size < 5 || prefixcmp(line, \"diff \");\n+\t\tdefault:\n+\t\t\treturn -1;\n+\t\t}\n+\t\tline_nr++;\n+\t}\n+}\n+\n /*\n * Parse a unified diff fragment header of the\n * form \"@@ -a,b +c,d @@\"\n@@ -1013,6 +1058,9 @@ static int parse_fragment(char *line, unsigned long size,\n \toffset = parse_fragment_header(line, len, fragment);\n \tif (offset < 0)\n \t\treturn -1;\n+\tif (offset > 0 && patch->recount &&\n+\t\t\trecount_diff(line + offset, size - offset, fragment))\n+\t\treturn -1;\n \toldlines = fragment->oldlines;\n \tnewlines = fragment->newlines;\n \tleading = 0;\n@@ -2912,7 +2960,8 @@ static void prefix_patches(struct patch *p)\n \t}\n }\n \n-static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n+static int apply_patch(int fd, const char *filename, int inaccurate_eof,\n+\t\tint recount)\n {\n \tsize_t offset;\n \tstruct strbuf buf;\n@@ -2929,6 +2978,7 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n \n \t\tpatch = xcalloc(1, sizeof(*patch));\n \t\tpatch->inaccurate_eof = inaccurate_eof;\n+\t\tpatch->recount = recount;\n \t\tnr = parse_chunk(buf.buf + offset, buf.len - offset, patch);\n \t\tif (nr < 0)\n \t\t\tbreak;\n@@ -2998,6 +3048,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \tint i;\n \tint read_stdin = 1;\n \tint inaccurate_eof = 0;\n+\tint recount = 0;\n \tint errs = 0;\n \tint is_not_gitdir;\n \n@@ -3015,7 +3066,8 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\tint fd;\n \n \t\tif (!strcmp(arg, \"-\")) {\n-\t\t\terrs |= apply_patch(0, \"\", inaccurate_eof);\n+\t\t\terrs |= apply_patch(0, \"\", inaccurate_eof,\n+\t\t\t\t\trecount);\n \t\t\tread_stdin = 0;\n \t\t\tcontinue;\n \t\t}\n@@ -3118,6 +3170,10 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\tinaccurate_eof = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(arg, \"--recount\")) {\n+\t\t\trecount = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (0 < prefix_length)\n \t\t\targ = prefix_filename(prefix, prefix_length, arg);\n \n@@ -3126,12 +3182,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\tdie(\"can't open patch '%s': %s\", arg, strerror(errno));\n \t\tread_stdin = 0;\n \t\tset_default_whitespace_mode(whitespace_option);\n-\t\terrs |= apply_patch(fd, arg, inaccurate_eof);\n+\t\terrs |= apply_patch(fd, arg, inaccurate_eof, recount);\n \t\tclose(fd);\n \t}\n \tset_default_whitespace_mode(whitespace_option);\n \tif (read_stdin)\n-\t\terrs |= apply_patch(0, \"\", inaccurate_eof);\n+\t\terrs |= apply_patch(0, \"\", inaccurate_eof, recount);\n \tif (whitespace_error) {\n \t\tif (squelch_whitespace_errors &&\n \t\t squelch_whitespace_errors < whitespace_error) {\n-- \n1.5.6.84.ge5c1\n"},{"id":"80953","messageId":"200806242108.24207.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080624050901.GA19224@sigill.intra.peff.net","subject":"[PATCH 2/3] git-add: introduce --edit (to edit the diff vs. the index)","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-24T19:08:22Z","receivedAt":"2008-06-24T19:08:22Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"From: Johannes Schindelin \n\nWith \"git add -e []\", Git will fire up an editor with the current\ndiff relative to the index (i.e. what you would get with \"git diff\n[]\").\n\nNow you can edit the patch as much as you like, including adding/removing\nlines, editing the text, whatever. Make sure, though, that the first\ncharacter of the hunk lines is still a space, a plus or a minus.\n\nAfter you closed the editor, Git will adjust the line counts of the\nhunks if necessary, thanks to the --fixup-line-counts option of apply,\nand commit the patch. Except if you deleted everything, in which case\nnothing happens (for obvious reasons).\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Thomas Rast \n---\n Documentation/git-add.txt | 12 ++++-\n builtin-add.c | 55 +++++++++++++++++++-\n t/t3702-add-edit.sh | 126 +++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 191 insertions(+), 2 deletions(-)\n create mode 100755 t/t3702-add-edit.sh\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex b8e3fa6..c6de028 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git-add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]\n-\t [--update | -u] [--refresh] [--ignore-errors] [--]\n+\t [--edit | -e] [--update | -u] [--refresh] [--ignore-errors] [--]\n \t ...\n \n DESCRIPTION\n@@ -76,6 +76,16 @@ OPTIONS\n \tbypassed and the 'patch' subcommand is invoked using each of\n \tthe specified filepatterns before exiting.\n \n+-e::\n+--edit::\n+\tOpen the diff vs. the index in an editor and let the user\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+\n -u::\n --update::\n \tUpdate only files that git already knows about, staging modified\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 9930cf5..b9e9a17 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -19,7 +19,7 @@ static const char * const builtin_add_usage[] = {\n \t\"git-add [options] [--] ...\",\n \tNULL\n };\n-static int patch_interactive = 0, add_interactive = 0;\n+static int patch_interactive = 0, add_interactive = 0, edit_interactive = 0;\n static int take_worktree_changes;\n \n static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n@@ -186,6 +186,56 @@ int interactive_add(int argc, const char **argv, const char *prefix)\n \treturn status;\n }\n \n+int edit_patch(int argc, const char **argv, const char *prefix)\n+{\n+\tchar *file = xstrdup(git_path(\"ADD_EDIT.patch\"));\n+\tconst char *apply_argv[] = { \"apply\", \"--recount\", \"--cached\",\n+\t\tfile, NULL };\n+\tstruct child_process child;\n+\tint result = 0, ac;\n+\tstruct stat st;\n+\n+\tmemset(&child, 0, sizeof(child));\n+\tchild.argv = xcalloc(sizeof(const char *), (argc + 5));\n+\tac = 0;\n+\tchild.git_cmd = 1;\n+\tchild.argv[ac++] = \"diff-files\";\n+\tchild.argv[ac++] = \"--no-color\";\n+\tchild.argv[ac++] = \"-p\";\n+\tchild.argv[ac++] = \"--\";\n+\tif (argc) {\n+\t\tconst char **pathspec = validate_pathspec(argc, argv, prefix);\n+\t\tif (!pathspec)\n+\t\t\treturn -1;\n+\t\tmemcpy(&(child.argv[ac]), pathspec, sizeof(*argv) * argc);\n+\t\tac += argc;\n+\t}\n+\tchild.argv[ac] = NULL;\n+\tchild.out = open(file, O_CREAT | O_WRONLY, 0644);\n+\tresult = child.out < 0 && error(\"Could not write to '%s'\", file);\n+\n+\tif (!result)\n+\t\tresult = run_command(&child);\n+\tfree(child.argv);\n+\n+\tlaunch_editor(file, NULL, NULL);\n+\n+\tif (!result)\n+\t\tresult = stat(file, &st) && error(\"Could not stat '%s'\", file);\n+\tif (!result && !st.st_size)\n+\t\tresult = error(\"Empty patch. Aborted.\");\n+\n+\tmemset(&child, 0, sizeof(child));\n+\tchild.git_cmd = 1;\n+\tchild.argv = apply_argv;\n+\tif (!result)\n+\t\tresult = run_command(&child) &&\n+\t\t\terror(\"Could not apply '%s'\", file);\n+\tif (!result)\n+\t\tunlink(file);\n+\treturn result;\n+}\n+\n static struct lock_file lock_file;\n \n static const char ignore_error[] =\n@@ -202,6 +252,7 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactive patching\"),\n \tOPT_BOOLEAN('f', \"force\", &ignored_too, \"allow adding otherwise ignored files\"),\n \tOPT_BOOLEAN('u', \"update\", &take_worktree_changes, \"update tracked files\"),\n+\tOPT_BOOLEAN('e', \"edit\", &edit_interactive, \"super-interactive patching\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\n \tOPT_BOOLEAN( 0 , \"ignore-errors\", &ignore_add_errors, \"just skip files which cannot be added because of errors\"),\n \tOPT_END(),\n@@ -226,6 +277,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, builtin_add_options,\n \t\t\t builtin_add_usage, 0);\n+\tif (edit_interactive)\n+\t\treturn(edit_patch(argc, argv, prefix));\n \tif (patch_interactive)\n \t\tadd_interactive = 1;\n \tif (add_interactive)\ndiff --git a/t/t3702-add-edit.sh b/t/t3702-add-edit.sh\nnew file mode 100755\nindex 0000000..decf727\n--- /dev/null\n+++ b/t/t3702-add-edit.sh\n@@ -0,0 +1,126 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Johannes E. Schindelin\n+#\n+\n+test_description='add -e basic tests'\n+. ./test-lib.sh\n+\n+\n+cat > file << EOF\n+LO, praise of the prowess of people-kings\n+of spear-armed Danes, in days long sped,\n+we have heard, and what honor the athelings won!\n+Oft Scyld the Scefing from squadroned foes,\n+from many a tribe, the mead-bench tore,\n+awing the earls. Since erst he lay\n+friendless, a foundling, fate repaid him:\n+for he waxed under welkin, in wealth he throve,\n+till before him the folk, both far and near,\n+who house by the whale-path, heard his mandate,\n+gave him gifts: a good king he!\n+EOF\n+\n+test_expect_success 'setup' '\n+\n+\tgit add file &&\n+\ttest_tick &&\n+\tgit commit -m initial file\n+\n+'\n+\n+cat > patch << EOF\n+diff --git a/file b/file\n+index b9834b5..ef6e94c 100644\n+--- a/file\n++++ b/file\n+@@ -3,1 +3,333 @@ of spear-armed Danes, in days long sped,\n+ we have heard, and what honor the athelings won!\n++\n+ Oft Scyld the Scefing from squadroned foes,\n+@@ -2,7 +1,5 @@ awing the earls. Since erst he lay\n+ friendless, a foundling, fate repaid him:\n++\n+ for he waxed under welkin, in wealth he throve,\n+EOF\n+\n+cat > expected << EOF\n+diff --git a/file b/file\n+index b9834b5..ef6e94c 100644\n+--- a/file\n++++ b/file\n+@@ -1,10 +1,12 @@\n+ LO, praise of the prowess of people-kings\n+ of spear-armed Danes, in days long sped,\n+ we have heard, and what honor the athelings won!\n++\n+ Oft Scyld the Scefing from squadroned foes,\n+ from many a tribe, the mead-bench tore,\n+ awing the earls. Since erst he lay\n+ friendless, a foundling, fate repaid him:\n++\n+ for he waxed under welkin, in wealth he throve,\n+ till before him the folk, both far and near,\n+ who house by the whale-path, heard his mandate,\n+EOF\n+\n+echo \"#!$SHELL_PATH\" >fake-editor.sh\n+cat >> fake-editor.sh <<\\EOF\n+mv -f \"$1\" orig-patch &&\n+mv -f patch \"$1\"\n+EOF\n+\n+test_set_editor \"$(pwd)/fake-editor.sh\"\n+chmod a+x fake-editor.sh\n+\n+test_expect_success 'add -e' '\n+\n+\tcp fake-editor.sh file &&\n+\tgit add -e &&\n+\ttest_cmp fake-editor.sh file &&\n+\tgit diff --cached > out &&\n+\ttest_cmp out expected\n+\n+'\n+\n+cat > patch << EOF\n+diff --git a/file b/file\n+--- a/file\n++++ b/file\n+@@ -1,1 +1,1 @@\n+ gave him gifts: a good king he!\n++\n+EOF\n+\n+test_expect_success 'add -e adds to the end of the file' '\n+\n+\ttest_tick &&\n+\tgit commit -m update &&\n+\tgit checkout &&\n+\tgit add -e &&\n+\tgit diff --cached > out &&\n+\ttest \"\" = \"$(git show :file | tail -n 1)\"\n+\n+'\n+\n+cat > patch << EOF\n+diff --git a/file b/file\n+--- a/file\n++++ b/file\n+@@ -1,1 +1,1 @@\n++\n+ LO, praise of the prowess of people-kings\n+EOF\n+\n+test_expect_success 'add -e adds to the beginning of the file' '\n+\n+\ttest_tick &&\n+\tgit commit -m update &&\n+\tgit checkout &&\n+\tgit add -e &&\n+\tgit diff --cached > out &&\n+\ttest \"\" = \"$(git show :file | head -n 1)\"\n+\n+'\n+\n+test_done\n-- \n1.5.6.84.ge5c1\n"},{"id":"80952","messageId":"200806242108.30677.trast@student.ethz.ch","threadId":"13641","inReplyTo":"20080624050901.GA19224@sigill.intra.peff.net","subject":"[PATCH 3/3] git-add--interactive: manual hunk editing mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-06-24T19:08:29Z","receivedAt":"2008-06-24T19:08:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Adds a new option 'e' to the 'add -p' command loop that lets you edit\nthe current hunk in your favourite editor.\n\nIf the resulting patch applies cleanly, the edited hunk will\nimmediately be marked for staging. If it does not apply cleanly, you\nwill be given an opportunity to edit again. If all lines of the hunk\nare removed, then the edit is aborted and the hunk is left unchanged.\n\nApplying the changed hunk(s) relies on Johannes Schindelin's new\n--recount option for git-apply.\n\nNote that the \"real patch\" test intentionally uses\n (echo e; echo n; echo d) | git add -p\neven though the 'n' and 'd' are superfluous at first sight. They\nserve to get out of the interaction loop if git add -p wrongly\nconcludes the patch does not apply.\n\nMany thanks to Jeff King for lots of help and\nsuggestions.\n\nSigned-off-by: Thomas Rast \n---\n Documentation/git-add.txt | 1 +\n git-add--interactive.perl | 124 +++++++++++++++++++++++++++++++++++++++++++-\n t/t3701-add-interactive.sh | 67 ++++++++++++++++++++++++\n 3 files changed, 191 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex c6de028..1d8d209 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -246,6 +246,7 @@ patch::\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+ e - manually edit the current hunk\n ? - print help\n +\n After deciding the fate for all hunks, if there is any hunk\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 903953e..6bb117a 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,7 @@\n \n use strict;\n use Git;\n+use File::Temp;\n \n my $repo = Git->repository();\n \n@@ -18,6 +19,18 @@ my ($fraginfo_color) =\n \t$diff_use_color ? (\n \t\t$repo->get_color('color.diff.frag', 'cyan'),\n \t) : ();\n+my ($diff_plain_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.plain', ''),\n+\t) : ();\n+my ($diff_old_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.old', 'red'),\n+\t) : ();\n+my ($diff_new_color) =\n+\t$diff_use_color ? (\n+\t\t$repo->get_color('color.diff.new', 'green'),\n+\t) : ();\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n@@ -770,6 +783,104 @@ sub coalesce_overlapping_hunks {\n \treturn @out;\n }\n \n+sub color_diff {\n+\treturn map {\n+\t\tcolored((/^@/ ? $fraginfo_color :\n+\t\t\t /^\\+/ ? $diff_new_color :\n+\t\t\t /^-/ ? $diff_old_color :\n+\t\t\t $diff_plain_color),\n+\t\t\t$_);\n+\t} @_;\n+}\n+\n+sub edit_hunk_manually {\n+\tmy ($oldtext) = @_;\n+\n+\tmy $t = File::Temp->new(\n+\t\tTEMPLATE => $repo->repo_path . \"/git-hunk-edit.XXXXXX\",\n+\t\tSUFFIX => '.diff'\n+\t);\n+\tprint $t \"# Manual hunk edit mode -- see bottom for a quick guide\\n\";\n+\tprint $t @$oldtext;\n+\tprint $t <config(\"core.editor\")\n+\t\t|| $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n+\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $t);\n+\n+\topen my $fh, '<', $t\n+\t\tor die \"failed to open hunk edit file for reading: \" . $!;\n+\tmy @newtext = grep { !/^#/ } <$fh>;\n+\tclose $fh;\n+\n+\t# Abort if nothing remains\n+\tif (!grep { /\\S/ } @newtext) {\n+\t\treturn undef;\n+\t}\n+\n+\t# Reinsert the first hunk header if the user accidentally deleted it\n+\tif ($newtext[0] !~ /^@/) {\n+\t\tunshift @newtext, $oldtext->[0];\n+\t}\n+\treturn \\@newtext;\n+}\n+\n+sub diff_applies {\n+\tmy $fh;\n+\topen $fh, '| git apply --recount --cached --check';\n+\tfor my $h (@_) {\n+\t\tprint $fh @{$h->{TEXT}};\n+\t}\n+\treturn close $fh;\n+}\n+\n+sub prompt_yesno {\n+\tmy ($prompt) = @_;\n+\twhile (1) {\n+\t\tprint colored $prompt_color, $prompt;\n+\t\tmy $line = ;\n+\t\treturn 0 if $line =~ /^n/i;\n+\t\treturn 1 if $line =~ /^y/i;\n+\t}\n+}\n+\n+sub edit_hunk_loop {\n+\tmy ($head, $hunk, $ix) = @_;\n+\tmy $text = $hunk->[$ix]->{TEXT};\n+\n+\twhile (1) {\n+\t\t$text = edit_hunk_manually($text);\n+\t\tif (!defined $text) {\n+\t\t\treturn undef;\n+\t\t}\n+\t\tmy $newhunk = { TEXT => $text, USE => 1 };\n+\t\tif (diff_applies($head,\n+\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t $newhunk,\n+\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n+\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$text})];\n+\t\t\treturn $newhunk;\n+\t\t}\n+\t\telse {\n+\t\t\tprompt_yesno(\n+\t\t\t\t'Your edited hunk does not apply. Edit again '\n+\t\t\t\t. '(saying \"no\" discards!) [y/n]? '\n+\t\t\t\t) or return undef;\n+\t\t}\n+\t}\n+}\n+\n sub help_patch_cmd {\n \tprint colored $help_color, <<\\EOF ;\n y - stage this hunk\n@@ -781,6 +892,7 @@ J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n+e - manually edit the current hunk\n ? - print help\n EOF\n }\n@@ -846,6 +958,7 @@ sub patch_update_file {\n \n \t$num = scalar @hunk;\n \t$ix = 0;\n+\tmy $need_recount = 0;\n \n \twhile (1) {\n \t\tmy ($prev, $next, $other, $undecided, $i);\n@@ -885,6 +998,7 @@ sub patch_update_file {\n \t\tif (hunk_splittable($hunk[$ix]{TEXT})) {\n \t\t\t$other .= '/s';\n \t\t}\n+\t\t$other .= '/e';\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -949,6 +1063,13 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tmy $newhunk = edit_hunk_loop($head, \\@hunk, $ix);\n+\t\t\t\tif (defined $newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t\t$need_recount = 1;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n \t\t\t\tnext;\n@@ -1002,7 +1123,8 @@ sub patch_update_file {\n \tif (@result) {\n \t\tmy $fh;\n \n-\t\topen $fh, '| git apply --cached';\n+\t\topen $fh, '| git apply --cached'\n+\t\t\t. ($need_recount ? ' --recount' : '');\n \t\tfor (@{$head->{TEXT}}, @result) {\n \t\t\tprint $fh $_;\n \t\t}\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex fae64ea..e95663d 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -66,6 +66,73 @@ test_expect_success 'revert works (commit)' '\n \tgrep \"unchanged *+3/-0 file\" output\n '\n \n+cat >expected <fake_editor.sh < diff &&\n+\ttest_cmp expected diff\n+'\n+\n+cat >patch <fake_editor.sh\n+cat >>fake_editor.sh <<\\EOF\n+mv -f \"$1\" oldpatch &&\n+mv -f patch \"$1\"\n+EOF\n+chmod a+x fake_editor.sh\n+test_set_editor \"$(pwd)/fake_editor.sh\"\n+test_expect_success 'bad edit rejected' '\n+\tgit reset &&\n+\t(echo e; echo n; echo d) | git add -p >output &&\n+\tgrep \"hunk does not apply\" output\n+'\n+\n+cat >patch <output &&\n+\tgrep \"hunk does not apply\" output\n+'\n+\n+cat >patch <expected <output &&\n+\ttest_cmp expected output\n+'\n+\n if test \"$(git config --bool core.filemode)\" = false\n then\n say 'skipping filemode tests (filesystem does not properly support modes)'\n-- \n1.5.6.84.ge5c1\n"},{"id":"80968","messageId":"20080624195334.GC29404@genesis.frugalware.org","threadId":"13641","inReplyTo":"200806242108.08654.trast@student.ethz.ch","subject":"Re: [PATCH 0/3] Manual editing for 'add' and 'add -p'","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-24T19:53:34Z","receivedAt":"2008-06-24T19:53:34Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Tue, Jun 24, 2008 at 09:07:54PM +0200, Thomas Rast wrote:\n> I hope this series turns out right in mail; I'm struggling a bit to\n> properly import the mails to KMail. Most annoyingly, format-patch\n> apparently cannot turn other people's patches into mails by myself\n> with an appropriate \"From:\" line. :-(\n\nWhy don't you just use git-send-email? It already handles the situation.\n"},{"id":"81023","messageId":"7vfxr29zju.fsf@gitster.siamese.dyndns.org","threadId":"13641","inReplyTo":"200806242108.16379.trast@student.ethz.ch","subject":"Re: [PATCH 1/3] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-24T23:35:33Z","receivedAt":"2008-06-24T23:35:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index c497889..34c220f 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -153,6 +153,7 @@ struct patch {\n> \tunsigned int is_binary:1;\n> \tunsigned int is_copy:1;\n> \tunsigned int is_rename:1;\n> +\tunsigned int recount:1;\n> \tstruct fragment *fragments;\n> \tchar *result;\n> \tsize_t resultsize;\n\nWhy doesn't anybody find this quite wrong?\n\nWhat is a \"struct patch\"? It describes a change to a single file\n(i.e. information contained from one \"diff --git\" til next \"diff --git\"),\ngroups the hunks (called \"fragments\") together and holds the postimage\nafter applying these hunks. Is this new \"recount\" field a per file\nattribute?\n\n> +\tfragment->oldpos = 2;\n> +\tfragment->oldlines = fragment->newlines = 0;\n\nWhy is this discarding the position information? \n\nIt is none of this function's business. The ONLY THING this function\nshould do is to discard fragment->oldlines and fragment->newlines, count\nthe contents and update these two fields, and nothing else. Touching\noldpos, leading and other things is an absolute no-no.\n\n> @@ -1013,6 +1058,9 @@ static int parse_fragment(char *line, unsigned long size,\n> \toffset = parse_fragment_header(line, len, fragment);\n> \tif (offset < 0)\n> \t\treturn -1;\n> +\tif (offset > 0 && patch->recount &&\n> +\t\t\trecount_diff(line + offset, size - offset, fragment))\n> +\t\treturn -1;\n\nAnd recount should not cause parse_fragment() to fail out either. If you\nmiscounted, the codepath that follows this part knows how to handle broken\npatch correctly anyway.\n\nI think I've already mentioned the above two points when this was\noriginally posted.\n\nSomewhat disgusted...\n"},{"id":"81075","messageId":"20080625054519.GC2209@sigill.intra.peff.net","threadId":"13641","inReplyTo":"7vfxr29zju.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-25T05:45:19Z","receivedAt":"2008-06-25T05:45:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 24, 2008 at 04:35:33PM -0700, Junio C Hamano wrote:\n\n> I think I've already mentioned the above two points when this was\n> originally posted.\n> \n> Somewhat disgusted...\n\nSorry, this is my fault. I hadn't looked at the recount patches closely,\nbut I had thought they were submit-worthy, so I encouraged Thomas to\ninclude them in his series (since his patch depends on them).\n\nI'll try to look them over more closely in the next day or two.\n\n-Peff\n"},{"id":"81447","messageId":"alpine.DEB.1.00.0806271823220.9925@racer","threadId":"13641","inReplyTo":"7vfxr29zju.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-27T17:43:09Z","receivedAt":"2008-06-27T17:43:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 24 Jun 2008, Junio C Hamano wrote:\n\n> Thomas Rast writes:\n> \n> > diff --git a/builtin-apply.c b/builtin-apply.c\n> > index c497889..34c220f 100644\n> > --- a/builtin-apply.c\n> > +++ b/builtin-apply.c\n> > @@ -153,6 +153,7 @@ struct patch {\n> > \tunsigned int is_binary:1;\n> > \tunsigned int is_copy:1;\n> > \tunsigned int is_rename:1;\n> > +\tunsigned int recount:1;\n> > \tstruct fragment *fragments;\n> > \tchar *result;\n> > \tsize_t resultsize;\n> \n> Why doesn't anybody find this quite wrong?\n> \n> What is a \"struct patch\"? It describes a change to a single file\n> (i.e. information contained from one \"diff --git\" til next \"diff --git\"),\n> groups the hunks (called \"fragments\") together and holds the postimage\n> after applying these hunks. Is this new \"recount\" field a per file\n> attribute?\n\nActually, it is not. But then, it is an attribute of the patch: it will \nbe recounted.\n\nIn addition, the patch would get quite large and unwieldy with \"recount\" \nbeing passed between the functions, because we do not have \"apply_options\" \nyet.\n\nI was even briefly working on apply_options, but this would be a _huge_ \npatch (I had an initial working version, but given my limited time, I \ncould not clean it up for submission anyway, so I scrapped it).\n\n> > +\tfragment->oldpos = 2;\n> > +\tfragment->oldlines = fragment->newlines = 0;\n> \n> Why is this discarding the position information?\n\nSorry, I forgot.\n\n> > @@ -1013,6 +1058,9 @@ static int parse_fragment(char *line, unsigned long size,\n> > \toffset = parse_fragment_header(line, len, fragment);\n> > \tif (offset < 0)\n> > \t\treturn -1;\n> > +\tif (offset > 0 && patch->recount &&\n> > +\t\t\trecount_diff(line + offset, size - offset, fragment))\n> > +\t\treturn -1;\n> \n> And recount should not cause parse_fragment() to fail out either. If \n> you miscounted, the codepath that follows this part knows how to handle \n> broken patch correctly anyway.\n\nOkay.\n\n> I think I've already mentioned the above two points when this was \n> originally posted.\n> \n> Somewhat disgusted...\n\nSorry.\n\nThis is my updated patch (making changes to my \"add -e\" patch, which I \nmay submit later), according to your comments so far:\n\n-- snipsnap --\n[PATCH] Allow git-apply to recount the lines in a hunk (AKA recountdiff)\n\nSometimes, the easiest way to fix up a patch is to edit it directly, even\nadding or deleting lines. Now, many people are not as divine as certain\nbenevolent dictators as to update the hunk headers correctly at the first\ntry.\n\nSo teach the tool to do it for us.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-apply.txt | 7 +++-\n builtin-apply.c | 76 +++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 75 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex c834763..c5ee636 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git-apply' [--stat] [--numstat] [--summary] [--check] [--index]\n \t [--apply] [--no-add] [--build-fake-ancestor ] [-R | --reverse]\n \t [--allow-binary-replacement | --binary] [--reject] [-z]\n-\t [-pNUM] [-CNUM] [--inaccurate-eof] [--cached]\n+\t [-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]\n \t [--whitespace=]\n \t [--exclude=PATH] [--verbose] [...]\n \n@@ -177,6 +177,11 @@ behavior:\n \tcurrent patch being applied will be printed. This option will cause\n \tadditional information to be reported.\n \n+--recount::\n+\tDo not trust the line counts in the hunk headers, but infer them\n+\tby inspecting the patch (e.g. after editing the patch without\n+\tadjusting the hunk headers appropriately).\n+\n Configuration\n -------------\n \ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex c497889..c819652 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -153,6 +153,7 @@ struct patch {\n \tunsigned int is_binary:1;\n \tunsigned int is_copy:1;\n \tunsigned int is_rename:1;\n+\tunsigned int recount:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -882,6 +883,57 @@ static int parse_range(const char *line, int len, int offset, const char *expect\n \treturn offset + ex;\n }\n \n+static int recount_diff(char *line, int size, struct fragment *fragment)\n+{\n+\tint oldlines = 0, newlines = 0, ret = 0;\n+\n+\tif (size < 1) {\n+\t\twarning(\"recount: ignore empty hunk\");\n+\t\treturn -1;\n+\t}\n+\n+\tfor (;;) {\n+\t\tint len = linelen(line, size);\n+\t\tsize -= len;\n+\t\tline += len;\n+\n+\t\tif (size < 1)\n+\t\t\tbreak;\n+\n+\t\tswitch (*line) {\n+\t\tcase ' ': case '\\n':\n+\t\t\tnewlines++;\n+\t\t\t/* fall through */\n+\t\tcase '-':\n+\t\t\toldlines++;\n+\t\t\tcontinue;\n+\t\tcase '+':\n+\t\t\tnewlines++;\n+\t\t\tcontinue;\n+\t\tcase '\\\\':\n+\t\t\tbreak;\n+\t\tcase '@':\n+\t\t\tret = size < 3 || prefixcmp(line, \"@@ \");\n+\t\t\tbreak;\n+\t\tcase 'd':\n+\t\t\tret = size < 5 || prefixcmp(line, \"diff \");\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tret = -1;\n+\t\t\tbreak;\n+\t\t}\n+\t\tif (ret) {\n+\t\t\twarning(\"recount: unexpected line: %.*s\",\n+\t\t\t\t(int)linelen(line, size), line);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tbreak;\n+\t}\n+\tfragment->oldlines = oldlines;\n+\tfragment->newlines = newlines;\n+\treturn 0;\n+}\n+\n /*\n * Parse a unified diff fragment header of the\n * form \"@@ -a,b +c,d @@\"\n@@ -1013,6 +1065,8 @@ static int parse_fragment(char *line, unsigned long size,\n \toffset = parse_fragment_header(line, len, fragment);\n \tif (offset < 0)\n \t\treturn -1;\n+\tif (offset > 0 && patch->recount)\n+\t\trecount_diff(line + offset, size - offset, fragment);\n \toldlines = fragment->oldlines;\n \tnewlines = fragment->newlines;\n \tleading = 0;\n@@ -2912,7 +2966,10 @@ static void prefix_patches(struct patch *p)\n \t}\n }\n \n-static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n+#define INACCURATE_EOF\t(1<<0)\n+#define RECOUNT\t\t(1<<1)\n+\n+static int apply_patch(int fd, const char *filename, int options)\n {\n \tsize_t offset;\n \tstruct strbuf buf;\n@@ -2928,7 +2985,8 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n \t\tint nr;\n \n \t\tpatch = xcalloc(1, sizeof(*patch));\n-\t\tpatch->inaccurate_eof = inaccurate_eof;\n+\t\tpatch->inaccurate_eof = !!(options & INACCURATE_EOF);\n+\t\tpatch->recount = !!(options & RECOUNT);\n \t\tnr = parse_chunk(buf.buf + offset, buf.len - offset, patch);\n \t\tif (nr < 0)\n \t\t\tbreak;\n@@ -2997,7 +3055,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n {\n \tint i;\n \tint read_stdin = 1;\n-\tint inaccurate_eof = 0;\n+\tint options = 0;\n \tint errs = 0;\n \tint is_not_gitdir;\n \n@@ -3015,7 +3073,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\tint fd;\n \n \t\tif (!strcmp(arg, \"-\")) {\n-\t\t\terrs |= apply_patch(0, \"\", inaccurate_eof);\n+\t\t\terrs |= apply_patch(0, \"\", options);\n \t\t\tread_stdin = 0;\n \t\t\tcontinue;\n \t\t}\n@@ -3115,7 +3173,11 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--inaccurate-eof\")) {\n-\t\t\tinaccurate_eof = 1;\n+\t\t\toptions |= INACCURATE_EOF;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"--recount\")) {\n+\t\t\toptions |= RECOUNT;\n \t\t\tcontinue;\n \t\t}\n \t\tif (0 < prefix_length)\n@@ -3126,12 +3188,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\tdie(\"can't open patch '%s': %s\", arg, strerror(errno));\n \t\tread_stdin = 0;\n \t\tset_default_whitespace_mode(whitespace_option);\n-\t\terrs |= apply_patch(fd, arg, inaccurate_eof);\n+\t\terrs |= apply_patch(fd, arg, options);\n \t\tclose(fd);\n \t}\n \tset_default_whitespace_mode(whitespace_option);\n \tif (read_stdin)\n-\t\terrs |= apply_patch(0, \"\", inaccurate_eof);\n+\t\terrs |= apply_patch(0, \"\", options);\n \tif (whitespace_error) {\n \t\tif (squelch_whitespace_errors &&\n \t\t squelch_whitespace_errors < whitespace_error) {\n-- \n1.5.6.173.gde14c\n"}]}