{"thread":{"id":"15974","subject":"[PATCH, RFC] diff: add option to show context between close chunks","startedAt":"2008-10-19T17:59:23Z","lastAt":"2008-10-21T20:48:52Z","messageCount":11,"participants":["René Scharfe","Johannes Sixt","Junio C Hamano","Jeff King","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"93472","messageId":"48FB757B.9030105@lsrfire.ath.cx","threadId":"15974","inReplyTo":null,"subject":"[PATCH, RFC] diff: add option to show context between close chunks","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2008-10-19T17:59:23Z","receivedAt":"2008-10-19T17:59:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This patch adds a new diff option, --inter-chunk-context.  It can be\nused to show the context in gaps between chunks, thereby creating a\nbig chunk out of two close chunks, in order to have an unbroken\ncontext, making reviews easier.\n\nWith --inter-chunk-context=1, patches have the same number of lines\nas without the option, as only the chunk header is replaced by the\ncontext line it was shadowing.\n\nYou can use commit b0b44bc7b26c8c4b4221a377ce6ba174b843cb8d in the\ngit repo to try out this option; there's a chunk in transport.c\nwhich is just one line away from the next.  (I found this option\nhelpful in reviewing my own patch before sending. :)\n\nI think it makes sense to make 1, or even 3, the default for this\noption for all commands that create patches intended for human\nconsumption.  The patch keeps the default at 0, though.\n\nThere are downsides, of course: values higher than 1 potentially make\nthe resulting patch longer.  More context means a higher probability\nof (perhaps unnecessary) merge conflicts.\n\nComments?\n---\n Documentation/diff-options.txt      |    4 ++\n diff.c                              |    4 ++\n diff.h                              |    1 +\n t/t4030-diff-inter-chunk-context.sh |   75 +++++++++++++++++++++++++++++++++++\n xdiff/xdiff.h                       |    1 +\n xdiff/xemit.c                       |    3 +-\n 6 files changed, 87 insertions(+), 1 deletions(-)\n create mode 100755 t/t4030-diff-inter-chunk-context.sh\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex c62b45c..e0c6d8c 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -215,6 +215,10 @@ endif::git-format-patch[]\n -w::\n \tShorthand for \"--ignore-all-space\".\n \n+--inter-chunk-context=<lines>::\n+\tShow the context between diff chunks, up to the specified number\n+\tof lines, thereby fusing chunks that are close to each other.\n+\n --exit-code::\n \tMake the program exit with codes similar to diff(1).\n \tThat is, it exits with 1 if there were differences and\ndiff --git a/diff.c b/diff.c\nindex 1c6be89..4a3b486 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1594,6 +1594,7 @@ static void builtin_diff(const char *name_a,\n \t\tecbdata.file = o->file;\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\n+\t\txecfg.interchunkctxlen = o->interchunkcontext;\n \t\txecfg.flags = XDL_EMIT_FUNCNAMES;\n \t\tif (pe)\n \t\t\txdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);\n@@ -2677,6 +2678,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->b_prefix = arg + 13;\n \telse if (!strcmp(arg, \"--no-prefix\"))\n \t\toptions->a_prefix = options->b_prefix = \"\";\n+\telse if (opt_arg(arg, '\\0', \"inter-chunk-context\",\n+\t\t\t &options->interchunkcontext))\n+\t\t;\n \telse if (!prefixcmp(arg, \"--output=\")) {\n \t\toptions->file = fopen(arg + strlen(\"--output=\"), \"w\");\n \t\toptions->close_file = 1;\ndiff --git a/diff.h b/diff.h\nindex a49d865..6003024 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -77,6 +77,7 @@ struct diff_options {\n \tconst char *a_prefix, *b_prefix;\n \tunsigned flags;\n \tint context;\n+\tint interchunkcontext;\n \tint break_opt;\n \tint detect_rename;\n \tint skip_stat_unmatch;\ndiff --git a/t/t4030-diff-inter-chunk-context.sh b/t/t4030-diff-inter-chunk-context.sh\nnew file mode 100755\nindex 0000000..448f1b9\n--- /dev/null\n+++ b/t/t4030-diff-inter-chunk-context.sh\n@@ -0,0 +1,75 @@\n+#!/bin/sh\n+\n+test_description='diff chunk merging'\n+\n+. ./test-lib.sh\n+\n+f() {\n+\ti=1\n+\techo $1\n+\twhile test $i -le $2\n+\tdo\n+\t\techo $i\n+\t\ti=$(expr $i + 1)\n+\tdone\n+\techo $3\n+}\n+\n+test_expect_success 'setup' '\n+\tf a 1 b >f1 &&\n+\tf a 2 b >f2 &&\n+\tf a 3 b >f3 &&\n+\tgit add f1 f2 f3 &&\n+\tgit commit -q -m. f1 f2 f3 &&\n+\tf x 1 y >f1 &&\n+\tf x 2 y >f2 &&\n+\tf x 3 y >f3\n+'\n+\n+t() {\n+\tcase \"$2\" in\n+\t'')\tcmd=\"diff -U$1\" ;;\n+\t*)\tcmd=\"diff -U$1 --inter-chunk-context=$2\" ;;\n+\tesac\n+\tlabel=\"$cmd, $3 common $4\"\n+\n+\ttest_expect_success \"$label: count chunks ($5)\" \"\n+\t\ttest $(git $cmd f$3 | grep '^@@ ' | wc -l) = $5\n+\t\"\n+\n+\texpected=\"expected.$1.$3.$5\"\n+\ttest -f $expected &&\n+\ttest_expect_success \"$label: check output\" \"\n+\t\tgit $cmd f$3 >actual &&\n+\t\ttest_cmp $expected actual\n+\t\"\n+}\n+\n+cat <<EOF >expected.0.1.1 || exit 1\n+diff --git a/f1 b/f1\n+index f1e80ce..ae397d0 100644\n+--- a/f1\n++++ b/f1\n+@@ -1,3 +1,3 @@\n+-a\n++x\n+ 1\n+-b\n++y\n+EOF\n+\n+# ctx\tintrctx\tcommon\tlines\tchunks\n+t 0\t''\t1\tline\t2\n+t 0\t0\t1\tline\t2\n+t 0\t1\t1\tline\t1\n+t 0\t2\t1\tline\t1\n+t 0\t''\t2\tlines\t2\n+t 0\t0\t2\tlines\t2\n+t 0\t1\t2\tlines\t2\n+t 0\t2\t2\tlines\t1\n+t 1\t''\t3\tlines\t2\n+t 1\t0\t3\tlines\t2\n+t 1\t1\t3\tlines\t1\n+t 1\t2\t3\tlines\t1\n+\n+test_done\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex deebe02..d8f14e6 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -84,6 +84,7 @@ typedef long (*find_func_t)(const char *line, long line_len, char *buffer, long\n \n typedef struct s_xdemitconf {\n \tlong ctxlen;\n+\tlong interchunkctxlen;\n \tunsigned long flags;\n \tfind_func_t find_func;\n \tvoid *find_func_priv;\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex d3d9c84..3bf2581 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -60,9 +60,10 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *\n  */\n static xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) {\n \txdchange_t *xch, *xchp;\n+\tlong max_common = 2 * xecfg->ctxlen + xecfg->interchunkctxlen;\n \n \tfor (xchp = xscr, xch = xscr->next; xch; xchp = xch, xch = xch->next)\n-\t\tif (xch->i1 - (xchp->i1 + xchp->chg1) > 2 * xecfg->ctxlen)\n+\t\tif (xch->i1 - (xchp->i1 + xchp->chg1) > max_common)\n \t\t\tbreak;\n \n \treturn xchp;\n-- \n1.6.0.2.287.g3791f\n"},{"id":"93522","messageId":"48FC9685.8030704@viscovery.net","threadId":"15974","inReplyTo":"48FB757B.9030105@lsrfire.ath.cx","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-10-20T14:32:37Z","receivedAt":"2008-10-20T14:32:37Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"René Scharfe schrieb:\n> This patch adds a new diff option, --inter-chunk-context.  It can be\n> used to show the context in gaps between chunks, thereby creating a\n> big chunk out of two close chunks, in order to have an unbroken\n> context, making reviews easier.\n> \n> With --inter-chunk-context=1, patches have the same number of lines\n> as without the option, as only the chunk header is replaced by the\n> context line it was shadowing.\n> \n> You can use commit b0b44bc7b26c8c4b4221a377ce6ba174b843cb8d in the\n> git repo to try out this option; there's a chunk in transport.c\n> which is just one line away from the next.  (I found this option\n> helpful in reviewing my own patch before sending. :)\n> \n> I think it makes sense to make 1, or even 3, the default for this\n> option for all commands that create patches intended for human\n> consumption.  The patch keeps the default at 0, though.\n> \n> There are downsides, of course: values higher than 1 potentially make\n> the resulting patch longer.  More context means a higher probability\n> of (perhaps unnecessary) merge conflicts.\n> \n> Comments?\n\nWhy can't you just use -U6 instead instead of --inter-chunk-context=3? If\nthis is intended for human consumption anyway, then you can just as well\nincrease the overall number of context lines: You get extra context lines\nin the places where hunks are not fused, but this cannot be a disadvantage\nfor the targeted audience.\n\n-- Hannes\n"},{"id":"93551","messageId":"48FCC8A1.5090109@lsrfire.ath.cx","threadId":"15974","inReplyTo":"48FC9685.8030704@viscovery.net","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2008-10-20T18:06:25Z","receivedAt":"2008-10-20T18:06:25Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Sixt schrieb:\n> Why can't you just use -U6 instead instead of --inter-chunk-context=3? If\n> this is intended for human consumption anyway, then you can just as well\n> increase the overall number of context lines: You get extra context lines\n> in the places where hunks are not fused, but this cannot be a disadvantage\n> for the targeted audience.\n\nThe extra lines _are_ a disadvantage for me, since each chunk gets them\nwhile only close ones need them -- that's wasted space.  The intent is\nto save time by not having to apply and rediff a patch on a mailing list\nin order to see the hidden text; part of the savings would be lost if\nthe reader had to skip over extra lines.\n\nBesides, this option is probably most useful if set by default on\nmachines not controlled by me.  E.g. I wouldn't want to change an option\non a gitweb page, I'd want to get fused chunks by default.  I wouldn't\nwant to write \"best viewed with -U6\" in a patch description, I'd want it\nto just work (e.g. for gitk users).\n\nI have to admit my main motivation was that one line gap, where a chunk\nheader hid an interesting line of context.  Showing it didn't change the\nlength of the patch, so I found this to be a sad wastage.\n\nOptimizing patches for readability makes sense since they are such an\nimportant part of our communication here.  I think --i-c-c=1 results in\nan obvious win without much of a downside.  I was surprised to find that\nalmost 4% of the chunks in git 1.6.0 would be eliminated by that option:\n\n   $ git log -p v1.6.0 | grep -c '^@@ '\n   60544\n   $ git log -p v1.6.0 --inter-chunk-context=1 | grep -c '^@@ '\n   58471\n\nBut perhaps a higher value would be even better?\n\n   $ git log v1.6.0 | wc -l\n   225441\n   $ git log -p v1.6.0 | wc -l\n   1736188\n   $ git log -p --inter-chunk-context=10 v1.6.0 | wc -l\n   1779048\n   $ git log -p -U8 v1.6.0 | wc -l\n   2214448\n\nWell, I don't know if those patches are easier to read (haven't looked\nat them), but I imagine that some related changes are presented with the\nfull context between them (e.g. changes to loop header and footer,\nfunction signature and body).  The numbers only show that it's\naffordable (3% more lines with -i-c-c=10, 30% more lines with the\ncomparable -U8).\n\n(Why 10?  With the default of -U3, a chunk is 6 lines of context plus at\nleast one line of actual change.  Two chunks are at least 14 lines long,\nthus only 10 more fit into 24 lines, i.e. a terminal window..)\n\nRené\n"},{"id":"93564","messageId":"7vabcy3k9l.fsf@gitster.siamese.dyndns.org","threadId":"15974","inReplyTo":"48FB757B.9030105@lsrfire.ath.cx","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-20T23:43:34Z","receivedAt":"2008-10-20T23:43:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> I think it makes sense to make 1, or even 3, the default for this\n> option for all commands that create patches intended for human\n> consumption.  The patch keeps the default at 0, though.\n\nI think defaulting to 1 would make sense, or alternatively, just\nhardcoding that behaviour without any new option.  That would give you\nmore information with the same number of patch lines, iow, upside without\nany downside.\n\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex d3d9c84..3bf2581 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -60,9 +60,9 @@ static int xdl_emit_record(xdfile_t *xdf, \n  */\n static xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) {\n \txdchange_t *xch, *xchp;\n \n \tfor (xchp = xscr, xch = xscr->next; xch; xchp = xch, xch = xch->next)\n-\t\tif (xch->i1 - (xchp->i1 + xchp->chg1) > 2 * xecfg->ctxlen)\n+\t\tif (xch->i1 - (xchp->i1 + xchp->chg1) > 2 * xecfg->ctxlen + 1)\n \t\t\tbreak;\n \n \treturn xchp;\n"},{"id":"93577","messageId":"48FD721D.9030105@viscovery.net","threadId":"15974","inReplyTo":"48FCC8A1.5090109@lsrfire.ath.cx","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-10-21T06:09:33Z","receivedAt":"2008-10-21T06:09:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"René Scharfe schrieb:\n> I have to admit my main motivation was that one line gap, where a chunk\n> header hid an interesting line of context.  Showing it didn't change the\n> length of the patch, so I found this to be a sad wastage.\n\n\"Wastage\" is relative. For a given patch, the one line of context that was\nhidden by the hunk header would be welcome by a human reader, but it is\nnot necessarily useful if the patch is to be applied, in particular, if it\nis applied to a version of the file that has *more* than one line between\nthe hunk contexts. This is the reason that diff does not produce 7 lines\nof context between changes in -U3 mode (\"you asked for 3 lines of context,\nyou get 3 lines of context\").\n\nBTW, nomenclature seems to have settled at the word \"hunk\", not \"chunk\".\n\n-- Hannes\n"},{"id":"93579","messageId":"48FD781C.2000103@viscovery.net","threadId":"15974","inReplyTo":"7vabcy3k9l.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-10-21T06:35:08Z","receivedAt":"2008-10-21T06:35:08Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n> \n>> I think it makes sense to make 1, or even 3, the default for this\n>> option for all commands that create patches intended for human\n>> consumption.  The patch keeps the default at 0, though.\n> \n> I think defaulting to 1 would make sense, or alternatively, just\n> hardcoding that behaviour without any new option.  That would give you\n> more information with the same number of patch lines, iow, upside without\n> any downside.\n\nAre you sure about the \"without any downside\" part? The extra context line\ninhibits that the patch applies cleanly to a version of the file that has\nthat very line modified (including a different number of lines).\n\n-- Hannes\n"},{"id":"93580","messageId":"7vskqqzajy.fsf@gitster.siamese.dyndns.org","threadId":"15974","inReplyTo":"48FD781C.2000103@viscovery.net","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-21T07:12:17Z","receivedAt":"2008-10-21T07:12:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Junio C Hamano schrieb:\n>> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n>> \n>>> I think it makes sense to make 1, or even 3, the default for this\n>>> option for all commands that create patches intended for human\n>>> consumption.  The patch keeps the default at 0, though.\n>> \n>> I think defaulting to 1 would make sense, or alternatively, just\n>> hardcoding that behaviour without any new option.  That would give you\n>> more information with the same number of patch lines, iow, upside without\n>> any downside.\n>\n> Are you sure about the \"without any downside\" part? The extra context line\n> inhibits that the patch applies cleanly to a version of the file that has\n> that very line modified (including a different number of lines).\n\nYeah.  René wanted this for _human consumption_, not mechanical patch\napplication, so \"hardcoding\" literally there in the very low level of the\ndiff callchain is not quite right (it would affect format-patch which is\nprimarily for mechanical application).\n\nI guess you could make the hardcoded value 1 for everybody else and 0 for\nformat-patch.\n"},{"id":"93592","messageId":"20081021112040.GB17363@coredump.intra.peff.net","threadId":"15974","inReplyTo":"7vskqqzajy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-21T11:20:40Z","receivedAt":"2008-10-21T11:20:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 21, 2008 at 12:12:17AM -0700, Junio C Hamano wrote:\n\n> Yeah.  René wanted this for _human consumption_, not mechanical patch\n> application, so \"hardcoding\" literally there in the very low level of the\n> diff callchain is not quite right (it would affect format-patch which is\n> primarily for mechanical application).\n> \n> I guess you could make the hardcoded value 1 for everybody else and 0 for\n> format-patch.\n\nI see your reasoning, but at the same time, a large portion of patches I\nread are from format-patch (and René even said that he was trying to\nsave the user from the \"apply then diff just to look at the patch\"\nannoyance). And I have personally, as a patch submitter, created some\nformat-patch output sent to the git list with -U5 to combine hunks and\nmake it more readable for reviewers.\n\nNot to mention that I sometimes apply or post the output of \"git diff\".\n\nTo me that it implies that either:\n\n - the increased chance of conflict is not a problem in practice, and we\n   should have the option on by default everywhere\n\n - it is a problem, in which case we should ask the user to turn on the\n   feature manually instead of second-guessing how they will use the\n   resulting patch (which they might not even know, since they are\n   making assumptions about how other people might use the patch, and\n   they must decide for their situation between shipping something that\n   is more readable but slightly more conflict prone, or as easy to\n   apply as possible)\n\n-Peff\n"},{"id":"93620","messageId":"alpine.LNX.1.00.0810211403120.19665@iabervon.org","threadId":"15974","inReplyTo":"48FD781C.2000103@viscovery.net","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-10-21T18:16:10Z","receivedAt":"2008-10-21T18:16:10Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 21 Oct 2008, Johannes Sixt wrote:\n\n> Junio C Hamano schrieb:\n> > René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n> > \n> >> I think it makes sense to make 1, or even 3, the default for this\n> >> option for all commands that create patches intended for human\n> >> consumption.  The patch keeps the default at 0, though.\n> > \n> > I think defaulting to 1 would make sense, or alternatively, just\n> > hardcoding that behaviour without any new option.  That would give you\n> > more information with the same number of patch lines, iow, upside without\n> > any downside.\n> \n> Are you sure about the \"without any downside\" part? The extra context line\n> inhibits that the patch applies cleanly to a version of the file that has\n> that very line modified (including a different number of lines).\n\nWe could start allowing \"fuzz\" by default in the case of a patch with more \ncontext than we'd expect to see. That is, git-apply would ignore context \nlines more than 3 lines away from any changed lines, sharing the \nassumption of our patch-generation side that lines that far away don't \nmatter in general. (Now, if people were in the habit of including as \ncontext additional lines in fragile locations, this wouldn't be a good \nassumption, but I doubt anybody would be able to identify such lines to \ninclude them).\n\n\t-Daniel\n*This .sig left intentionally blank*"},{"id":"93633","messageId":"48FE3F53.6030303@lsrfire.ath.cx","threadId":"15974","inReplyTo":"48FD721D.9030105@viscovery.net","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2008-10-21T20:45:07Z","receivedAt":"2008-10-21T20:45:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Sixt schrieb:\n> René Scharfe schrieb:\n>> I have to admit my main motivation was that one line gap, where a chunk\n>> header hid an interesting line of context.  Showing it didn't change the\n>> length of the patch, so I found this to be a sad wastage.\n> \n> \"Wastage\" is relative. For a given patch, the one line of context that was\n> hidden by the hunk header would be welcome by a human reader, but it is\n> not necessarily useful if the patch is to be applied, in particular, if it\n> is applied to a version of the file that has *more* than one line between\n> the hunk contexts. This is the reason that diff does not produce 7 lines\n> of context between changes in -U3 mode (\"you asked for 3 lines of context,\n> you get 3 lines of context\").\n\nYes, that's an interesting example of the possible \"merge conflicts\" I\nmentioned in my original mail, and one I didn't think of.  And since I\ndon't do any merges myself, I don't know how much of a problem this is.\n\nAs Daniel writes, one could teach git-apply to ignore extra context.  It\nshould even be possible to infer the -U option used to create a patch\nand to remove any extra lines (which might get complicated for patches\nthat change both the start and the end of a file, though).\n\nAlso, I'd like to know how many patches of a given repo would be have\ncreated such a problem, but I can't think of a way to count them at the\nmoment.  I need some sleep first.\n\n> BTW, nomenclature seems to have settled at the word \"hunk\", not \"chunk\".\n\nWhile http://en.wikipedia.org/wiki/Diff uses both and defines \"chunk\" as\n\"change hunk\", the GNU patch(1) manpage uses \"hunk\" throughout.  \"Hunk\"\nit is, then. :)\n\nRené\n"},{"id":"93634","messageId":"48FE4034.2090707@lsrfire.ath.cx","threadId":"15974","inReplyTo":"20081021112040.GB17363@coredump.intra.peff.net","subject":"Re: [PATCH, RFC] diff: add option to show context between close chunks","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2008-10-21T20:48:52Z","receivedAt":"2008-10-21T20:48:52Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Jeff King schrieb:\n> On Tue, Oct 21, 2008 at 12:12:17AM -0700, Junio C Hamano wrote:\n> \n>> Yeah.  René wanted this for _human consumption_, not mechanical patch\n>> application, so \"hardcoding\" literally there in the very low level of the\n>> diff callchain is not quite right (it would affect format-patch which is\n>> primarily for mechanical application).\n>>\n>> I guess you could make the hardcoded value 1 for everybody else and 0 for\n>> format-patch.\n> \n> I see your reasoning, but at the same time, a large portion of patches I\n> read are from format-patch (and René even said that he was trying to\n> save the user from the \"apply then diff just to look at the patch\"\n> annoyance). And I have personally, as a patch submitter, created some\n> format-patch output sent to the git list with -U5 to combine hunks and\n> make it more readable for reviewers.\n> \n> Not to mention that I sometimes apply or post the output of \"git diff\".\n\nWell, yes, perhaps I was trying to get ahead of myself.  I sure would\nlike to see everyone create patches with fused hunks (because they are\neasier to read), but step 1 is to have the option to create such patches\nat all.  We should then try it out for some time or verify its\nusefulness statistically and only then turn it on by default.  Or\nperhaps throw it away, depending on the results.\n\nAnd I consider the output of format-patch and git-diff to be intended\nprimarily for human consumption.\n\n> To me that it implies that either:\n> \n>  - the increased chance of conflict is not a problem in practice, and we\n>    should have the option on by default everywhere\n> \n>  - it is a problem, in which case we should ask the user to turn on the\n>    feature manually instead of second-guessing how they will use the\n>    resulting patch (which they might not even know, since they are\n>    making assumptions about how other people might use the patch, and\n>    they must decide for their situation between shipping something that\n>    is more readable but slightly more conflict prone, or as easy to\n>    apply as possible)\n\nTo decide which one it is, I'd like to see numbers: how many times would\na patch with fused hunks have led to a problem for e.g. the kernel repo?\n  What is the optimal default value (0, 1, even more)?  Before even\nthinking about how to get these stats, I'd better head for bed for\ntoday, though..\n\nRené\n"}]}