{"thread":{"id":"13809","subject":"[PATCH 1/2] Allow git-apply to fix up the line counts","startedAt":"2008-06-05T10:16:16Z","lastAt":"2008-06-06T17:35:21Z","messageCount":39,"participants":["Johannes Schindelin","Johannes Sixt","Pieter de Bie","Junio C Hamano","Govind Salinas","Paolo Bonzini","Olivier Marin","Sergei Organov"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"78735","messageId":"alpine.DEB.1.00.0806051115570.21190@racer","threadId":"13809","inReplyTo":null,"subject":"[PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T10:16:16Z","receivedAt":"2008-06-05T10:16:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\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 | 6 ++++-\n builtin-apply.c | 55 +++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 56 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex 2dec2ec..ba3dba7 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-line-counts] [--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..3fd80e8 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,6 +883,41 @@ 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 size, struct fragment *fragment)\n+{\n+\tif (size < 1)\n+\t\treturn -1;\n+\n+\tfragment->oldlines = fragment->newlines = -1;\n+\n+\tfor (;;) {\n+\t\tint len = linelen(line, size);\n+\t\tif (!len)\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\t/* Probably \"diff ...\" */\n+\t\t\treturn 0;\n+\t\t}\n+\n+\t\tsize -= len;\n+\t\tline += len;\n+\t\tif (size < 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@@ -1013,6 +1049,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->fixup &&\n+\t\t\tfixup_counts(line + offset, size - offset, fragment))\n+\t\treturn -1;\n \toldlines = fragment->oldlines;\n \tnewlines = fragment->newlines;\n \tleading = 0;\n@@ -2912,7 +2951,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 +2969,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 +3039,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 +3057,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 +3161,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 +3173,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.rc1.181.gb439d\n"},{"id":"78736","messageId":"alpine.DEB.1.00.0806051116360.21190@racer","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051115570.21190@racer","subject":"[PATCH 2/2] git-add: introduce --edit (to edit the diff vs. the index)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T10:17:28Z","receivedAt":"2008-06-05T10:17:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\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 \n---\n\n\tThis was too useful to let slip by. I even committed it using \n\t\"git add -e \" several times!\n\n\tAnyway, bed time.\n\n Documentation/git-add.txt | 9 ++++++-\n builtin-add.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 55 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 1afd0c6..dd744f1 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -8,8 +8,8 @@ git-add - Add file contents to the index\n SYNOPSIS\n --------\n [verse]\n-'git-add' [-n] [-v] [-f] [--interactive | -i] [--patch | -p] [-u] [--refresh]\n-\t [--ignore-errors] [--] ...\n+'git-add' [-n] [-v] [-f] [--interactive | -i] [--patch | -p] [--edit | -e]\n+\t [-u] [--refresh] [--ignore-errors] [--] ...\n \n DESCRIPTION\n -----------\n@@ -70,6 +70,11 @@ OPTIONS\n \tbypassed and the 'patch' subcommand is invoked using each of\n \tthe specified filepatterns before exiting.\n \n+-e, \\--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 -u::\n \tUpdate only files that git already knows about, staging modified\n \tcontent for commit and marking deleted files for removal. This\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 1da22ee..05ae40d 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,50 @@ 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+\tstatic struct lock_file lock;\n+\tstruct child_process child;\n+\tint 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 = hold_lock_file_for_update(&lock, git_path(\"EDIT_PATCH\"), 1);\n+\n+\tif (run_command(&child))\n+\t\treturn 1;\n+\tfree(child.argv);\n+\n+\tlaunch_editor(lock.filename, NULL, NULL);\n+\n+\tif (stat(lock.filename, &st))\n+\t\treturn 1;\n+\tif (!st.st_size) {\n+\t\tfprintf(stderr, \"Empty patch. Aborted.\\n\");\n+\t\treturn 0;\n+\t}\n+\n+\texecl_git_cmd(\"apply\", \"--fixup-line-counts\", \"--cached\",\n+\t\t\tlock.filename, NULL);\n+\n+\treturn 1;\n+}\n+\n static struct lock_file lock_file;\n \n static const char ignore_error[] =\n@@ -200,6 +244,7 @@ static struct option builtin_add_options[] = {\n \tOPT_GROUP(\"\"),\n \tOPT_BOOLEAN('i', \"interactive\", &add_interactive, \"interactive picking\"),\n \tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactive patching\"),\n+\tOPT_BOOLEAN('e', \"edit\", &edit_interactive, \"super-interactive patching\"),\n \tOPT_BOOLEAN('f', NULL, &ignored_too, \"allow adding otherwise ignored files\"),\n \tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update tracked files\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\n@@ -226,6 +271,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)\n-- \n1.5.6.rc1.181.gb439d\n"},{"id":"78747","messageId":"4847CCD9.6000305@viscovery.net","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051115570.21190@racer","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-06-05T11:24:09Z","receivedAt":"2008-06-05T11:24:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> +--fixup-line-counts::\n> +\tFix up the line counts (e.g. after editing the patch without\n> +\tadjusting the hunk headers appropriately).\n\nThis sort of implies that there is some kind of output that tells the\ncorrect line counts. But that isn't the case (if I read the patch\ncorrectly). So I suggest to name the option --ignore-line-counts.\n\n-- Hannes\n"},{"id":"78765","messageId":"alpine.DEB.1.00.0806051403370.21190@racer","threadId":"13809","inReplyTo":"4847CCD9.6000305@viscovery.net","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T13:04:27Z","receivedAt":"2008-06-05T13:04:27Z","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, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\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> This sort of implies that there is some kind of output that tells the\n> correct line counts. But that isn't the case (if I read the patch\n> correctly). So I suggest to name the option --ignore-line-counts.\n\nBut there is some kind of output: the hunks themselves. And the line \ncounts are not ignored, but they are actively rewritten. But if you have \na suggestion which keeps the spirit, I am very interested...\n\nCiao,\nDscho\n"},{"id":"78770","messageId":"4847EBC3.8060509@viscovery.net","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051403370.21190@racer","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-06-05T13:36:03Z","receivedAt":"2008-06-05T13:36:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> Hi,\n> \n> On Thu, 5 Jun 2008, Johannes Sixt wrote:\n> \n>> Johannes Schindelin schrieb:\n>>> +--fixup-line-counts::\n>>> +\tFix up the line counts (e.g. after editing the patch without\n>>> +\tadjusting the hunk headers appropriately).\n>> This sort of implies that there is some kind of output that tells the\n>> correct line counts. But that isn't the case (if I read the patch\n>> correctly). So I suggest to name the option --ignore-line-counts.\n> \n> But there is some kind of output: the hunks themselves.\n\nIs there? I did this (it rewrites all line counts to 1):\n\n$ git diff ..HEAD~1 |\n\tsed -e '/^@@/s/,[0-9]+ /,1 /g' |\n\t./git-apply --fixup-line-counts\n\nand there was no output. Instead, the patch was applied.\n\n> And the line \n> counts are not ignored, but they are actively rewritten.\n\nOf course, internally there is some sort of \"output\" from the fixup\nroutine, and the line counts are rewritten and then are not ignored. But\nthe user doesn't care about this internal procedure. From the user's\nperspective, the line counts of the input patch are ignored.\n\nApart from this color of the bikeshed I like your patch.\n\n-- Hannes\n"},{"id":"78774","messageId":"alpine.DEB.1.00.0806051441560.21190@racer","threadId":"13809","inReplyTo":"4847EBC3.8060509@viscovery.net","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T13:47:24Z","receivedAt":"2008-06-05T13:47:24Z","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, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> \n> > On Thu, 5 Jun 2008, Johannes Sixt wrote:\n> > \n> >> Johannes Schindelin schrieb:\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> >> This sort of implies that there is some kind of output that tells the \n> >> correct line counts. But that isn't the case (if I read the patch \n> >> correctly). So I suggest to name the option --ignore-line-counts.\n> > \n> > But there is some kind of output: the hunks themselves.\n> \n> Is there?\n\nYes!\n\n> I did this (it rewrites all line counts to 1):\n> \n> $ git diff ..HEAD~1 |\n> \tsed -e '/^@@/s/,[0-9]+ /,1 /g' |\n> \t./git-apply --fixup-line-counts\n> \n> and there was no output. Instead, the patch was applied.\n\nAs I said, the data is in the _hunks_, but I maybe should have added _not \nin the hunk headers_.\n\nSo in a very real sense, you edit the hunks, and the hunk headers are \nadjusted to that. You did not adjust the hunks, so they got applied.\n\nIt seems that you think the hunk header's line counts are heeded, and the \nhunk adjusted, with --fixup-line-counts? Sorry, I find that rather \ncounterintuitive.\n\n> > And the line counts are not ignored, but they are actively rewritten.\n> \n> Of course, internally there is some sort of \"output\" from the fixup \n> routine, and the line counts are rewritten and then are not ignored. But \n> the user doesn't care about this internal procedure. From the user's \n> perspective, the line counts of the input patch are ignored.\n\nBut they are not!\n\nThere are _two_ things that are the line counts. Those numbers in the \nhunk header, and the real line counts of the hunks.\n\nNow, if you say they are _ignored_, would that not imply in plain English \nthat they are left unchanged (in limbo, because those two types of numbers \ncontradict each other)?\n\nOkay, how about shikebedding this to --adjust-line-counts?\n\nCiao,\nDscho\n"},{"id":"78777","messageId":"4847F49F.8090004@viscovery.net","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051441560.21190@racer","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-06-05T14:13:51Z","receivedAt":"2008-06-05T14:13:51Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> Hi,\n> \n> On Thu, 5 Jun 2008, Johannes Sixt wrote:\n> \n>> Johannes Schindelin schrieb:\n>>\n>>> On Thu, 5 Jun 2008, Johannes Sixt wrote:\n>>>\n>>>> Johannes Schindelin schrieb:\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>>>> This sort of implies that there is some kind of output that tells the \n>>>> correct line counts. But that isn't the case (if I read the patch \n>>>> correctly). So I suggest to name the option --ignore-line-counts.\n>>> But there is some kind of output: the hunks themselves.\n>> Is there?\n> \n> Yes!\n> \n>> I did this (it rewrites all line counts to 1):\n>>\n>> $ git diff ..HEAD~1 |\n>> \tsed -e '/^@@/s/,[0-9]+ /,1 /g' |\n>> \t./git-apply --fixup-line-counts\n>>\n>> and there was no output. Instead, the patch was applied.\n> \n> As I said, the data is in the _hunks_, but I maybe should have added _not \n> in the hunk headers_.\n\nYes, of course.\n\n> So in a very real sense, you edit the hunks, and the hunk headers are \n> adjusted to that. You did not adjust the hunks, so they got applied.\n\nYes, of course.\n\nBut the example pretends that the hunks have been edited so heavily that\nthey in no way match the line counts in the hunk headers.\n\n> It seems that you think the hunk header's line counts are heeded, and the \n> hunk adjusted, with --fixup-line-counts?\n\nNO, of course *NOT*.\n\n> Sorry, I find that rather \n> counterintuitive.\n\nSo would I.\n\n>>> And the line counts are not ignored, but they are actively rewritten.\n>> Of course, internally there is some sort of \"output\" from the fixup \n>> routine, and the line counts are rewritten and then are not ignored. But \n>> the user doesn't care about this internal procedure. From the user's \n>> perspective, the line counts of the input patch are ignored.\n> \n> But they are not!\n\n> There are _two_ things that are the line counts. Those numbers in the \n> hunk header, and the real line counts of the hunks.\n\nAnd I was always talking about the numbers in the hunk headers.\n\n> Now, if you say they are _ignored_, would that not imply in plain English \n> that they are left unchanged (in limbo, because those two types of numbers \n> contradict each other)?\n\nThat you *internally* rewrite those numbers and then do *not* ignore them\nis totally pointless for the user. It's an implementation detail. The user\ndoesn't see what is going on nor should he care. From the user's\nperspective, the hunk header line counts are _ignored_ (because if they\nwere not ignored, then there would be an error message in the\ncontradicting case).\n\n> Okay, how about shikebedding this to --adjust-line-counts?\n\n>From the user's perspective, nothing is \"adjusted\"; the hunk header line\ncounts are ... you guess it ... *ignored*.\n\n-- Hannes\n"},{"id":"78779","messageId":"alpine.DEB.1.00.0806051548140.21190@racer","threadId":"13809","inReplyTo":"4847F49F.8090004@viscovery.net","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T14:54:04Z","receivedAt":"2008-06-05T14:54:04Z","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, Johannes Sixt wrote:\n\n> > Now, if you say they are _ignored_, would that not imply in plain \n> > English that they are left unchanged (in limbo, because those two \n> > types of numbers contradict each other)?\n> \n> That you *internally* rewrite those numbers and then do *not* ignore \n> them is totally pointless for the user. It's an implementation detail. \n> The user doesn't see what is going on nor should he care. From the \n> user's perspective, the hunk header line counts are _ignored_ (because \n> if they were not ignored, then there would be an error message in the \n> contradicting case).\n> \n> > Okay, how about shikebedding this to --adjust-line-counts?\n> \n> From the user's perspective, nothing is \"adjusted\"; the hunk header line \n> counts are ... you guess it ... *ignored*.\n\nOh... I start to see what you mean. It's just that for me, the line \ncounts are the actual line counts, not what is recorded in the hunk \nheader.\n\nIn any case, I really do not feel strongly about it, since I do not want \nto use it, except with git add -e. Which I really grew fond of in these \nlast hours ;-)\n\nSo how about --ignore-hunk-headers? I think this is much more \ndescriptive, and catches your complaint, IMHO.\n\nCiao,\nDscho\n"},{"id":"78780","messageId":"48480123.7030903@viscovery.net","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051548140.21190@racer","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-06-05T15:07:15Z","receivedAt":"2008-06-05T15:07:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> So how about --ignore-hunk-headers? I think this is much more \n> descriptive, and catches your complaint, IMHO.\n\nYes, that's good as well. :-)\n\n-- Hannes\n"},{"id":"78782","messageId":"alpine.DEB.1.00.0806051719170.21190@racer","threadId":"13809","inReplyTo":"48480123.7030903@viscovery.net","subject":"[PATCH v2 0/2] git add --edit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T16:19:28Z","receivedAt":"2008-06-05T16:19:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nChanges relative to the first version:\n\n- rename the apply option to --ignore-hunk-headers\n\n- add a test\n\nJohannes Schindelin (2):\n Allow git-apply to ignore the hunk headers\n git-add: introduce --edit (to edit the diff vs. the index)\n\n Documentation/git-add.txt | 9 +++-\n Documentation/git-apply.txt | 7 +++-\n builtin-add.c | 47 +++++++++++++++++++++++-\n builtin-apply.c | 57 ++++++++++++++++++++++++++--\n t/t3702-add-edit.sh | 86 +++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 198 insertions(+), 8 deletions(-)\n create mode 100755 t/t3702-add-edit.sh\n"},{"id":"78784","messageId":"alpine.DEB.1.00.0806051720070.21190@racer","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051719170.21190@racer","subject":"[PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T16:20:17Z","receivedAt":"2008-06-05T16:20:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\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 | 57 ++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 59 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex 2dec2ec..e4c5530 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] [--ignore-hunk-headers] [--cached]\n \t [--whitespace=]\n \t [--exclude=PATH] [--verbose] [...]\n \n@@ -169,6 +169,11 @@ behavior:\n \tcorrectly. This option adds support for applying such patches by\n \tworking around this bug.\n \n+--ignore-hunk-headers::\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, --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..b357e35 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 ignore_hunk_headers:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -882,6 +883,41 @@ 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 size, struct fragment *fragment)\n+{\n+\tif (size < 1)\n+\t\treturn -1;\n+\n+\tfragment->oldlines = fragment->newlines = -1;\n+\n+\tfor (;;) {\n+\t\tint len = linelen(line, size);\n+\t\tif (!len)\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\t/* Probably \"diff ...\" */\n+\t\t\treturn 0;\n+\t\t}\n+\n+\t\tsize -= len;\n+\t\tline += len;\n+\t\tif (size < 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@@ -1013,6 +1049,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->ignore_hunk_headers &&\n+\t\t\tfixup_counts(line + offset, size - offset, fragment))\n+\t\treturn -1;\n \toldlines = fragment->oldlines;\n \tnewlines = fragment->newlines;\n \tleading = 0;\n@@ -2912,7 +2951,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 ignore_hunk_headers)\n {\n \tsize_t offset;\n \tstruct strbuf buf;\n@@ -2929,6 +2969,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->ignore_hunk_headers = ignore_hunk_headers;\n \t\tnr = parse_chunk(buf.buf + offset, buf.len - offset, patch);\n \t\tif (nr < 0)\n \t\t\tbreak;\n@@ -2998,6 +3039,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 ignore_hunk_headers = 0;\n \tint errs = 0;\n \tint is_not_gitdir;\n \n@@ -3015,7 +3057,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\tignore_hunk_headers);\n \t\t\tread_stdin = 0;\n \t\t\tcontinue;\n \t\t}\n@@ -3118,6 +3161,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, \"--ignore-hunk-headers\")) {\n+\t\t\tignore_hunk_headers = 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 +3173,14 @@ 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,\n+\t\t\t\tignore_hunk_headers);\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,\n+\t\t\t\tignore_hunk_headers);\n \tif (whitespace_error) {\n \t\tif (squelch_whitespace_errors &&\n \t\t squelch_whitespace_errors < whitespace_error) {\n-- \n1.5.6.rc1.181.gb439d\n"},{"id":"78783","messageId":"alpine.DEB.1.00.0806051720300.21190@racer","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051719170.21190@racer","subject":"[PATCH v2 2/2] git-add: introduce --edit (to edit the diff vs. the index)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T16:20:46Z","receivedAt":"2008-06-05T16:20:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\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 \n---\n Documentation/git-add.txt | 9 ++++-\n builtin-add.c | 47 ++++++++++++++++++++++++-\n t/t3702-add-edit.sh | 86 +++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 139 insertions(+), 3 deletions(-)\n create mode 100755 t/t3702-add-edit.sh\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 1afd0c6..dd744f1 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -8,8 +8,8 @@ git-add - Add file contents to the index\n SYNOPSIS\n --------\n [verse]\n-'git-add' [-n] [-v] [-f] [--interactive | -i] [--patch | -p] [-u] [--refresh]\n-\t [--ignore-errors] [--] ...\n+'git-add' [-n] [-v] [-f] [--interactive | -i] [--patch | -p] [--edit | -e]\n+\t [-u] [--refresh] [--ignore-errors] [--] ...\n \n DESCRIPTION\n -----------\n@@ -70,6 +70,11 @@ OPTIONS\n \tbypassed and the 'patch' subcommand is invoked using each of\n \tthe specified filepatterns before exiting.\n \n+-e, \\--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 -u::\n \tUpdate only files that git already knows about, staging modified\n \tcontent for commit and marking deleted files for removal. This\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 1da22ee..216f331 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,48 @@ 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+\tstatic struct lock_file lock;\n+\tstruct child_process child;\n+\tint 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 = hold_lock_file_for_update(&lock, git_path(\"EDIT_PATCH\"), 1);\n+\n+\tif (run_command(&child))\n+\t\treturn 1;\n+\tfree(child.argv);\n+\n+\tlaunch_editor(lock.filename, NULL, NULL);\n+\n+\tif (stat(lock.filename, &st))\n+\t\treturn 1;\n+\tif (!st.st_size)\n+\t\tdie (\"Empty patch. Aborted.\");\n+\n+\texecl_git_cmd(\"apply\", \"--ignore-hunk-headers\", \"--cached\",\n+\t\t\tlock.filename, NULL);\n+\n+\treturn 1;\n+}\n+\n static struct lock_file lock_file;\n \n static const char ignore_error[] =\n@@ -200,6 +242,7 @@ static struct option builtin_add_options[] = {\n \tOPT_GROUP(\"\"),\n \tOPT_BOOLEAN('i', \"interactive\", &add_interactive, \"interactive picking\"),\n \tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactive patching\"),\n+\tOPT_BOOLEAN('e', \"edit\", &edit_interactive, \"super-interactive patching\"),\n \tOPT_BOOLEAN('f', NULL, &ignored_too, \"allow adding otherwise ignored files\"),\n \tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update tracked files\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\n@@ -226,6 +269,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..be2f4da\n--- /dev/null\n+++ b/t/t3702-add-edit.sh\n@@ -0,0 +1,86 @@\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 \"$1\" orig-patch &&\n+mv 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+test_done\n-- \n1.5.6.rc1.181.gb439d\n"},{"id":"78789","messageId":"DDEBE262-2D0A-4F2E-8928-C268A845F645@ai.rug.nl","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051720300.21190@racer","subject":"Re: [PATCH v2 2/2] git-add: introduce --edit (to edit the diff vs. the index)","fromName":"Pieter de Bie","fromEmail":"pdebie@ai.rug.nl","sentAt":"2008-06-05T18:12:10Z","receivedAt":"2008-06-05T18:12:10Z","isPatch":true,"sender":{"key":"pdebie@ai.rug.nl","avatar":null},"body":"\nOn 5 jun 2008, at 18:20, Johannes Schindelin wrote:\n\n>\n> With \"git add -e []\", Git will fire up an editor with the \n> current\n> diff relative to the index (i.e. what you would get with \"git diff\n> []\").\n>\n> Now you can edit the patch as much as you like, including adding/ \n> removing\n> lines, editing the text, whatever. Make sure, though, that the first\n> character of the hunk lines is still a space, a plus or a minus.\n\nNice feature! However, the lockfile isn't deleted on my system (OS X),\nperhaps because the atexit() isn't called after an exec(). How about \nthis\npatch?\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 05ae40d..07fdd2e 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -192,6 +192,8 @@ int edit_patch(int argc, const char **argv, const \nchar *prefix)\n struct child_process child;\n int ac;\n struct stat st;\n+ const char * apply_args[] = { \"apply\", \"--fixup-line-counts\",\n+ \"--cached\", lock.filename, NULL };\n\n memset(&child, 0, sizeof(child));\n child.argv = xcalloc(sizeof(const char *), (argc + 5));\n@@ -224,10 +226,11 @@ int edit_patch(int argc, const char **argv, \nconst char *prefix)\n return 0;\n }\n\n- execl_git_cmd(\"apply\", \"--fixup-line-counts\", \"--cached\",\n- lock.filename, NULL);\n+ child.argv = apply_args;\n+ if (run_command(&child))\n+ return 1;\n\n- return 1;\n+ return 0;\n }\n\n static struct lock_file lock_file;\n"},{"id":"78794","messageId":"7v4p873ewm.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051441560.21190@racer","subject":"Re: [PATCH 1/2] Allow git-apply to fix up the line counts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-05T18:39:05Z","receivedAt":"2008-06-05T18:39:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> As I said, the data is in the _hunks_, but I maybe should have added _not \n> in the hunk headers_.\n\nSo you _are_ ignoring the line counts recorded in the hunk headers. It is\nnot even 'adjust' but 'count lines to guess'.\n\nIf the incoming patch text does not have confusing contents at the end,\nthe guessing is reasonably safe. You need to watch out for blank lines,\nwhich means the same as /^ $/, and mail signature separators /^-- $/.\nThey can confuse you into guessing wrongly and include more preimage lines\nthan there actually are.\n\nSo it would be more like\n\n--ignore-line-counts::\n\tIgnore number of lines recorded in the hunk headers; instead count\n lines that look like hunk contents to determine how big each hunk\n\tis.\n\nI haven't started to nitpick the actual code yet but I know the original\nis a tricky pice of code, so we may find something interesting ;-)\n"},{"id":"78824","messageId":"7vabhz1t2f.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806051720070.21190@racer","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-05T21:16:08Z","receivedAt":"2008-06-05T21:16:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\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>\n> So teach the tool to do it for us.\n\nTwo comments and a half.\n\n * Latest POSIX draft talks about unified context and allows an empty line\n to represent an empty common context line. GNU diff already emits such\n a diff. fixup_counts() should take this into account.\n\n * I'd sleep better at night if 'Probably \"diff ...\"' part were written in\n a bit more robust way.\n\n * (minor) There is an established term for this operation: recountdiff,\n so --recount might be a better name. fixup_counts() also is better\n called recount_diff() if we go this route.\n\nIf you are too narrowly focused to only support \"git add -e\", the first\nissue does not matter, because we always emit \"SP LF\" for such a common\ncontext. The reason why I care about the first two points is because we\nmay want to teach git-am about this new option as well in 1.6.0.\n\nAnd the robustness issue I worry about the second point also applies to a\nline that is \"^-- $\", especially if we were to make this available to\ngit-am. Perhaps when the line begins with a '-', the logic could be extra\ncareful to detect the case where the line looks like the e-mail signature\nseparator and check one line beyond it to see if it does not look anything\nlike part of a diff (in which case you stop, without considering the line\nyou are currently looking at, \"^-- $\", a deletion of \"^- $\", as part of\nthe preimage context).\n\nAs to code structure, we might want to make the later parameters to\napply_patch() an integer, of OR'ed flag values, or even a pointer to a\nstructure that holds options.\n\nOther than that, the patch looks reasonably isolated and clean.\n"},{"id":"78830","messageId":"alpine.DEB.1.00.0806052304300.21190@racer","threadId":"13809","inReplyTo":"7vabhz1t2f.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T22:39:48Z","receivedAt":"2008-06-05T22:39:48Z","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, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Sometimes, the easiest way to fix up a patch is to edit it directly, \n> > even adding or deleting lines. Now, many people are not as divine as \n> > certain benevolent dictators as to update the hunk headers correctly \n> > at the first try.\n> >\n> > So teach the tool to do it for us.\n> \n> Two comments and a half.\n> \n> * Latest POSIX draft talks about unified context and allows an empty line\n> to represent an empty common context line. GNU diff already emits such\n> a diff. fixup_counts() should take this into account.\n\nAs you pointed out, I wanted to support only add -e. But that should not \nbe an issue at all. I think a \"case ' ': case '\\n':\" should be enough, \nright?\n\n> * I'd sleep better at night if 'Probably \"diff ...\"' part were written \n> in a bit more robust way.\n\nHow about stopping on \"@@\" and end of file only, and complaining \notherwise?\n\n> * (minor) There is an established term for this operation: recountdiff, \n> so --recount might be a better name. fixup_counts() also is better \n> called recount_diff() if we go this route.\n\nFine!\n\n> If you are too narrowly focused to only support \"git add -e\", the first \n> issue does not matter, because we always emit \"SP LF\" for such a common \n> context. The reason why I care about the first two points is because we \n> may want to teach git-am about this new option as well in 1.6.0.\n\nPoint taken.\n\n> And the robustness issue I worry about the second point also applies to \n> a line that is \"^-- $\", especially if we were to make this available to \n> git-am. Perhaps when the line begins with a '-', the logic could be \n> extra careful to detect the case where the line looks like the e-mail \n> signature separator and check one line beyond it to see if it does not \n> look anything like part of a diff (in which case you stop, without \n> considering the line you are currently looking at, \"^-- $\", a deletion \n> of \"^- $\", as part of the preimage context).\n\nIs this really an issue? fixup_counts() is only called after a hunk \nheader was read, and that should be well after any \"^-- $\".\n\n> As to code structure, we might want to make the later parameters to \n> apply_patch() an integer, of OR'ed flag values, or even a pointer to a \n> structure that holds options.\n\nRight.\n\nWill fix up and resubmit.\n\nCiao,\nDscho\n"},{"id":"78835","messageId":"alpine.DEB.1.00.0806060005581.21190@racer","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806052304300.21190@racer","subject":"[PATCH v3 0/2] git add --edit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T23:06:16Z","receivedAt":"2008-06-05T23:06:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nChanges relative to v2:\n\n- it works now not by chance, but by design,\n\n- empty lines are interpreted as if they contained a single space,\n\n- it works when adding lines to the beginning or end of a file, and\n\n- the apply option has been renamed to --recount, as per Junio's request.\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\n Documentation/git-add.txt | 13 ++++-\n Documentation/git-apply.txt | 7 ++-\n builtin-add.c | 55 ++++++++++++++++++-\n builtin-apply.c | 64 ++++++++++++++++++++--\n t/t3702-add-edit.sh | 126 +++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 257 insertions(+), 8 deletions(-)\n create mode 100755 t/t3702-add-edit.sh\n"},{"id":"78837","messageId":"alpine.DEB.1.00.0806060006370.21190@racer","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806060005581.21190@racer","subject":"[PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T23:06:50Z","receivedAt":"2008-06-05T23:06:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\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 | 64 ++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 66 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex 2dec2ec..2fa660e 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@@ -169,6 +169,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, --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..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.rc1.181.gb439d\n"},{"id":"78836","messageId":"alpine.DEB.1.00.0806060007000.21190@racer","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806060005581.21190@racer","subject":"[PATCH v3 2/2] git-add: introduce --edit (to edit the diff vs. the index)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T23:07:12Z","receivedAt":"2008-06-05T23:07:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\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 \n---\n Documentation/git-add.txt | 13 ++++-\n builtin-add.c | 55 +++++++++++++++++++-\n t/t3702-add-edit.sh | 126 +++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 191 insertions(+), 3 deletions(-)\n create mode 100755 t/t3702-add-edit.sh\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 1afd0c6..8620ae2 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -8,8 +8,8 @@ git-add - Add file contents to the index\n SYNOPSIS\n --------\n [verse]\n-'git-add' [-n] [-v] [-f] [--interactive | -i] [--patch | -p] [-u] [--refresh]\n-\t [--ignore-errors] [--] ...\n+'git-add' [-n] [-v] [-f] [--interactive | -i] [--patch | -p] [--edit | -e]\n+\t [-u] [--refresh] [--ignore-errors] [--] ...\n \n DESCRIPTION\n -----------\n@@ -70,6 +70,15 @@ OPTIONS\n \tbypassed and the 'patch' subcommand is invoked using each of\n \tthe specified filepatterns before exiting.\n \n+-e, \\--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 \tUpdate only files that git already knows about, staging modified\n \tcontent for commit and marking deleted files for removal. This\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 1da22ee..fe31453 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@@ -200,6 +250,7 @@ static struct option builtin_add_options[] = {\n \tOPT_GROUP(\"\"),\n \tOPT_BOOLEAN('i', \"interactive\", &add_interactive, \"interactive picking\"),\n \tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactive patching\"),\n+\tOPT_BOOLEAN('e', \"edit\", &edit_interactive, \"super-interactive patching\"),\n \tOPT_BOOLEAN('f', NULL, &ignored_too, \"allow adding otherwise ignored files\"),\n \tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update tracked files\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\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.rc1.181.gb439d\n"},{"id":"78840","messageId":"7v4p87zcv6.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806052304300.21190@racer","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-05T23:22:05Z","receivedAt":"2008-06-05T23:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> And the robustness issue I worry about the second point also applies to \n>> a line that is \"^-- $\", especially if we were to make this available to \n>> git-am. Perhaps when the line begins with a '-', the logic could be \n>> extra careful to detect the case where the line looks like the e-mail \n>> signature separator and check one line beyond it to see if it does not \n>> look anything like part of a diff (in which case you stop, without \n>> considering the line you are currently looking at, \"^-- $\", a deletion \n>> of \"^- $\", as part of the preimage context).\n>\n> Is this really an issue? fixup_counts() is only called after a hunk \n> header was read, and that should be well after any \"^-- $\".\n\nAre you talking about \"^-- $\" or \"^---$\"? Yes we are way past the\nthree-dash separator at this point, but e-mail signature separator happens\nat the very end after the patch.\n\nYou read a hunk header line \"@@ -l,m +n,o @@\", and start counting the diff\ntext because you do not trust m and o. When you read the last hunk in a\npatch e-mail, you may hit a e-mail signature separator, like what is given\nby format-patch output at the end. Mistaking that as an extra preimage\ncontext to remove \"^- $\" is what I was worried about.\n\n-- \nI worry, therefore I am...\n"},{"id":"78842","messageId":"alpine.DEB.1.00.0806060030160.21190@racer","threadId":"13809","inReplyTo":"7v4p87zcv6.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T23:36:36Z","receivedAt":"2008-06-05T23:36:36Z","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, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> And the robustness issue I worry about the second point also applies to \n> >> a line that is \"^-- $\", especially if we were to make this available to \n> >> git-am. Perhaps when the line begins with a '-', the logic could be \n> >> extra careful to detect the case where the line looks like the e-mail \n> >> signature separator and check one line beyond it to see if it does not \n> >> look anything like part of a diff (in which case you stop, without \n> >> considering the line you are currently looking at, \"^-- $\", a deletion \n> >> of \"^- $\", as part of the preimage context).\n> >\n> > Is this really an issue? fixup_counts() is only called after a hunk \n> > header was read, and that should be well after any \"^-- $\".\n> \n> Are you talking about \"^-- $\" or \"^---$\"? Yes we are way past the \n> three-dash separator at this point, but e-mail signature separator \n> happens at the very end after the patch.\n\nOh yes, I was thinking about the \"^---$\".\n\n> You read a hunk header line \"@@ -l,m +n,o @@\", and start counting the \n> diff text because you do not trust m and o. When you read the last hunk \n> in a patch e-mail, you may hit a e-mail signature separator, like what \n> is given by format-patch output at the end. Mistaking that as an extra \n> preimage context to remove \"^- $\" is what I was worried about.\n> \n> -- \n> I worry, therefore I am...\n\nNice signature...\n\nI could check for garbage after a line that consists of exactly \"^-- $\" \nwith something like this (on top of 1/2):\n\n@@ -0,0 +0,0 @@\n \tdefault:\n-\t\treturn -1;\n+\t\treturn len != 4 && memcmp(line - len, \"-- \\n\", len);\n \t}\n\nHmm?\n\nHowever, this will not work if anybody has a signature starting with \n\"@@ \", \"+\", \" \", \"-\" or \"diff \"...\n\nCiao,\nDscho\n\n-- \nWon't dorry, he bappy\n"},{"id":"78860","messageId":"7vve0nw4b7.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806060006370.21190@racer","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-06T04:55:08Z","receivedAt":"2008-06-06T04:55:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> +static int recount_diff(char *line, int size, struct fragment *fragment)\n> +{\n> +\tint line_nr = 0;\n\nAt this point, line points at the beginning of the line that immediately\nfollows \"@@ -oldpos,oldlines +newpos,newlines @@ ...\\n\", right?\n\n> +\tif (size < 1)\n> +\t\treturn -1;\n> +\n> +\tfragment->oldpos = 2;\n\nWhy do you discard oldpos information, and use magic number \"2\"?\n\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\nAnd you look at the line of the patch, measure how long it is, and you\nalready advance line to point at the next line without ever looking at the\ncontents of the line (you could look at line[-len], but that is crazy).\n\n> +\t\tif (size < 1)\n> +\t\t\treturn 0;\n\nWhy? It may be the last line in the hunk but you haven't done anything to\nthe current line yet.\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\nAgain, why muck with oldpos? Also leading and trailing? After you\nrecount the newlines and oldlines you ignored from the hunk header, the\ncaller will behave as if the original patch had the right numbers on the\nhunk header to compute leading and trailing, doesn't it?\n\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\nI do not understand these return values. Your caller, parse_fragment()\nwith your patch, gives you chance to recount the old and new line count,\nand you are responsible for only recounting them. The change you made to\nparse_fragment() returns error, aborting the whole git-apply, when\nrecount_diff() returns non-zero, but having extra lines after the patch\ntext is perfectly fine and there is no reason to force aborting from\nhere. IOW, this function should not even have power to do so. The one\nand only thing this function should do well is to reliably count the\nnumber of patch lines.\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 this \"return -1\" is uncalled for.\n"},{"id":"78861","messageId":"7vr6bbw4a3.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806060005581.21190@racer","subject":"Re: [PATCH v3 0/2] git add --edit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-06T04:55:48Z","receivedAt":"2008-06-06T04:55:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Changes relative to v2:\n>\n> - it works now not by chance, but by design,\n\nFunny. The old one worked by chance?\n"},{"id":"78864","messageId":"5d46db230806052218r67e79a46rd0150cd9fe2af970@mail.gmail.com","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806060006370.21190@racer","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Govind Salinas","fromEmail":"govind@sophiasuchtig.com","sentAt":"2008-06-06T05:18:56Z","receivedAt":"2008-06-06T05:18:56Z","isPatch":true,"sender":{"key":"govind@sophiasuchtig.com","avatar":null},"body":"On Thu, Jun 5, 2008 at 6:06 PM, Johannes Schindelin\n wrote:\n>\n> +\n> + switch (*line) {\n> + case ' ': case '\\n':\n> + fragment->newlines++;\n> + /* fall through */\n> + case '-':\n> + fragment->oldlines++;\n> + break;\n> + case '+':\n> + fragment->newlines++;\n> + if (line_nr == 0) {\n> + fragment->leading = 1;\n> + fragment->oldpos = 1;\n> + }\n> + fragment->trailing = 1;\n> + break;\n> + case '@':\n> + return size < 3 || prefixcmp(line, \"@@ \");\n> + case 'd':\n> + return size < 5 || prefixcmp(line, \"diff \");\n> + default:\n> + return -1;\n> + }\n> + line_nr++;\n> + }\n> +}\n\nPerhaps this is accounted for and I did not see, but I believe that\na backslash is used for the \"no newline at end of file\" line. Does that\nneed to be allowed here?\n\nThanks,\nGovind.\n"},{"id":"78874","messageId":"4848E105.7050405@gnu.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806060030160.21190@racer","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-06-06T07:02:29Z","receivedAt":"2008-06-06T07:02:29Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"\n> @@ -0,0 +0,0 @@\n> \tdefault:\n> -\t\treturn -1;\n> +\t\treturn len != 4 && memcmp(line - len, \"-- \\n\", len);\n> \t}\n\nYou're never returning -1 here, right?\n\n> However, this will not work if anybody has a signature starting with \n> \"@@ \", \"+\", \" \", \"-\" or \"diff \"...\n\nI think that the main worry is the patches made with git-format-patch, \nand those are not problematic.\n\nPaolo\n"},{"id":"78886","messageId":"48490B3A.4020900@free.fr","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806060007000.21190@racer","subject":"Re: [PATCH v3 2/2] git-add: introduce --edit (to edit the diff vs. the index)","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-06T10:02:34Z","receivedAt":"2008-06-06T10:02:34Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Johannes Schindelin a écrit :\n> \n> +int edit_patch(int argc, const char **argv, const char *prefix)\n> +{\n\n[...]\n\n> +\tif (!result)\n> +\t\tresult = run_command(&child);\n> +\tfree(child.argv);\n> +\n> +\tlaunch_editor(file, NULL, NULL);\n\nHere, it does not launch the editor I defined with core.editor because you\ncall edit_patch() before calling git_config() in cmd_add().\n\nAlso, wouldn't be better to have the edit_patch stuff in add--interactive\ninstead ? It seems to work the same way than the --patch option.\n\nJust my thoughts.\n\nOlivier.\n"},{"id":"78887","messageId":"87iqwmzwcn.fsf@osv.gnss.ru","threadId":"13809","inReplyTo":"7v4p87zcv6.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Sergei Organov","fromEmail":"osv@javad.com","sentAt":"2008-06-06T10:33:28Z","receivedAt":"2008-06-06T10:33:28Z","isPatch":true,"sender":{"key":"osv@javad.com","avatar":null},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>>> And the robustness issue I worry about the second point also applies to \n>>> a line that is \"^-- $\", especially if we were to make this available to \n>>> git-am. Perhaps when the line begins with a '-', the logic could be \n>>> extra careful to detect the case where the line looks like the e-mail \n>>> signature separator and check one line beyond it to see if it does not \n>>> look anything like part of a diff (in which case you stop, without \n>>> considering the line you are currently looking at, \"^-- $\", a deletion \n>>> of \"^- $\", as part of the preimage context).\n>>\n>> Is this really an issue? fixup_counts() is only called after a hunk \n>> header was read, and that should be well after any \"^-- $\".\n>\n> Are you talking about \"^-- $\" or \"^---$\"? Yes we are way past the\n> three-dash separator at this point, but e-mail signature separator happens\n> at the very end after the patch.\n>\n> You read a hunk header line \"@@ -l,m +n,o @@\", and start counting the diff\n> text because you do not trust m and o. When you read the last hunk in a\n> patch e-mail, you may hit a e-mail signature separator, like what is given\n> by format-patch output at the end. Mistaking that as an extra preimage\n> context to remove \"^- $\" is what I was worried about.\n\nDon't you think it's time to fix git-format-patch to put some reliable\n\"end-of-patch\" marker line before the signature? This change (along with\nrefusal to generate brain-damaged empty lines inside hunks) will make\ngit diffs easily parseable without information from hunk headers.\n\n-- Sergei.\n"},{"id":"78909","messageId":"alpine.DEB.1.00.0806061441120.1783@racer","threadId":"13809","inReplyTo":"7vve0nw4b7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T13:58:17Z","receivedAt":"2008-06-06T13:58:17Z","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, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > +static int recount_diff(char *line, int size, struct fragment *fragment)\n> > +{\n> > +\tint line_nr = 0;\n> \n> At this point, line points at the beginning of the line that immediately\n> follows \"@@ -oldpos,oldlines +newpos,newlines @@ ...\\n\", right?\n\nNo. At this point, it points directly after the last \"@@\", but still on \nthe same line.\n\n> > +\tif (size < 1)\n> > +\t\treturn -1;\n> > +\n> > +\tfragment->oldpos = 2;\n> \n> Why do you discard oldpos information, and use magic number \"2\"?\n\nBecause the old information should be ignored. If the first line is a \"+\" \nline, the line number needs to be set to 1, otherwise the patch will not \napply.\n\nMaybe the easiest would be to set it to 1 regardless of the hunk.\n\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> And you look at the line of the patch, measure how long it is, and you\n> already advance line to point at the next line without ever looking at the\n> contents of the line (you could look at line[-len], but that is crazy).\n\nNo, as I said, the \"line\" parameter points just after \"@@\" of the hunk \nheader.\n\nBTW this is what I referred to when I said that it now works by design; \nformerly, it relied on a space being present after the \"@@\", which it \nwould mistakenly count as a common line, which was the reason I set the \ncounts to -1 initially.\n\n> > +\t\tif (size < 1)\n> > +\t\t\treturn 0;\n> \n> Why? It may be the last line in the hunk but you haven't done anything to\n> the current line yet.\n\nNo, see above.\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> \n> Again, why muck with oldpos? Also leading and trailing? After you\n> recount the newlines and oldlines you ignored from the hunk header, the\n> caller will behave as if the original patch had the right numbers on the\n> hunk header to compute leading and trailing, doesn't it?\n\nNo. Because I can never know if the _positions_ in the hunk header are \nright.\n\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> \n> I do not understand these return values. Your caller, parse_fragment() \n> with your patch, gives you chance to recount the old and new line count, \n> and you are responsible for only recounting them. The change you made \n> to parse_fragment() returns error, aborting the whole git-apply, when \n> recount_diff() returns non-zero, but having extra lines after the patch \n> text is perfectly fine and there is no reason to force aborting from \n> here.\n\nThen I understood you not correctly at all when you complained about the \n\"Probably a diff\" part.\n\nSo what do you want? Should it be anal, or lax? You can't have both.\n\n> IOW, this function should not even have power to do so. The one\n> and only thing this function should do well is to reliably count the\n> number of patch lines.\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 this \"return -1\" is uncalled for.\n\nAgain. Lax or not lax?\n\nCiao,\nDscho\n \n"},{"id":"78910","messageId":"alpine.DEB.1.00.0806061458430.1783@racer","threadId":"13809","inReplyTo":"7vr6bbw4a3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3 0/2] git add --edit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T13:59:48Z","receivedAt":"2008-06-06T13:59:48Z","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, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Changes relative to v2:\n> >\n> > - it works now not by chance, but by design,\n> \n> Funny. The old one worked by chance?\n\nYes, it did. In my tests, I did not realize that the \"line\" parameter was \nset just after the second \"@@\" in the hunk header. Therefore, the code \ncounted a space at the beginning of the hunk header comment as a common \nline.\n\nIn my later tests, I finally realized my error, and changed the code.\n\nCiao,\nDscho\n"},{"id":"78911","messageId":"alpine.DEB.1.00.0806061500290.1783@racer","threadId":"13809","inReplyTo":"5d46db230806052218r67e79a46rd0150cd9fe2af970@mail.gmail.com","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T14:00:59Z","receivedAt":"2008-06-06T14:00:59Z","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, Govind Salinas wrote:\n\n> On Thu, Jun 5, 2008 at 6:06 PM, Johannes Schindelin\n> wrote:\n> >\n> > +\n> > + switch (*line) {\n> > + case ' ': case '\\n':\n> > + fragment->newlines++;\n> > + /* fall through */\n> > + case '-':\n> > + fragment->oldlines++;\n> > + break;\n> > + case '+':\n> > + fragment->newlines++;\n> > + if (line_nr == 0) {\n> > + fragment->leading = 1;\n> > + fragment->oldpos = 1;\n> > + }\n> > + fragment->trailing = 1;\n> > + break;\n> > + case '@':\n> > + return size < 3 || prefixcmp(line, \"@@ \");\n> > + case 'd':\n> > + return size < 5 || prefixcmp(line, \"diff \");\n> > + default:\n> > + return -1;\n> > + }\n> > + line_nr++;\n> > + }\n> > +}\n> \n> Perhaps this is accounted for and I did not see, but I believe that\n> a backslash is used for the \"no newline at end of file\" line. Does that\n> need to be allowed here?\n\nWill change.\n\nCiao,\nDscho\n"},{"id":"78912","messageId":"alpine.DEB.1.00.0806061502030.1783@racer","threadId":"13809","inReplyTo":"4848E105.7050405@gnu.org","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T14:04:01Z","receivedAt":"2008-06-06T14:04:01Z","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, Paolo Bonzini wrote:\n\n> > @@ -0,0 +0,0 @@\n> > \tdefault:\n> > -\t\treturn -1;\n> > +\t\treturn len != 4 && memcmp(line - len, \"-- \\n\", len);\n> > }\n> \n> You're never returning -1 here, right?\n\nYou are a clever guy! I really do not return -1 here. But then, the \nreturn value is only checked for non-zeroness. As is obvious from the \npart you did not quote.\n\n> > However, this will not work if anybody has a signature starting with \n> > \"@@ \", \"+\", \" \", \"-\" or \"diff \"...\n> \n> I think that the main worry is the patches made with git-format-patch, \n> and those are not problematic.\n\nActually, this change was done in v3 on _explicit_ request from Junio who \nwants to be able to use the patch for git-am, where we cannot rely on \nformat-patch.\n\nSo yes, they _are_ problematic.\n\nThanks,\nDscho\n"},{"id":"78914","messageId":"alpine.DEB.1.00.0806061514510.1783@racer","threadId":"13809","inReplyTo":"48490B3A.4020900@free.fr","subject":"Re: [PATCH v3 2/2] git-add: introduce --edit (to edit the diff vs. the index)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T14:21:01Z","receivedAt":"2008-06-06T14:21:01Z","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, Olivier Marin wrote:\n\n> Johannes Schindelin a écrit :\n> > \n> > +int edit_patch(int argc, const char **argv, const char *prefix)\n> > +{\n> \n> [...]\n> \n> > +\tif (!result)\n> > +\t\tresult = run_command(&child);\n> > +\tfree(child.argv);\n> > +\n> > +\tlaunch_editor(file, NULL, NULL);\n> \n> Here, it does not launch the editor I defined with core.editor because \n> you call edit_patch() before calling git_config() in cmd_add().\n\nWill fix.\n\n> Also, wouldn't be better to have the edit_patch stuff in \n> add--interactive instead ? It seems to work the same way than the \n> --patch option.\n\nActually, no. It does something completely different. For example, it \navoids calling a perl script. At least as long as your editor is not a \nPerl script.\n\nCiao,\nDscho\n"},{"id":"78916","messageId":"alpine.DEB.1.00.0806061525410.1783@racer","threadId":"13809","inReplyTo":"87iqwmzwcn.fsf@osv.gnss.ru","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T14:27:11Z","receivedAt":"2008-06-06T14:27:11Z","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, Sergei Organov wrote:\n\n> Junio C Hamano writes:\n> \n> > You read a hunk header line \"@@ -l,m +n,o @@\", and start counting the \n> > diff text because you do not trust m and o. When you read the last \n> > hunk in a patch e-mail, you may hit a e-mail signature separator, like \n> > what is given by format-patch output at the end. Mistaking that as an \n> > extra preimage context to remove \"^- $\" is what I was worried about.\n> \n> Don't you think it's time to fix git-format-patch to put some reliable \n> \"end-of-patch\" marker line before the signature? This change (along with \n> refusal to generate brain-damaged empty lines inside hunks) will make \n> git diffs easily parseable without information from hunk headers.\n\nActually, what do you think the numbers in the hunk headers are good for? \nThey _are_ reliable end-of-hunk markers. And if the next line does not \nexist, or does not start, it is end-of-diff. Reliable. Simple.\n\nOnly when you need to get sloppy, things get worse. I need to get sloppy.\n\nBut that's hardly the fault of format-patch.\n\nCiao,\nDscho\n"},{"id":"78918","messageId":"7vej7awq6z.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"87iqwmzwcn.fsf@osv.gnss.ru","subject":"Re: [PATCH v2 1/2] Allow git-apply to ignore the hunk headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-06T15:14:44Z","receivedAt":"2008-06-06T15:14:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergei Organov writes:\n\n> Don't you think it's time to fix git-format-patch to put some reliable\n> \"end-of-patch\" marker line before the signature?\n\nNot at all. git-apply is designed to (and has to) grok any reasonable\ne-mailed patch, not just format-patch output. And we should consider\nhaving e-mail signature separator at the end as \"reasonable\".\n\nIt only becomes an issue when you start deviating from the rule of\nreliable diff parsing, such as ignoring the old/new line count, which is\nthe topic of Dscho's patch.\n"},{"id":"78920","messageId":"7v63smwpaj.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806061441120.1783@racer","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-06T15:34:12Z","receivedAt":"2008-06-06T15:34:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Then I understood you not correctly at all when you complained about the \n> \"Probably a diff\" part.\n\nThe wish was this.\n\nWhen recounting this hunk:\n\n\t@@ -l,m +n,o @@$\n preimage$\n -deleted$\n -deleted$\n Some other text\n\nI want you to say \"three and one\", and not error out. Reading \"Some other\ntext\" and deciding that the stream of fragments for the current patch has\nended is the job for parse_single_patch().\n\nIf on the other hand the input were:\n\n\t@@ -l,m +n,o @@$\n preimage$\n -deleted$\n -deleted$\n\t-- $\n This space intentionally left blank.\n\tperhaps a few more lines of sig\n\t<>\n\nI do not want you to say \"four and one\" silently. It is more likely that\nthe answer is \"three and one\", and the last line that begins with a '-' is\nnot part of the diff, but for the sake of robustness, I do not want to\nhear \"three and one\" without warning either. Erroring out, because you\ncannot recount reliably, would be prudent. We do not have a clever tool\nthat sometimes does a wrong thing silently.\n\nI was fooled by the \"where does the line begin when we come to this\nfunction\", so I need to re-read the code and see what you are doing.\n"},{"id":"78927","messageId":"7vr6bav8ww.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806061441120.1783@racer","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-06T16:13:19Z","receivedAt":"2008-06-06T16:13:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> At this point, line points at the beginning of the line that immediately\n>> follows \"@@ -oldpos,oldlines +newpos,newlines @@ ...\\n\", right?\n>\n> No. At this point, it points directly after the last \"@@\", but still on \n> the same line.\n\nAh, I personally think that is a crazy calling convention but the\nfunction's loop consistently uses the \"line at the beginning of iteration\npoints somewhere in the previous line to be skipped\", so it would \"work\".\nI was fooled by that.\n\n>> > +\tif (size < 1)\n>> > +\t\treturn -1;\n>> > +\n>> > +\tfragment->oldpos = 2;\n>> \n>> Why do you discard oldpos information, and use magic number \"2\"?\n>\n> Because the old information should be ignored. If the first line is a \"+\" \n> line, the line number needs to be set to 1, otherwise the patch will not \n> apply.\n\nI do not think line number information should be discarded. If you have\ntwo blocks of lines that look alike, the line number information does get\nused to see which place the hunk should apply.\n\nDeleting the common context lines from the beginning, or adding a new\n\"+added line\" at the very beginning of a hunk, is a user error for\nsomebody who edits the diff. Because you are not calling apply with\nunidiff-zero, the sanity check applies to such a hunk. Working around the\nsanity check by discarding the line number information to make the patch\napplication even more error prone is an unacceptable hackery.\n\n> Maybe the easiest would be to set it to 1 regardless of the hunk.\n\nAnd that is even worse, and I thought you knew a lot better than that.\nSigh...\n\n> Then I understood you not correctly at all when you complained about the \n> \"Probably a diff\" part.\n>\n> So what do you want? Should it be anal, or lax? You can't have both.\n\nI explained what I wanted to _happen_ in a separate message. Now to _how_\nyou would make it happen...\n\nThe way you use the return value from here is to cause parse_fragment() to\nsay \"The patch is corrupt\". You do _not_ detect that here. You are only\ncounting number of preimage and postimage lines in this function. If the\nnext line does not look like a part of the current hunk, you stop counting\n(iow, the only side effect you cause is to update oldlines and newlines in\nfragment structure) without including that non-patch line, and return.\nYou let the caller to decide what that next line you excluded from the\ncurrent hunk is, because the caller _already_ has logic to decide what is\npart of the patch text (it knows not just how hunk meat looks like but\nalso how hunk headers and \"diff \" to start the next patch looks like).\nYou do not want that information or logic here.\n\nSo the answer to \"anal or lax\" is \"Neither. It's none of your business\".\n\n>> > +\tif (offset > 0 && patch->recount &&\n>> > +\t\t\trecount_diff(line + offset, size - offset, fragment))\n>> > +\t\treturn -1;\n>> \n>> ... and this \"return -1\" is uncalled for.\n>\n> Again. Lax or not lax?\n\nNeither. This calling site should not even decide. The only thing\nrecount will tell its caller parse_fragment() is \"I've recounted the\nlines, so by iterating that many lines you will reach the end of the\ncurrent hunk as I determined. Decide what the line beyond that is _your_\nbusiness, not mine\".\n"},{"id":"78928","messageId":"alpine.DEB.1.00.0806061735550.1783@racer","threadId":"13809","inReplyTo":"7vr6bav8ww.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T16:37:50Z","receivedAt":"2008-06-06T16:37:50Z","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, Junio C Hamano wrote:\n\n> Deleting the common context lines from the beginning, or adding a new\n> \"+added line\" at the very beginning of a hunk, is a user error for\n> somebody who edits the diff.\n\nUnfortunately, that was exactly what I needed to do. Not in a theoretical \nsense, but a very practical one.\n\nOnly in hindsight do I realize that I could have increased the context, \nbut that was not easily possible with the way I drive diff-files via \nrun_command().\n\nOh well, another iteration.\n\nCiao,\nDscho\n"},{"id":"78931","messageId":"7vej7av7df.fsf@gitster.siamese.dyndns.org","threadId":"13809","inReplyTo":"alpine.DEB.1.00.0806061735550.1783@racer","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-06T16:46:36Z","receivedAt":"2008-06-06T16:46:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Only in hindsight do I realize that I could have increased the context, \n\nYeah, I was thinking about suggesting that, using -U7 or something before\ndumping the output to the editor.\n"},{"id":"78935","messageId":"alpine.DEB.1.00.0806061835130.1783@racer","threadId":"13809","inReplyTo":"7vej7av7df.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3 1/2] Allow git-apply to ignore the hunk headers (AKA recountdiff)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-06T17:35:21Z","receivedAt":"2008-06-06T17:35:21Z","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, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Only in hindsight do I realize that I could have increased the \n> > context,\n> \n> Yeah, I was thinking about suggesting that, using -U7 or something \n> before dumping the output to the editor.\n\nGood idea.\n\nCiao,\nDscho\n"}]}