{"thread":{"id":"8866","subject":"[PATCH 2/2] diff: add custom regular expressions for function names","startedAt":"2007-07-04T18:06:20Z","lastAt":"2007-07-09T11:40:41Z","messageCount":29,"participants":["Johannes Schindelin","Linus Torvalds","Junio C Hamano","Florian Weimer","しらいしななこ","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"46502","messageId":"Pine.LNX.4.64.0707041905570.4071@racer.site","threadId":"8866","inReplyTo":null,"subject":"[PATCH 2/2] diff: add custom regular expressions for function names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-04T18:06:20Z","receivedAt":"2007-07-04T18:06:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nFor convenience, the hunk headers contain function names. The\nheuristics used for that in \"git diff\" is extremely simple, but\nworks quite well for most C/C++ and script source code.\n\nHowever, it usually fails for Java source code.\n\nThis patch introduces a config variable diff.functionNameRegexp\nwhich replaces the default heuristics. If the pattern contains\na group, the match of this group is used for the hunk header\ninstead of the whole match.\n\nYou can give more than one regular expression (all of which have to\nmatch), by separating them with '\\n'. By prefixing an expression\nwith '!', it is negated, i.e. a non-match is required. The last\nexpression is used for the hunk header, and therefore must not be\nnegated.\n\nA special exception: since Java is so common nowadays, you can set\nthe config variable to \"java\", and it will be substituted by a sensible\nheuristic.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tYes, I know. .gitattributes integration would be nice. But for now\n\tI am done with it. I invested way too much time into this.\n\n Documentation/config.txt | 17 ++++++++++\n diff.c | 4 ++\n t/t4018-diff-funcname.sh | 60 +++++++++++++++++++++++++++++++++++\n xdiff-interface.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++\n xdiff-interface.h | 2 +\n xdiff/xdiff.h | 4 ++\n xdiff/xemit.c | 37 +++++++++++++--------\n 7 files changed, 188 insertions(+), 14 deletions(-)\n create mode 100644 t/t4018-diff-funcname.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1d96adf..326e27b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -390,6 +390,23 @@ diff.renames::\n \twill enable basic rename detection. If set to \"copies\" or\n \t\"copy\", it will detect copies, as well.\n \n+diff.functionNameRegexp::\n+\tInstead of the builtin heuristic to determine the function names\n+\tfor the hunk headers, use the given regular expression to match\n+\tlines which contain the function name. If this regular expression\n+\tcontains at least one group, the match of the first group is\n+\tused, rather than the whole line.\n++\n+Sometimes, one regular expression is not enough. Use '\\n' to separate\n+multiple regular expressions. All of these expressions have to match,\n+and the last one determines the returned function name. Prefix the\n+expression with '!' to require a non-match instead of a match.\n++\n+As a special exception, the pattern 'java' is substituted with a preset\n+pattern which can be expressed as: 'At least two identifiers, followed by\n+a \"(\", and not followed by a \";\", and the first identifier must not be a\n+reserved keyword such as \"new\", \"return\", etc.'\n+\n fetch.unpackLimit::\n \tIf the number of objects fetched over the git native\n \ttransfer is below this\ndiff --git a/diff.c b/diff.c\nindex bea0163..24b66d0 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -131,6 +131,10 @@ int git_diff_ui_config(const char *var, const char *value)\n \t\tcolor_parse(value, var, diff_colors[slot]);\n \t\treturn 0;\n \t}\n+\tif (!prefixcmp(var, \"diff.functionnameregexp\")) {\n+\t\txdiff_set_find_func(&default_xecfg, value);\n+\t\treturn 0;\n+\t}\n \n \treturn git_default_config(var, value);\n }\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nnew file mode 100644\nindex 0000000..90c6605\n--- /dev/null\n+++ b/t/t4018-diff-funcname.sh\n@@ -0,0 +1,60 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Johannes E. Schindelin\n+#\n+\n+test_description='Test custom diff function name patterns'\n+\n+. ./test-lib.sh\n+\n+LF='\n+'\n+\n+cat > Beer.java << EOF\n+public class Beer\n+{\n+\tint special;\n+\tpublic static void main(String args[])\n+\t{\n+\t\tString s=\" \";\n+\t\tfor(int x = 99; x > 0; x--)\n+\t\t{\n+\t\t\tSystem.out.print(x + \" bottles of beer on the wall \"\n+\t\t\t\t+ x + \" bottles of beer\\n\"\n+\t\t\t\t+ \"Take one down, pass it around, \" + (x - 1)\n+\t\t\t\t+ \" bottles of beer on the wall.\\n\");\n+\t\t}\n+\t\tSystem.out.print(\"Go to the store, buy some more,\\n\"\n+\t\t\t+ \"99 bottles of beer on the wall.\\n\");\n+\t}\n+}\n+EOF\n+\n+sed 's/beer\\\\/beer,\\\\/' < Beer.java > Beer-correct.java\n+\n+test_expect_success 'default behaviour' '\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ public class Beer\"\n+'\n+\n+test_expect_success 'preset java pattern' '\n+\tgit config diff.functionnameregexp java &&\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ public static void main(\"\n+'\n+\n+git config diff.functionnameregexp '!static\n+!String\n+[^ \t].*s.*'\n+\n+test_expect_success 'custom pattern' '\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ int special;$\"\n+'\n+\n+test_expect_success 'last regexp must not be negated' '\n+\tgit config diff.functionnameregexp \"!static\" &&\n+\t! git diff Beer.java Beer-correct.java\n+'\n+\n+test_done\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex e407cf1..b0cbd60 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -129,3 +129,81 @@ int buffer_is_binary(const char *ptr, unsigned long size)\n \t\tsize = FIRST_FEW_BYTES;\n \treturn !!memchr(ptr, 0, size);\n }\n+\n+struct ff_regs {\n+\tint nr;\n+\tstruct ff_reg {\n+\t\tregex_t re;\n+\t\tint negate;\n+\t} *array;\n+};\n+\n+static long ff_regexp(const char *line, long len,\n+\t\tchar *buffer, long buffer_size, void *priv)\n+{\n+\tchar *line_buffer = xstrndup(line, len); /* make NUL terminated */\n+\tstruct ff_regs *regs = priv;\n+\tregmatch_t pmatch[2];\n+\tint result = 0, i;\n+\n+\tfor (i = 0; i < regs->nr; i++) {\n+\t\tstruct ff_reg *reg = regs->array + i;\n+\t\tif (reg->negate ^ !!regexec(®->re,\n+\t\t\t\t\tline_buffer, 2, pmatch, 0)) {\n+\t\t\tfree(line_buffer);\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\ti = pmatch[1].rm_so >= 0 ? 1 : 0;\n+\tline += pmatch[i].rm_so;\n+\tresult = pmatch[i].rm_eo - pmatch[i].rm_so;\n+\tif (result > buffer_size)\n+\t\tresult = buffer_size;\n+\telse\n+\t\twhile (result > 0 && (isspace(line[result - 1]) ||\n+\t\t\t\t\tline[result - 1] == '\\n'))\n+\t\t\tresult--;\n+\tmemcpy(buffer, line, result);\n+\tfree(line_buffer);\n+\treturn result;\n+}\n+\n+void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)\n+{\n+\tint i;\n+\tstruct ff_regs *regs;\n+\n+\tif (!strcmp(value, \"java\"))\n+\t\tvalue = \"!^[ \t]*\\\\(catch\\\\|do\\\\|for\\\\|if\\\\|instanceof\\\\|\"\n+\t\t\t\t\"new\\\\|return\\\\|switch\\\\|throw\\\\|while\\\\)\\n\"\n+\t\t\t\"^[ \t]*\\\\(\\\\([ \t]*\"\n+\t\t\t\t\"[A-Za-z_][A-Za-z_0-9]*\\\\)\\\\{2,\\\\}\"\n+\t\t\t\t\"[ \t]*([^;]*$\\\\)\";\n+\n+\txecfg->find_func = ff_regexp;\n+\tregs = xecfg->find_func_priv = xmalloc(sizeof(struct ff_regs));\n+\tfor (i = 0, regs->nr = 1; value[i]; i++)\n+\t\tif (value[i] == '\\n')\n+\t\t\tregs->nr++;\n+\tregs->array = xmalloc(regs->nr * sizeof(struct ff_reg));\n+\tfor (i = 0; i < regs->nr; i++) {\n+\t\tstruct ff_reg *reg = regs->array + i;\n+\t\tconst char *ep = strchr(value, '\\n'), *expression;\n+\t\tchar *buffer = NULL;\n+\n+\t\treg->negate = (*value == '!');\n+\t\tif (reg->negate && i == regs->nr - 1)\n+\t\t\tdie(\"Last expression must not be negated: %s\", value);\n+\t\tif (*value == '!')\n+\t\t\tvalue++;\n+\t\tif (ep)\n+\t\t\texpression = buffer = xstrndup(value, ep - value);\n+\t\telse\n+\t\t\texpression = value;\n+\t\tif (regcomp(®->re, expression, 0))\n+\t\t\tdie(\"Invalid diff.functionNameRegexp: %s\", expression);\n+\t\tif (buffer)\n+\t\t\tfree(buffer);\n+\t\tvalue = ep + 1;\n+\t}\n+}\ndiff --git a/xdiff-interface.h b/xdiff-interface.h\nindex 536f4e4..fb742db 100644\n--- a/xdiff-interface.h\n+++ b/xdiff-interface.h\n@@ -20,4 +20,6 @@ int parse_hunk_header(char *line, int len,\n int read_mmfile(mmfile_t *ptr, const char *filename);\n int buffer_is_binary(const char *ptr, unsigned long size);\n \n+extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line);\n+\n #endif\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 9402bb0..c00ddaa 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -73,9 +73,13 @@ typedef struct s_xdemitcb {\n \tint (*outf)(void *, mmbuffer_t *, int);\n } xdemitcb_t;\n \n+typedef long (*find_func_t)(const char *line, long line_len, char *buffer, long buffer_size, void *priv);\n+\n typedef struct s_xdemitconf {\n \tlong ctxlen;\n \tunsigned long flags;\n+\tfind_func_t find_func;\n+\tvoid *find_func_priv;\n } xdemitconf_t;\n \n typedef struct s_bdiffparam {\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex 4b6e639..7237054 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -69,7 +69,24 @@ static xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) {\n }\n \n \n-static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll) {\n+static long def_ff(const char *rec, long len, char *buf, long sz, void *priv)\n+{\n+\tif (len > 0 &&\n+\t\t\t(isalpha((unsigned char)*rec) || /* identifier? */\n+\t\t\t *rec == '_' ||\t/* also identifier? */\n+\t\t\t *rec == '$')) { /* mysterious GNU diff's invention */\n+\t\tif (len > sz)\n+\t\t\tlen = sz;\n+\t\twhile (0 < len && isspace((unsigned char)rec[len - 1]))\n+\t\t\tlen--;\n+\t\tmemcpy(buf, rec, len);\n+\t\treturn len;\n+\t}\n+\treturn -1;\n+}\n+\n+static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll,\n+\t\tfind_func_t ff, void *ff_priv) {\n \n \t/*\n \t * Be quite stupid about this for now. Find a line in the old file\n@@ -80,22 +97,12 @@ static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll) {\n \tconst char *rec;\n \tlong len;\n \n-\t*ll = 0;\n \twhile (i-- > 0) {\n \t\tlen = xdl_get_rec(xf, i, &rec);\n-\t\tif (len > 0 &&\n-\t\t (isalpha((unsigned char)*rec) || /* identifier? */\n-\t\t *rec == '_' ||\t/* also identifier? */\n-\t\t *rec == '$')) {\t/* mysterious GNU diff's invention */\n-\t\t\tif (len > sz)\n-\t\t\t\tlen = sz;\n-\t\t\twhile (0 < len && isspace((unsigned char)rec[len - 1]))\n-\t\t\t\tlen--;\n-\t\t\tmemcpy(buf, rec, len);\n-\t\t\t*ll = len;\n+\t\tif ((*ll = ff(rec, len, buf, sz, ff_priv)) >= 0)\n \t\t\treturn;\n-\t\t}\n \t}\n+\t*ll = 0;\n }\n \n \n@@ -120,6 +127,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \txdchange_t *xch, *xche;\n \tchar funcbuf[80];\n \tlong funclen = 0;\n+\tfind_func_t ff = xecfg->find_func ? xecfg->find_func : def_ff;\n \n \tif (xecfg->flags & XDL_EMIT_COMMON)\n \t\treturn xdl_emit_common(xe, xscr, ecb, xecfg);\n@@ -143,7 +151,8 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \n \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n \t\t\txdl_find_func(&xe->xdf1, s1, funcbuf,\n-\t\t\t\t sizeof(funcbuf), &funclen);\n+\t\t\t\t sizeof(funcbuf), &funclen,\n+\t\t\t\t ff, xecfg->find_func_priv);\n \t\t}\n \t\tif (xdl_emit_hunk_hdr(s1 + 1, e1 - s1, s2 + 1, e2 - s2,\n \t\t\t\t funcbuf, funclen, ecb) < 0)\n-- \n1.5.3.rc0.804.g7068\n"},{"id":"46504","messageId":"alpine.LFD.0.98.0707041140230.9434@woody.linux-foundation.org","threadId":"8866","inReplyTo":"Pine.LNX.4.64.0707041905570.4071@racer.site","subject":"Re: [PATCH 2/2] diff: add custom regular expressions for function names","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-07-04T18:41:34Z","receivedAt":"2007-07-04T18:41:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 4 Jul 2007, Johannes Schindelin wrote:\n> \n> This patch introduces a config variable diff.functionNameRegexp\n> which replaces the default heuristics. If the pattern contains\n> a group, the match of this group is used for the hunk header\n> instead of the whole match.\n\nUmm. Shouldn't it be a path-name based attribute instead?\n\nThat way you can set it to be different things for different source files \nin the same repository.. IOW, think mixed Java/C codebases.\n\n\t\tLinus\n"},{"id":"46505","messageId":"Pine.LNX.4.64.0707041944520.4071@racer.site","threadId":"8866","inReplyTo":"alpine.LFD.0.98.0707041140230.9434@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] diff: add custom regular expressions for function names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-04T18:45:26Z","receivedAt":"2007-07-04T18:45:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Jul 2007, Linus Torvalds wrote:\n\n> On Wed, 4 Jul 2007, Johannes Schindelin wrote:\n> > \n> > This patch introduces a config variable diff.functionNameRegexp\n> > which replaces the default heuristics. If the pattern contains\n> > a group, the match of this group is used for the hunk header\n> > instead of the whole match.\n> \n> Umm. Shouldn't it be a path-name based attribute instead?\n\n>From my original mail:\n\n Yes, I know. .gitattributes integration would be nice. But for now\n I am done with it. I invested way too much time into this.\n\nCiao,\nDscho\n"},{"id":"46511","messageId":"7vejjnhpap.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"alpine.LFD.0.98.0707041140230.9434@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] diff: add custom regular expressions for function names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-04T20:44:30Z","receivedAt":"2007-07-04T20:44:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Wed, 4 Jul 2007, Johannes Schindelin wrote:\n>> \n>> This patch introduces a config variable diff.functionNameRegexp\n>> which replaces the default heuristics. If the pattern contains\n>> a group, the match of this group is used for the hunk header\n>> instead of the whole match.\n>\n> Umm. Shouldn't it be a path-name based attribute instead?\n>\n> That way you can set it to be different things for different source files \n> in the same repository.. IOW, think mixed Java/C codebases.\n\nAbsolutely. I'll take it from there.\n"},{"id":"46515","messageId":"Pine.LNX.4.64.0707042220130.4071@racer.site","threadId":"8866","inReplyTo":"7vejjnhpap.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] diff: add custom regular expressions for function names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-04T21:20:33Z","receivedAt":"2007-07-04T21:20:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Jul 2007, Junio C Hamano wrote:\n\n> I'll take it from there.\n\nThanks,\nDscho\n"},{"id":"46526","messageId":"7vwsxfe96i.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"7vejjnhpap.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] diff: add custom regular expressions for function names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-05T05:00:53Z","receivedAt":"2007-07-05T05:00:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Linus Torvalds writes:\n>\n>> On Wed, 4 Jul 2007, Johannes Schindelin wrote:\n>>> \n>>> This patch introduces a config variable diff.functionNameRegexp\n>>> which replaces the default heuristics. If the pattern contains\n>>> a group, the match of this group is used for the hunk header\n>>> instead of the whole match.\n>>\n>> Umm. Shouldn't it be a path-name based attribute instead?\n>>\n>> That way you can set it to be different things for different source files \n>> in the same repository.. IOW, think mixed Java/C codebases.\n>\n> Absolutely. I'll take it from there.\n\nI'll follow-up this message with two patches.\n\nThe first one is almost the same as Johannes's, but I got rid of\nthe new global variable. The second one discards Johannes's use\nof a single config variable and replaces it with gitattribute\naccess.\n\nThe second one is still a WIP; currently you can mark the path\nto use java convention to find hunk header with:\n\n\t$ echo '*.java\tfuncname=java' >.gitattributes\n\nIf there are useful \"hunk header regexp\" to make built-in, you\ncan add them to diff_hunk_header_regexp().\n\nWe also should allow user override or define custom regexp with\na configuration section, perhaps like:\n\n\t[funcname]\n \tjava = ...\n perl = ...\n default = ...\n\nThere are enough existing code in diff and convert area that use\n(path -> attribute -> config) mapping in a very similar way and\nany git hacker wannabe should be able to mimick them to\nimplement such a support, but tonight's WIP does not have it.\n\n*NOTE IN BIG RED LETTERS*\n\nI do not particularly like the way multiple regexps are used in\nJohannes's patch, but I left it as-is, as that part of the\nchange is orthogonal from the support to use the gitattribute\nmechanism, and the primary reason I got involved in this topic\nis to give help around the latter area.\n\nI think using multiple regexp is cute, but if we do that, it\nshould allow people to pick from:\n\n\tpublic class Beer\n\t{\n\t\tint special;\n\t\tpublic static void main(String args[])\n {\n \t... modified part is here ...\n\nwith two regexp matches, say:\n\n\t/^(public|private|protectd) class (.*)/ then\n /^\t+.* (\\w*\\(.*)$/\n\nand define the hunk_header format as something like:\n\n\t\"\\[1,2]::\\[2,1]\"\n\nmeaning, \"pick the second capture group from the match data of\nthe first regexp, followed by double-colon, and pick the first\ncapture group from the match data of the second regexp\", to\nresult in \"Beer::main(String args[])\". You should be able\nto pick \"/package (\\w+);/ then /^sub (\\w+)/\" in Perl code using\nthe same idea.\n\nI am not married to the syntax I used in the above examples,\nthough.\n"},{"id":"46527","messageId":"7vr6nne949.fsf_-_@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"7vwsxfe96i.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 1/2] Future-proof source for changes in xdemitconf_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-05T05:02:14Z","receivedAt":"2007-07-05T05:02:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Johannes Schindelin \nDate: Wed, 4 Jul 2007 19:05:46 +0100\n\nThe instances of xdemitconf_t were initialized member by member.\nInstead, initialize them to all zero, so we do not have\nto update those places each time we introduce a new member.\n\n[jc: minimally fixed by getting rid of a new global]\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Junio C Hamano \n---\n builtin-blame.c | 2 +-\n builtin-rerere.c | 2 +-\n combine-diff.c | 3 +--\n diff.c | 10 +++++-----\n merge-file.c | 1 +\n merge-tree.c | 2 +-\n 6 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex da23a6f..0519339 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -518,8 +518,8 @@ static struct patch *compare_buffer(mmfile_t *file_p, mmfile_t *file_o,\n \txdemitcb_t ecb;\n \n \txpp.flags = xdl_opts;\n+\tmemset(&xecfg, 0, sizeof(xecfg));\n \txecfg.ctxlen = context;\n-\txecfg.flags = 0;\n \tecb.outf = xdiff_outf;\n \tecb.priv = &state;\n \tmemset(&state, 0, sizeof(state));\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex 29fb075..01f3848 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -282,8 +282,8 @@ static int diff_two(const char *file1, const char *label1,\n \tprintf(\"--- a/%s\\n+++ b/%s\\n\", label1, label2);\n \tfflush(stdout);\n \txpp.flags = XDF_NEED_MINIMAL;\n+\tmemset(&xecfg, 0, sizeof(xecfg));\n \txecfg.ctxlen = 3;\n-\txecfg.flags = 0;\n \tecb.outf = outf;\n \txdl_diff(&minus, &plus, &xpp, &xecfg, &ecb);\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex ea3ca5f..ef62234 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -215,8 +215,7 @@ static void combine_diff(const unsigned char *parent, mmfile_t *result_file,\n \tparent_file.ptr = grab_blob(parent, &sz);\n \tparent_file.size = sz;\n \txpp.flags = XDF_NEED_MINIMAL;\n-\txecfg.ctxlen = 0;\n-\txecfg.flags = 0;\n+\tmemset(&xecfg, 0, sizeof(xecfg));\n \tecb.outf = xdiff_outf;\n \tecb.priv = &state;\n \tmemset(&state, 0, sizeof(state));\ndiff --git a/diff.c b/diff.c\nindex 1958970..552f7c0 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -390,6 +390,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tmmfile_t minus, plus;\n \tint i;\n \n+\tmemset(&xecfg, 0, sizeof(xecfg));\n \tminus.size = diff_words->minus.text.size;\n \tminus.ptr = xmalloc(minus.size);\n \tmemcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);\n@@ -408,7 +409,6 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \n \txpp.flags = XDF_NEED_MINIMAL;\n \txecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;\n-\txecfg.flags = 0;\n \tecb.outf = xdiff_outf;\n \tecb.priv = diff_words;\n \tdiff_words->xm.consume = fn_out_diff_words_aux;\n@@ -1202,6 +1202,7 @@ static void builtin_diff(const char *name_a,\n \t\txdemitcb_t ecb;\n \t\tstruct emit_callback ecbdata;\n \n+\t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\tmemset(&ecbdata, 0, sizeof(ecbdata));\n \t\tecbdata.label_path = lbl;\n \t\tecbdata.color_diff = o->color_diff;\n@@ -1270,9 +1271,8 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t\txdemitconf_t xecfg;\n \t\txdemitcb_t ecb;\n \n+\t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n-\t\txecfg.ctxlen = 0;\n-\t\txecfg.flags = 0;\n \t\tecb.outf = xdiff_outf;\n \t\tecb.priv = diffstat;\n \t\txdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n@@ -1310,9 +1310,8 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\txdemitconf_t xecfg;\n \t\txdemitcb_t ecb;\n \n+\t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\txpp.flags = XDF_NEED_MINIMAL;\n-\t\txecfg.ctxlen = 0;\n-\t\txecfg.flags = 0;\n \t\tecb.outf = xdiff_outf;\n \t\tecb.priv = &data;\n \t\txdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n@@ -2764,6 +2763,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tint len1, len2;\n \n+\t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\tif (p->status == 0)\n \t\t\treturn error(\"internal diff status error\");\n \t\tif (p->status == DIFF_STATUS_UNKNOWN)\ndiff --git a/merge-file.c b/merge-file.c\nindex 748d15c..1e031ea 100644\n--- a/merge-file.c\n+++ b/merge-file.c\n@@ -62,6 +62,7 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)\n \txdemitcb_t ecb;\n \n \txpp.flags = XDF_NEED_MINIMAL;\n+\tmemset(&xecfg, 0, sizeof(xecfg));\n \txecfg.ctxlen = 3;\n \txecfg.flags = XDL_EMIT_COMMON;\n \tecb.outf = common_outf;\ndiff --git a/merge-tree.c b/merge-tree.c\nindex 3b8d9e6..7d4f628 100644\n--- a/merge-tree.c\n+++ b/merge-tree.c\n@@ -106,8 +106,8 @@ static void show_diff(struct merge_list *entry)\n \txdemitcb_t ecb;\n \n \txpp.flags = XDF_NEED_MINIMAL;\n+\tmemset(&xecfg, 0, sizeof(xecfg));\n \txecfg.ctxlen = 3;\n-\txecfg.flags = 0;\n \tecb.outf = show_outf;\n \tecb.priv = NULL;\n \n-- \n1.5.3.rc0.818.ga2b52\n"},{"id":"46528","messageId":"7vlkdve93o.fsf_-_@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"7vwsxfe96i.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 2/2] WIP per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-05T05:02:35Z","receivedAt":"2007-07-05T05:02:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This makes\"diff -p\" hunk headers customizable via gitattributes mechanism.\nIt is based on Johannes's earlier patch that allowed to define a single\nregexp to be used for everything.\n\nThe mechanism to arrive at the regexp that is used to define hunk header\nis the same as other use of gitattributes. You assign an attribute, funcname\n(because \"diff -p\" typically uses the name of the function the patch is about\nas the hunk header), a simple string value. This can be one of the names of\nbuilt-in pattern (currently, \"java\" is defined) or a custom pattern name, to\nbe looked up from the configuration file.\n\n (in .gitattributes)\n *.java funcname=java\n *.perl funcname=perl\n\n (in .git/config)\n [funcname]\n java = ... # ugly and complicated regexp to override the built-in one.\n perl = ... # another ugly and complicated regexp to define a new one.\n\nThis is still a WIP in that the code to look-up the configuration is not there\nyet.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 121 +++++++++++++++++++++++++++++++++-------------\n diffcore-delta.c | 2 +-\n diffcore.h | 3 +\n t/t4018-diff-funcname.sh | 63 ++++++++++++++++++++++++\n xdiff-interface.c | 71 +++++++++++++++++++++++++++\n xdiff-interface.h | 2 +\n xdiff/xdiff.h | 4 ++\n xdiff/xemit.c | 37 +++++++++-----\n 8 files changed, 254 insertions(+), 49 deletions(-)\n create mode 100644 t/t4018-diff-funcname.sh\n\ndiff --git a/diff.c b/diff.c\nindex 552f7c0..0c7d2c6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1101,31 +1101,91 @@ static void emit_binary_diff(mmfile_t *one, mmfile_t *two)\n static void setup_diff_attr_check(struct git_attr_check *check)\n {\n \tstatic struct git_attr *attr_diff;\n+\tstatic struct git_attr *attr_diff_func_name;\n \n-\tif (!attr_diff)\n+\tif (!attr_diff) {\n \t\tattr_diff = git_attr(\"diff\", 4);\n-\tcheck->attr = attr_diff;\n+\t\tattr_diff_func_name = git_attr(\"funcname\", 8);\n+\t}\n+\tcheck[0].attr = attr_diff;\n+\tcheck[1].attr = attr_diff_func_name;\n }\n \n-static int file_is_binary(struct diff_filespec *one)\n+static void diff_filespec_check_attr(struct diff_filespec *one)\n {\n-\tstruct git_attr_check attr_diff_check;\n+\tstruct git_attr_check attr_diff_check[2];\n \n-\tsetup_diff_attr_check(&attr_diff_check);\n-\tif (!git_checkattr(one->path, 1, &attr_diff_check)) {\n-\t\tconst char *value = attr_diff_check.value;\n+\tif (one->checked_attr)\n+\t\treturn;\n+\n+\tsetup_diff_attr_check(attr_diff_check);\n+\tone->is_binary = 0;\n+\tone->hunk_header_ident = NULL;\n+\n+\tif (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) {\n+\t\tconst char *value;\n+\n+\t\t/* binaryness */\n+\t\tvalue = attr_diff_check[0].value;\n \t\tif (ATTR_TRUE(value))\n-\t\t\treturn 0;\n+\t\t\t;\n \t\telse if (ATTR_FALSE(value))\n-\t\t\treturn 1;\n+\t\t\tone->is_binary = 1;\n+\n+\t\t/* hunk header ident */\n+\t\tvalue = attr_diff_check[1].value;\n+\t\tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))\n+\t\t\t;\n+\t\telse\n+\t\t\tone->hunk_header_ident = value;\n \t}\n \n-\tif (!one->data) {\n-\t\tif (!DIFF_FILE_VALID(one))\n-\t\t\treturn 0;\n+\tif (!one->data && DIFF_FILE_VALID(one))\n \t\tdiff_populate_filespec(one, 0);\n+\n+\tif (one->data)\n+\t\tone->is_binary = buffer_is_binary(one->data, one->size);\n+\n+}\n+\n+int diff_filespec_is_binary(struct diff_filespec *one)\n+{\n+\tdiff_filespec_check_attr(one);\n+\treturn one->is_binary;\n+}\n+\n+static const char *diff_hunk_header_regexp(struct diff_filespec *one)\n+{\n+\tconst char *ident;\n+\n+\tdiff_filespec_check_attr(one);\n+\tident = one->hunk_header_ident;\n+\n+\tif (!ident) {\n+\t\t/*\n+\t\t * If the config file has \"funcname.default\" defined, return\n+\t\t * that regexp here, instead of NULL.\n+\t\t */\n+\t\treturn NULL;\n \t}\n-\treturn buffer_is_binary(one->data, one->size);\n+\n+\t/*\n+\t * FIXME: look up custom \"funcname.$ident\" regexp from config,\n+\t * and return it here.\n+\t */\n+\n+\n+\t/*\n+\t * And define built-in fallback patterns here...\n+\t */\n+\tif (!strcmp(ident, \"java\"))\n+\t\treturn \"!^[ \t]*\\\\(catch\\\\|do\\\\|for\\\\|if\\\\|instanceof\\\\|\"\n+\t\t\t\"new\\\\|return\\\\|switch\\\\|throw\\\\|while\\\\)\\n\"\n+\t\t\t\"^[ \t]*\\\\(\\\\([ \t]*\"\n+\t\t\t\"[A-Za-z_][A-Za-z_0-9]*\\\\)\\\\{2,\\\\}\"\n+\t\t\t\"[ \t]*([^;]*$\\\\)\";\n+\n+\treturn NULL;\n }\n \n static void builtin_diff(const char *name_a,\n@@ -1182,7 +1242,8 @@ static void builtin_diff(const char *name_a,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (!o->text && (file_is_binary(one) || file_is_binary(two))) {\n+\tif (!o->text &&\n+\t (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {\n \t\t/* Quite common confusing case */\n \t\tif (mf1.size == mf2.size &&\n \t\t !memcmp(mf1.ptr, mf2.ptr, mf1.size))\n@@ -1201,6 +1262,11 @@ static void builtin_diff(const char *name_a,\n \t\txdemitconf_t xecfg;\n \t\txdemitcb_t ecb;\n \t\tstruct emit_callback ecbdata;\n+\t\tconst char *hunk_header_regexp;\n+\n+\t\thunk_header_regexp = diff_hunk_header_regexp(one);\n+\t\tif (!hunk_header_regexp)\n+\t\t\thunk_header_regexp = diff_hunk_header_regexp(two);\n \n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\tmemset(&ecbdata, 0, sizeof(ecbdata));\n@@ -1210,6 +1276,8 @@ static void builtin_diff(const char *name_a,\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\n \t\txecfg.flags = XDL_EMIT_FUNCNAMES;\n+\t\tif (hunk_header_regexp)\n+\t\t\txdiff_set_find_func(&xecfg, hunk_header_regexp);\n \t\tif (!diffopts)\n \t\t\t;\n \t\telse if (!prefixcmp(diffopts, \"--unified=\"))\n@@ -1261,7 +1329,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (file_is_binary(one) || file_is_binary(two)) {\n+\tif (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {\n \t\tdata->is_binary = 1;\n \t\tdata->added = mf2.size;\n \t\tdata->deleted = mf1.size;\n@@ -1302,7 +1370,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (file_is_binary(two))\n+\tif (diff_filespec_is_binary(two))\n \t\tgoto free_and_return;\n \telse {\n \t\t/* Crazy xdl interfaces.. */\n@@ -1879,8 +1947,8 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \n \t\tif (o->binary) {\n \t\t\tmmfile_t mf;\n-\t\t\tif ((!fill_mmfile(&mf, one) && file_is_binary(one)) ||\n-\t\t\t (!fill_mmfile(&mf, two) && file_is_binary(two)))\n+\t\t\tif ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||\n+\t\t\t (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))\n \t\t\t\tabbrev = 40;\n \t\t}\n \t\tlen += snprintf(msg + len, sizeof(msg) - len,\n@@ -2783,7 +2851,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\t\treturn error(\"unable to read files to diff\");\n \n \t\t/* Maybe hash p->two? into the patch id? */\n-\t\tif (file_is_binary(p->two))\n+\t\tif (diff_filespec_is_binary(p->two))\n \t\t\tcontinue;\n \n \t\tlen1 = remove_space(p->one->path, strlen(p->one->path));\n@@ -3011,21 +3079,6 @@ void diffcore_std(struct diff_options *options)\n \tif (options->quiet)\n \t\treturn;\n \n-\t/*\n-\t * break/rename count similarity differently depending on\n-\t * the binary-ness.\n-\t */\n-\tif ((options->break_opt != -1) || (options->detect_rename)) {\n-\t\tstruct diff_queue_struct *q = &diff_queued_diff;\n-\t\tint i;\n-\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tp->one->is_binary = file_is_binary(p->one);\n-\t\t\tp->two->is_binary = file_is_binary(p->two);\n-\t\t}\n-\t}\n-\n \tif (options->break_opt != -1)\n \t\tdiffcore_break(options->break_opt);\n \tif (options->detect_rename)\ndiff --git a/diffcore-delta.c b/diffcore-delta.c\nindex a038b16..d9729e5 100644\n--- a/diffcore-delta.c\n+++ b/diffcore-delta.c\n@@ -129,7 +129,7 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one)\n \tstruct spanhash_top *hash;\n \tunsigned char *buf = one->data;\n \tunsigned int sz = one->size;\n-\tint is_text = !one->is_binary;\n+\tint is_text = !diff_filespec_is_binary(one);\n \n \ti = INITIAL_HASH_SIZE;\n \thash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<mode) != 0)\n \tunsigned should_free : 1; /* data should be free()'ed */\n \tunsigned should_munmap : 1; /* data should be munmap()'ed */\n+\tunsigned checked_attr : 1;\n \tunsigned is_binary : 1; /* data should be considered \"binary\" */\n };\n \n@@ -46,6 +48,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \n extern int diff_populate_filespec(struct diff_filespec *, int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n+extern int diff_filespec_is_binary(struct diff_filespec *);\n \n struct diff_filepair {\n \tstruct diff_filespec *one;\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nnew file mode 100644\nindex 0000000..0689b2f\n--- /dev/null\n+++ b/t/t4018-diff-funcname.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Johannes E. Schindelin\n+#\n+\n+test_description='Test custom diff function name patterns'\n+\n+. ./test-lib.sh\n+\n+LF='\n+'\n+\n+cat > Beer.java << EOF\n+public class Beer\n+{\n+\tint special;\n+\tpublic static void main(String args[])\n+\t{\n+\t\tString s=\" \";\n+\t\tfor(int x = 99; x > 0; x--)\n+\t\t{\n+\t\t\tSystem.out.print(x + \" bottles of beer on the wall \"\n+\t\t\t\t+ x + \" bottles of beer\\n\"\n+\t\t\t\t+ \"Take one down, pass it around, \" + (x - 1)\n+\t\t\t\t+ \" bottles of beer on the wall.\\n\");\n+\t\t}\n+\t\tSystem.out.print(\"Go to the store, buy some more,\\n\"\n+\t\t\t+ \"99 bottles of beer on the wall.\\n\");\n+\t}\n+}\n+EOF\n+\n+sed 's/beer\\\\/beer,\\\\/' < Beer.java > Beer-correct.java\n+\n+test_expect_success 'default behaviour' '\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ public class Beer\"\n+'\n+\n+test_expect_success 'preset java pattern' '\n+\techo \"*.java funcname=java\" >.gitattributes &&\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ public static void main(\"\n+'\n+\n+test_done\n+\n+\n+git config diff.functionnameregexp '!static\n+!String\n+[^ \t].*s.*'\n+\n+test_expect_success 'custom pattern' '\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ int special;$\"\n+'\n+\n+test_expect_success 'last regexp must not be negated' '\n+\tgit config diff.functionnameregexp \"!static\" &&\n+\t! git diff Beer.java Beer-correct.java\n+'\n+\n+test_done\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex e407cf1..be866d1 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -129,3 +129,74 @@ int buffer_is_binary(const char *ptr, unsigned long size)\n \t\tsize = FIRST_FEW_BYTES;\n \treturn !!memchr(ptr, 0, size);\n }\n+\n+struct ff_regs {\n+\tint nr;\n+\tstruct ff_reg {\n+\t\tregex_t re;\n+\t\tint negate;\n+\t} *array;\n+};\n+\n+static long ff_regexp(const char *line, long len,\n+\t\tchar *buffer, long buffer_size, void *priv)\n+{\n+\tchar *line_buffer = xstrndup(line, len); /* make NUL terminated */\n+\tstruct ff_regs *regs = priv;\n+\tregmatch_t pmatch[2];\n+\tint result = 0, i;\n+\n+\tfor (i = 0; i < regs->nr; i++) {\n+\t\tstruct ff_reg *reg = regs->array + i;\n+\t\tif (reg->negate ^ !!regexec(®->re,\n+\t\t\t\t\tline_buffer, 2, pmatch, 0)) {\n+\t\t\tfree(line_buffer);\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\ti = pmatch[1].rm_so >= 0 ? 1 : 0;\n+\tline += pmatch[i].rm_so;\n+\tresult = pmatch[i].rm_eo - pmatch[i].rm_so;\n+\tif (result > buffer_size)\n+\t\tresult = buffer_size;\n+\telse\n+\t\twhile (result > 0 && (isspace(line[result - 1]) ||\n+\t\t\t\t\tline[result - 1] == '\\n'))\n+\t\t\tresult--;\n+\tmemcpy(buffer, line, result);\n+\tfree(line_buffer);\n+\treturn result;\n+}\n+\n+void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)\n+{\n+\tint i;\n+\tstruct ff_regs *regs;\n+\n+\txecfg->find_func = ff_regexp;\n+\tregs = xecfg->find_func_priv = xmalloc(sizeof(struct ff_regs));\n+\tfor (i = 0, regs->nr = 1; value[i]; i++)\n+\t\tif (value[i] == '\\n')\n+\t\t\tregs->nr++;\n+\tregs->array = xmalloc(regs->nr * sizeof(struct ff_reg));\n+\tfor (i = 0; i < regs->nr; i++) {\n+\t\tstruct ff_reg *reg = regs->array + i;\n+\t\tconst char *ep = strchr(value, '\\n'), *expression;\n+\t\tchar *buffer = NULL;\n+\n+\t\treg->negate = (*value == '!');\n+\t\tif (reg->negate && i == regs->nr - 1)\n+\t\t\tdie(\"Last expression must not be negated: %s\", value);\n+\t\tif (*value == '!')\n+\t\t\tvalue++;\n+\t\tif (ep)\n+\t\t\texpression = buffer = xstrndup(value, ep - value);\n+\t\telse\n+\t\t\texpression = value;\n+\t\tif (regcomp(®->re, expression, 0))\n+\t\t\tdie(\"Invalid regexp to look for hunk header: %s\", expression);\n+\t\tif (buffer)\n+\t\t\tfree(buffer);\n+\t\tvalue = ep + 1;\n+\t}\n+}\ndiff --git a/xdiff-interface.h b/xdiff-interface.h\nindex 536f4e4..fb742db 100644\n--- a/xdiff-interface.h\n+++ b/xdiff-interface.h\n@@ -20,4 +20,6 @@ int parse_hunk_header(char *line, int len,\n int read_mmfile(mmfile_t *ptr, const char *filename);\n int buffer_is_binary(const char *ptr, unsigned long size);\n \n+extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line);\n+\n #endif\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 9402bb0..c00ddaa 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -73,9 +73,13 @@ typedef struct s_xdemitcb {\n \tint (*outf)(void *, mmbuffer_t *, int);\n } xdemitcb_t;\n \n+typedef long (*find_func_t)(const char *line, long line_len, char *buffer, long buffer_size, void *priv);\n+\n typedef struct s_xdemitconf {\n \tlong ctxlen;\n \tunsigned long flags;\n+\tfind_func_t find_func;\n+\tvoid *find_func_priv;\n } xdemitconf_t;\n \n typedef struct s_bdiffparam {\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex 4b6e639..d3d9c84 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -69,7 +69,24 @@ static xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) {\n }\n \n \n-static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll) {\n+static long def_ff(const char *rec, long len, char *buf, long sz, void *priv)\n+{\n+\tif (len > 0 &&\n+\t\t\t(isalpha((unsigned char)*rec) || /* identifier? */\n+\t\t\t *rec == '_' ||\t/* also identifier? */\n+\t\t\t *rec == '$')) { /* identifiers from VMS and other esoterico */\n+\t\tif (len > sz)\n+\t\t\tlen = sz;\n+\t\twhile (0 < len && isspace((unsigned char)rec[len - 1]))\n+\t\t\tlen--;\n+\t\tmemcpy(buf, rec, len);\n+\t\treturn len;\n+\t}\n+\treturn -1;\n+}\n+\n+static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll,\n+\t\tfind_func_t ff, void *ff_priv) {\n \n \t/*\n \t * Be quite stupid about this for now. Find a line in the old file\n@@ -80,22 +97,12 @@ static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll) {\n \tconst char *rec;\n \tlong len;\n \n-\t*ll = 0;\n \twhile (i-- > 0) {\n \t\tlen = xdl_get_rec(xf, i, &rec);\n-\t\tif (len > 0 &&\n-\t\t (isalpha((unsigned char)*rec) || /* identifier? */\n-\t\t *rec == '_' ||\t/* also identifier? */\n-\t\t *rec == '$')) {\t/* mysterious GNU diff's invention */\n-\t\t\tif (len > sz)\n-\t\t\t\tlen = sz;\n-\t\t\twhile (0 < len && isspace((unsigned char)rec[len - 1]))\n-\t\t\t\tlen--;\n-\t\t\tmemcpy(buf, rec, len);\n-\t\t\t*ll = len;\n+\t\tif ((*ll = ff(rec, len, buf, sz, ff_priv)) >= 0)\n \t\t\treturn;\n-\t\t}\n \t}\n+\t*ll = 0;\n }\n \n \n@@ -120,6 +127,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \txdchange_t *xch, *xche;\n \tchar funcbuf[80];\n \tlong funclen = 0;\n+\tfind_func_t ff = xecfg->find_func ? xecfg->find_func : def_ff;\n \n \tif (xecfg->flags & XDL_EMIT_COMMON)\n \t\treturn xdl_emit_common(xe, xscr, ecb, xecfg);\n@@ -143,7 +151,8 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \n \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n \t\t\txdl_find_func(&xe->xdf1, s1, funcbuf,\n-\t\t\t\t sizeof(funcbuf), &funclen);\n+\t\t\t\t sizeof(funcbuf), &funclen,\n+\t\t\t\t ff, xecfg->find_func_priv);\n \t\t}\n \t\tif (xdl_emit_hunk_hdr(s1 + 1, e1 - s1, s2 + 1, e2 - s2,\n \t\t\t\t funcbuf, funclen, ecb) < 0)\n-- \n1.5.3.rc0.818.ga2b52\n"},{"id":"46535","messageId":"87644zcl5x.fsf@mid.deneb.enyo.de","threadId":"8866","inReplyTo":"7vwsxfe96i.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] diff: add custom regular expressions for function names","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2007-07-05T08:24:58Z","receivedAt":"2007-07-05T08:24:58Z","isPatch":true,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Junio C. Hamano:\n\n> I think using multiple regexp is cute, but if we do that, it\n> should allow people to pick from:\n>\n> \tpublic class Beer\n> \t{\n> \t\tint special;\n> \t\tpublic static void main(String args[])\n> {\n> \t... modified part is here ...\n>\n> with two regexp matches, say:\n>\n> \t/^(public|private|protectd) class (.*)/ then\n> /^\t+.* (\\w*\\(.*)$/\n>\n> and define the hunk_header format as something like:\n>\n> \t\"\\[1,2]::\\[2,1]\"\n\nEven that doesn't work for C++ namespaces, or nested Java classes. 8-(\nIf you want to do it right, you need to spawn some helper program.\n"},{"id":"46538","messageId":"Pine.LNX.4.64.0707051200540.9789@racer.site","threadId":"8866","inReplyTo":"7vr6nne949.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Future-proof source for changes in xdemitconf_t","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-05T11:01:44Z","receivedAt":"2007-07-05T11:01:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Jul 2007, Junio C Hamano wrote:\n\n> [jc: minimally fixed by getting rid of a new global]\n\nFunny. This is identical to my first version of that patch ;-)\n\nCiao,\nDscho\n"},{"id":"46540","messageId":"Pine.LNX.4.64.0707051223520.9789@racer.site","threadId":"8866","inReplyTo":"7vlkdve93o.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] WIP per-path attribute based hunk header selection.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-05T11:25:36Z","receivedAt":"2007-07-05T11:25:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Jul 2007, Junio C Hamano wrote:\n\n> diff --git a/diff.c b/diff.c\n> index 552f7c0..0c7d2c6 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3011,21 +3079,6 @@ void diffcore_std(struct diff_options *options)\n> \tif (options->quiet)\n> \t\treturn;\n> \n> -\t/*\n> -\t * break/rename count similarity differently depending on\n> -\t * the binary-ness.\n> -\t */\n> -\tif ((options->break_opt != -1) || (options->detect_rename)) {\n> -\t\tstruct diff_queue_struct *q = &diff_queued_diff;\n> -\t\tint i;\n> -\n> -\t\tfor (i = 0; i < q->nr; i++) {\n> -\t\t\tstruct diff_filepair *p = q->queue[i];\n> -\t\t\tp->one->is_binary = file_is_binary(p->one);\n> -\t\t\tp->two->is_binary = file_is_binary(p->two);\n> -\t\t}\n> -\t}\n> -\n> \tif (options->break_opt != -1)\n> \t\tdiffcore_break(options->break_opt);\n> \tif (options->detect_rename)\n> diff --git a/diffcore-delta.c b/diffcore-delta.c\n> index a038b16..d9729e5 100644\n> --- a/diffcore-delta.c\n> +++ b/diffcore-delta.c\n> @@ -129,7 +129,7 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one)\n> \tstruct spanhash_top *hash;\n> \tunsigned char *buf = one->data;\n> \tunsigned int sz = one->size;\n> -\tint is_text = !one->is_binary;\n> +\tint is_text = !diff_filespec_is_binary(one);\n> \n> \ti = INITIAL_HASH_SIZE;\n> \thash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1< *NOTE IN BIG RED LETTERS*\n> \n> I do not particularly like the way multiple regexps are used in\n> Johannes's patch, but I left it as-is, as that part of the\n> change is orthogonal from the support to use the gitattribute\n> mechanism, and the primary reason I got involved in this topic\n> is to give help around the latter area.\n\nI do not like it particularly, either. Until two days ago, that POSIX \nregexps suck so much, in that they do not even have a look-ahead, let \nalone a negative look-ahead. _That_ is what I wanted.\n\nAlas, that is the first valid argument I encounter that would speak _for_ \npcre. It speaks volumes that _nobody_ pointed this out before, but all \npcre fans put out bogus arguments.\n\n> I think using multiple regexp is cute, but if we do that, it\n> should allow people to pick from:\n> \n> \tpublic class Beer\n> \t{\n> \t\tint special;\n> \t\tpublic static void main(String args[])\n> {\n> \t... modified part is here ...\n> \n> with two regexp matches, say:\n> \n> \t/^(public|private|protectd) class (.*)/ then\n> /^\t+.* (\\w*\\(.*)$/\n> \n> and define the hunk_header format as something like:\n> \n> \t\"\\[1,2]::\\[2,1]\"\n> \n> meaning, \"pick the second capture group from the match data of\n> the first regexp, followed by double-colon, and pick the first\n> capture group from the match data of the second regexp\", to\n> result in \"Beer::main(String args[])\". You should be able\n> to pick \"/package (\\w+);/ then /^sub (\\w+)/\" in Perl code using\n> the same idea.\n> \n> I am not married to the syntax I used in the above examples,\n> though.\n\nIs that really necessary?\n\nBut yeah, it is possible. You'd have to have some state in the struct, and \nintroduce yet another escape (probably in place of the first line of the \nregexp), similar to\n\n\t*[2,2]::[1,1]\n\t*keep-searching-after-match 1\n\t^\t+.* (\\w*\\(.*)$\n\t^(public|private|protected) class (.*)\n\nOf course, there _has_ to be a way to handle exceptions, such as in Perl, \nwhere you do not necessarily have a package, and thus do not want that \nsilly \"::\" in front of the rest.\n\nBut frankly, I think this is too complicated. Maybe somebody more \nintelligent than me can actually come up with a sane syntax and \nimplementation of this, but at some point I think it is Good Enough(tm). \nAnd IMHO it is already good enough matching just one line.\n\nCiao,\nDscho\n"},{"id":"46555","messageId":"7vwsxedcng.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"Pine.LNX.4.64.0707051223520.9789@racer.site","subject":"Re: [PATCH 2/2] WIP per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-05T16:43:31Z","receivedAt":"2007-07-05T16:43:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Wed, 4 Jul 2007, Junio C Hamano wrote:\n>\n>> diff --git a/diff.c b/diff.c\n>> index 552f7c0..0c7d2c6 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -3011,21 +3079,6 @@ void diffcore_std(struct diff_options *options)\n>> \tif (options->quiet)\n>> \t\treturn;\n>> \n>> -\t/*\n>> -\t * break/rename count similarity differently depending on\n>> -\t * the binary-ness.\n>> -\t */\n>> -\tif ((options->break_opt != -1) || (options->detect_rename)) {\n>> -\t\tstruct diff_queue_struct *q = &diff_queued_diff;\n>> -\t\tint i;\n>> -\n>> -\t\tfor (i = 0; i < q->nr; i++) {\n>> -\t\t\tstruct diff_filepair *p = q->queue[i];\n>> -\t\t\tp->one->is_binary = file_is_binary(p->one);\n>> -\t\t\tp->two->is_binary = file_is_binary(p->two);\n>> -\t\t}\n>> -\t}\n>> -\n>> \tif (options->break_opt != -1)\n>> \t\tdiffcore_break(options->break_opt);\n>> \tif (options->detect_rename)\n>> diff --git a/diffcore-delta.c b/diffcore-delta.c\n>> index a038b16..d9729e5 100644\n>> --- a/diffcore-delta.c\n>> +++ b/diffcore-delta.c\n>> @@ -129,7 +129,7 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one)\n>> \tstruct spanhash_top *hash;\n>> \tunsigned char *buf = one->data;\n>> \tunsigned int sz = one->size;\n>> -\tint is_text = !one->is_binary;\n>> +\tint is_text = !diff_filespec_is_binary(one);\n>> \n>> \ti = INITIAL_HASH_SIZE;\n>> \thash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<\n> It is quite cute to hide this in the funcname patch...\n\nThat is why it is marked as WIP.\n"},{"id":"46605","messageId":"7vejjmexm0.fsf_-_@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"Pine.LNX.4.64.0707051223520.9789@racer.site","subject":"[PATCH 1/3] Introduce diff_filespec_is_binary()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-06T08:37:43Z","receivedAt":"2007-07-06T08:37:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This replaces an explicit initialization of filespec->is_binary\nfield used for rename/break followed by direct access to that\nfield with a wrapper function that lazily iniaitlizes and\naccesses the field. We would add more attribute accesses for\nthe use of diff routines, and it would be better to make this\nabstraction earlier.\n\nSigned-off-by: Junio C Hamano \n---\n Johannes Schindelin writes:\n\n > It is quite cute to hide this in the funcname patch...\n\n So this is the proper refactoring _before_ any of your topics.\n\n diff.c | 71 +++++++++++++++++++++++++++--------------------------\n diffcore-delta.c | 2 +-\n diffcore.h | 2 +\n 3 files changed, 39 insertions(+), 36 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 1958970..16ea710 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1102,30 +1102,45 @@ static void setup_diff_attr_check(struct git_attr_check *check)\n {\n \tstatic struct git_attr *attr_diff;\n \n-\tif (!attr_diff)\n+\tif (!attr_diff) {\n \t\tattr_diff = git_attr(\"diff\", 4);\n-\tcheck->attr = attr_diff;\n+\t}\n+\tcheck[0].attr = attr_diff;\n }\n \n-static int file_is_binary(struct diff_filespec *one)\n+static void diff_filespec_check_attr(struct diff_filespec *one)\n {\n-\tstruct git_attr_check attr_diff_check;\n+\tstruct git_attr_check attr_diff_check[1];\n \n-\tsetup_diff_attr_check(&attr_diff_check);\n-\tif (!git_checkattr(one->path, 1, &attr_diff_check)) {\n-\t\tconst char *value = attr_diff_check.value;\n+\tif (one->checked_attr)\n+\t\treturn;\n+\n+\tsetup_diff_attr_check(attr_diff_check);\n+\tone->is_binary = 0;\n+\n+\tif (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) {\n+\t\tconst char *value;\n+\n+\t\t/* binaryness */\n+\t\tvalue = attr_diff_check[0].value;\n \t\tif (ATTR_TRUE(value))\n-\t\t\treturn 0;\n+\t\t\t;\n \t\telse if (ATTR_FALSE(value))\n-\t\t\treturn 1;\n+\t\t\tone->is_binary = 1;\n \t}\n \n-\tif (!one->data) {\n-\t\tif (!DIFF_FILE_VALID(one))\n-\t\t\treturn 0;\n+\tif (!one->data && DIFF_FILE_VALID(one))\n \t\tdiff_populate_filespec(one, 0);\n-\t}\n-\treturn buffer_is_binary(one->data, one->size);\n+\n+\tif (one->data)\n+\t\tone->is_binary = buffer_is_binary(one->data, one->size);\n+\n+}\n+\n+int diff_filespec_is_binary(struct diff_filespec *one)\n+{\n+\tdiff_filespec_check_attr(one);\n+\treturn one->is_binary;\n }\n \n static void builtin_diff(const char *name_a,\n@@ -1182,7 +1197,8 @@ static void builtin_diff(const char *name_a,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (!o->text && (file_is_binary(one) || file_is_binary(two))) {\n+\tif (!o->text &&\n+\t (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {\n \t\t/* Quite common confusing case */\n \t\tif (mf1.size == mf2.size &&\n \t\t !memcmp(mf1.ptr, mf2.ptr, mf1.size))\n@@ -1260,7 +1276,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (file_is_binary(one) || file_is_binary(two)) {\n+\tif (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {\n \t\tdata->is_binary = 1;\n \t\tdata->added = mf2.size;\n \t\tdata->deleted = mf1.size;\n@@ -1302,7 +1318,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (file_is_binary(two))\n+\tif (diff_filespec_is_binary(two))\n \t\tgoto free_and_return;\n \telse {\n \t\t/* Crazy xdl interfaces.. */\n@@ -1880,8 +1896,8 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \n \t\tif (o->binary) {\n \t\t\tmmfile_t mf;\n-\t\t\tif ((!fill_mmfile(&mf, one) && file_is_binary(one)) ||\n-\t\t\t (!fill_mmfile(&mf, two) && file_is_binary(two)))\n+\t\t\tif ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||\n+\t\t\t (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))\n \t\t\t\tabbrev = 40;\n \t\t}\n \t\tlen += snprintf(msg + len, sizeof(msg) - len,\n@@ -2783,7 +2799,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\t\treturn error(\"unable to read files to diff\");\n \n \t\t/* Maybe hash p->two? into the patch id? */\n-\t\tif (file_is_binary(p->two))\n+\t\tif (diff_filespec_is_binary(p->two))\n \t\t\tcontinue;\n \n \t\tlen1 = remove_space(p->one->path, strlen(p->one->path));\n@@ -3011,21 +3027,6 @@ void diffcore_std(struct diff_options *options)\n \tif (options->quiet)\n \t\treturn;\n \n-\t/*\n-\t * break/rename count similarity differently depending on\n-\t * the binary-ness.\n-\t */\n-\tif ((options->break_opt != -1) || (options->detect_rename)) {\n-\t\tstruct diff_queue_struct *q = &diff_queued_diff;\n-\t\tint i;\n-\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tp->one->is_binary = file_is_binary(p->one);\n-\t\t\tp->two->is_binary = file_is_binary(p->two);\n-\t\t}\n-\t}\n-\n \tif (options->break_opt != -1)\n \t\tdiffcore_break(options->break_opt);\n \tif (options->detect_rename)\ndiff --git a/diffcore-delta.c b/diffcore-delta.c\nindex a038b16..d9729e5 100644\n--- a/diffcore-delta.c\n+++ b/diffcore-delta.c\n@@ -129,7 +129,7 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one)\n \tstruct spanhash_top *hash;\n \tunsigned char *buf = one->data;\n \tunsigned int sz = one->size;\n-\tint is_text = !one->is_binary;\n+\tint is_text = !diff_filespec_is_binary(one);\n \n \ti = INITIAL_HASH_SIZE;\n \thash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<mode) != 0)\n \tunsigned should_free : 1; /* data should be free()'ed */\n \tunsigned should_munmap : 1; /* data should be munmap()'ed */\n+\tunsigned checked_attr : 1;\n \tunsigned is_binary : 1; /* data should be considered \"binary\" */\n };\n \n@@ -46,6 +47,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \n extern int diff_populate_filespec(struct diff_filespec *, int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n+extern int diff_filespec_is_binary(struct diff_filespec *);\n \n struct diff_filepair {\n \tstruct diff_filespec *one;\n-- \n1.5.3.rc0.822.g38609\n"},{"id":"46606","messageId":"7v8x9uexji.fsf_-_@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"Pine.LNX.4.64.0707051223520.9789@racer.site","subject":"[PATCH] Per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-06T08:39:13Z","receivedAt":"2007-07-06T08:39:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This makes\"diff -p\" hunk headers customizable via gitattributes mechanism.\nIt is based on Johannes's earlier patch that allowed to define a single\nregexp to be used for everything.\n\nThe mechanism to arrive at the regexp that is used to define hunk header\nis the same as other use of gitattributes. You assign an attribute, funcname\n(because \"diff -p\" typically uses the name of the function the patch is about\nas the hunk header), a simple string value. This can be one of the names of\nbuilt-in pattern (currently, \"java\" is defined) or a custom pattern name, to\nbe looked up from the configuration file.\n\n (in .gitattributes)\n *.java funcname=java\n *.perl funcname=perl\n\n (in .git/config)\n [funcname]\n java = ... # ugly and complicated regexp to override the built-in one.\n perl = ... # another ugly and complicated regexp to define a new one.\n\nSigned-off-by: Junio C Hamano \n---\n\n This three patch series is lacking 2/3, as it is exactly the\n same as the earlier \"1/2 Future-proof source for changes in\n xdemitconf_t\" patch.\n\n diff.c | 96 +++++++++++++++++++++++++++++++++++++++++++++-\n diffcore.h | 1 +\n t/t4018-diff-funcname.sh | 60 ++++++++++++++++++++++++++++\n xdiff-interface.c | 71 ++++++++++++++++++++++++++++++++++\n xdiff-interface.h | 2 +\n xdiff/xdiff.h | 4 ++\n xdiff/xemit.c | 37 +++++++++++-------\n 7 files changed, 256 insertions(+), 15 deletions(-)\n create mode 100644 t/t4018-diff-funcname.sh\n\ndiff --git a/diff.c b/diff.c\nindex d10e848..04e7e91 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1101,22 +1101,26 @@ static void emit_binary_diff(mmfile_t *one, mmfile_t *two)\n static void setup_diff_attr_check(struct git_attr_check *check)\n {\n \tstatic struct git_attr *attr_diff;\n+\tstatic struct git_attr *attr_diff_func_name;\n \n \tif (!attr_diff) {\n \t\tattr_diff = git_attr(\"diff\", 4);\n+\t\tattr_diff_func_name = git_attr(\"funcname\", 8);\n \t}\n \tcheck[0].attr = attr_diff;\n+\tcheck[1].attr = attr_diff_func_name;\n }\n \n static void diff_filespec_check_attr(struct diff_filespec *one)\n {\n-\tstruct git_attr_check attr_diff_check[1];\n+\tstruct git_attr_check attr_diff_check[2];\n \n \tif (one->checked_attr)\n \t\treturn;\n \n \tsetup_diff_attr_check(attr_diff_check);\n \tone->is_binary = 0;\n+\tone->hunk_header_ident = NULL;\n \n \tif (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) {\n \t\tconst char *value;\n@@ -1127,6 +1131,13 @@ static void diff_filespec_check_attr(struct diff_filespec *one)\n \t\t\t;\n \t\telse if (ATTR_FALSE(value))\n \t\t\tone->is_binary = 1;\n+\n+\t\t/* hunk header ident */\n+\t\tvalue = attr_diff_check[1].value;\n+\t\tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))\n+\t\t\t;\n+\t\telse\n+\t\t\tone->hunk_header_ident = value;\n \t}\n \n \tif (!one->data && DIFF_FILE_VALID(one))\n@@ -1143,6 +1154,82 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n \treturn one->is_binary;\n }\n \n+static struct hunk_header_regexp {\n+\tchar *name;\n+\tchar *regexp;\n+\tstruct hunk_header_regexp *next;\n+} *hunk_header_regexp_list, **hunk_header_regexp_tail;\n+\n+static int hunk_header_config(const char *var, const char *value)\n+{\n+\tstatic const char funcname[] = \"funcname.\";\n+\tstruct hunk_header_regexp *hh;\n+\n+\tif (prefixcmp(var, funcname))\n+\t\treturn 0;\n+\tvar += strlen(funcname);\n+\tfor (hh = hunk_header_regexp_list; hh; hh = hh->next)\n+\t\tif (!strcmp(var, hh->name)) {\n+\t\t\tfree(hh->regexp);\n+\t\t\thh->regexp = xstrdup(value);\n+\t\t\treturn 0;\n+\t\t}\n+\thh = xcalloc(1, sizeof(*hh));\n+\thh->name = xstrdup(var);\n+\thh->regexp = xstrdup(value);\n+\thh->next = NULL;\n+\t*hunk_header_regexp_tail = hh;\n+\treturn 0;\n+}\n+\n+static const char *hunk_header_regexp(const char *ident)\n+{\n+\tstruct hunk_header_regexp *hh;\n+\n+\tif (!hunk_header_regexp_tail) {\n+\t\thunk_header_regexp_tail = &hunk_header_regexp_list;\n+\t\tgit_config(hunk_header_config);\n+\t}\n+\tfor (hh = hunk_header_regexp_list; hh; hh = hh->next)\n+\t\tif (!strcmp(ident, hh->name))\n+\t\t\treturn hh->regexp;\n+\treturn NULL;\n+}\n+\n+static const char *diff_hunk_header_regexp(struct diff_filespec *one)\n+{\n+\tconst char *ident, *regexp;\n+\n+\tdiff_filespec_check_attr(one);\n+\tident = one->hunk_header_ident;\n+\n+\tif (!ident)\n+\t\t/*\n+\t\t * If the config file has \"funcname.default\" defined, that\n+\t\t * regexp is used; otherwise NULL is returned and xemit uses\n+\t\t * the built-in default.\n+\t\t */\n+\t\treturn hunk_header_regexp(\"default\");\n+\n+\t/* Look up custom \"funcname.$ident\" regexp from config. */\n+\tregexp = hunk_header_regexp(ident);\n+\tif (regexp)\n+\t\treturn regexp;\n+\n+\t/*\n+\t * And define built-in fallback patterns here. Note that\n+\t * these can be overriden by the user's config settings.\n+\t */\n+\tif (!strcmp(ident, \"java\"))\n+\t\treturn \"!^[ \t]*\\\\(catch\\\\|do\\\\|for\\\\|if\\\\|instanceof\\\\|\"\n+\t\t\t\"new\\\\|return\\\\|switch\\\\|throw\\\\|while\\\\)\\n\"\n+\t\t\t\"^[ \t]*\\\\(\\\\([ \t]*\"\n+\t\t\t\"[A-Za-z_][A-Za-z_0-9]*\\\\)\\\\{2,\\\\}\"\n+\t\t\t\"[ \t]*([^;]*$\\\\)\";\n+\n+\treturn NULL;\n+}\n+\n static void builtin_diff(const char *name_a,\n \t\t\t const char *name_b,\n \t\t\t struct diff_filespec *one,\n@@ -1217,6 +1304,11 @@ static void builtin_diff(const char *name_a,\n \t\txdemitconf_t xecfg;\n \t\txdemitcb_t ecb;\n \t\tstruct emit_callback ecbdata;\n+\t\tconst char *hunk_header_regexp;\n+\n+\t\thunk_header_regexp = diff_hunk_header_regexp(one);\n+\t\tif (!hunk_header_regexp)\n+\t\t\thunk_header_regexp = diff_hunk_header_regexp(two);\n \n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\tmemset(&ecbdata, 0, sizeof(ecbdata));\n@@ -1226,6 +1318,8 @@ static void builtin_diff(const char *name_a,\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\n \t\txecfg.flags = XDL_EMIT_FUNCNAMES;\n+\t\tif (hunk_header_regexp)\n+\t\t\txdiff_set_find_func(&xecfg, hunk_header_regexp);\n \t\tif (!diffopts)\n \t\t\t;\n \t\telse if (!prefixcmp(diffopts, \"--unified=\"))\ndiff --git a/diffcore.h b/diffcore.h\nindex dcab7e2..0598514 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -27,6 +27,7 @@ struct diff_filespec {\n \tchar *path;\n \tvoid *data;\n \tvoid *cnt_data;\n+\tconst void *hunk_header_ident;\n \tunsigned long size;\n \tint xfrm_flags;\t\t /* for use by the xfrm */\n \tunsigned short mode;\t /* file mode */\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nnew file mode 100644\nindex 0000000..dc7a47b\n--- /dev/null\n+++ b/t/t4018-diff-funcname.sh\n@@ -0,0 +1,60 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Johannes E. Schindelin\n+#\n+\n+test_description='Test custom diff function name patterns'\n+\n+. ./test-lib.sh\n+\n+LF='\n+'\n+\n+cat > Beer.java << EOF\n+public class Beer\n+{\n+\tint special;\n+\tpublic static void main(String args[])\n+\t{\n+\t\tString s=\" \";\n+\t\tfor(int x = 99; x > 0; x--)\n+\t\t{\n+\t\t\tSystem.out.print(x + \" bottles of beer on the wall \"\n+\t\t\t\t+ x + \" bottles of beer\\n\"\n+\t\t\t\t+ \"Take one down, pass it around, \" + (x - 1)\n+\t\t\t\t+ \" bottles of beer on the wall.\\n\");\n+\t\t}\n+\t\tSystem.out.print(\"Go to the store, buy some more,\\n\"\n+\t\t\t+ \"99 bottles of beer on the wall.\\n\");\n+\t}\n+}\n+EOF\n+\n+sed 's/beer\\\\/beer,\\\\/' < Beer.java > Beer-correct.java\n+\n+test_expect_success 'default behaviour' '\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ public class Beer\"\n+'\n+\n+test_expect_success 'preset java pattern' '\n+\techo \"*.java funcname=java\" >.gitattributes &&\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ public static void main(\"\n+'\n+\n+git config funcname.java '!static\n+!String\n+[^ \t].*s.*'\n+\n+test_expect_success 'custom pattern' '\n+\tgit diff Beer.java Beer-correct.java |\n+\tgrep \"^@@.*@@ int special;$\"\n+'\n+\n+test_expect_success 'last regexp must not be negated' '\n+\tgit config diff.functionnameregexp \"!static\" &&\n+\t! git diff Beer.java Beer-correct.java\n+'\n+\n+test_done\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex e407cf1..be866d1 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -129,3 +129,74 @@ int buffer_is_binary(const char *ptr, unsigned long size)\n \t\tsize = FIRST_FEW_BYTES;\n \treturn !!memchr(ptr, 0, size);\n }\n+\n+struct ff_regs {\n+\tint nr;\n+\tstruct ff_reg {\n+\t\tregex_t re;\n+\t\tint negate;\n+\t} *array;\n+};\n+\n+static long ff_regexp(const char *line, long len,\n+\t\tchar *buffer, long buffer_size, void *priv)\n+{\n+\tchar *line_buffer = xstrndup(line, len); /* make NUL terminated */\n+\tstruct ff_regs *regs = priv;\n+\tregmatch_t pmatch[2];\n+\tint result = 0, i;\n+\n+\tfor (i = 0; i < regs->nr; i++) {\n+\t\tstruct ff_reg *reg = regs->array + i;\n+\t\tif (reg->negate ^ !!regexec(®->re,\n+\t\t\t\t\tline_buffer, 2, pmatch, 0)) {\n+\t\t\tfree(line_buffer);\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\ti = pmatch[1].rm_so >= 0 ? 1 : 0;\n+\tline += pmatch[i].rm_so;\n+\tresult = pmatch[i].rm_eo - pmatch[i].rm_so;\n+\tif (result > buffer_size)\n+\t\tresult = buffer_size;\n+\telse\n+\t\twhile (result > 0 && (isspace(line[result - 1]) ||\n+\t\t\t\t\tline[result - 1] == '\\n'))\n+\t\t\tresult--;\n+\tmemcpy(buffer, line, result);\n+\tfree(line_buffer);\n+\treturn result;\n+}\n+\n+void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)\n+{\n+\tint i;\n+\tstruct ff_regs *regs;\n+\n+\txecfg->find_func = ff_regexp;\n+\tregs = xecfg->find_func_priv = xmalloc(sizeof(struct ff_regs));\n+\tfor (i = 0, regs->nr = 1; value[i]; i++)\n+\t\tif (value[i] == '\\n')\n+\t\t\tregs->nr++;\n+\tregs->array = xmalloc(regs->nr * sizeof(struct ff_reg));\n+\tfor (i = 0; i < regs->nr; i++) {\n+\t\tstruct ff_reg *reg = regs->array + i;\n+\t\tconst char *ep = strchr(value, '\\n'), *expression;\n+\t\tchar *buffer = NULL;\n+\n+\t\treg->negate = (*value == '!');\n+\t\tif (reg->negate && i == regs->nr - 1)\n+\t\t\tdie(\"Last expression must not be negated: %s\", value);\n+\t\tif (*value == '!')\n+\t\t\tvalue++;\n+\t\tif (ep)\n+\t\t\texpression = buffer = xstrndup(value, ep - value);\n+\t\telse\n+\t\t\texpression = value;\n+\t\tif (regcomp(®->re, expression, 0))\n+\t\t\tdie(\"Invalid regexp to look for hunk header: %s\", expression);\n+\t\tif (buffer)\n+\t\t\tfree(buffer);\n+\t\tvalue = ep + 1;\n+\t}\n+}\ndiff --git a/xdiff-interface.h b/xdiff-interface.h\nindex 536f4e4..fb742db 100644\n--- a/xdiff-interface.h\n+++ b/xdiff-interface.h\n@@ -20,4 +20,6 @@ int parse_hunk_header(char *line, int len,\n int read_mmfile(mmfile_t *ptr, const char *filename);\n int buffer_is_binary(const char *ptr, unsigned long size);\n \n+extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line);\n+\n #endif\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 9402bb0..c00ddaa 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -73,9 +73,13 @@ typedef struct s_xdemitcb {\n \tint (*outf)(void *, mmbuffer_t *, int);\n } xdemitcb_t;\n \n+typedef long (*find_func_t)(const char *line, long line_len, char *buffer, long buffer_size, void *priv);\n+\n typedef struct s_xdemitconf {\n \tlong ctxlen;\n \tunsigned long flags;\n+\tfind_func_t find_func;\n+\tvoid *find_func_priv;\n } xdemitconf_t;\n \n typedef struct s_bdiffparam {\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex 4b6e639..d3d9c84 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -69,7 +69,24 @@ static xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) {\n }\n \n \n-static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll) {\n+static long def_ff(const char *rec, long len, char *buf, long sz, void *priv)\n+{\n+\tif (len > 0 &&\n+\t\t\t(isalpha((unsigned char)*rec) || /* identifier? */\n+\t\t\t *rec == '_' ||\t/* also identifier? */\n+\t\t\t *rec == '$')) { /* identifiers from VMS and other esoterico */\n+\t\tif (len > sz)\n+\t\t\tlen = sz;\n+\t\twhile (0 < len && isspace((unsigned char)rec[len - 1]))\n+\t\t\tlen--;\n+\t\tmemcpy(buf, rec, len);\n+\t\treturn len;\n+\t}\n+\treturn -1;\n+}\n+\n+static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll,\n+\t\tfind_func_t ff, void *ff_priv) {\n \n \t/*\n \t * Be quite stupid about this for now. Find a line in the old file\n@@ -80,22 +97,12 @@ static void xdl_find_func(xdfile_t *xf, long i, char *buf, long sz, long *ll) {\n \tconst char *rec;\n \tlong len;\n \n-\t*ll = 0;\n \twhile (i-- > 0) {\n \t\tlen = xdl_get_rec(xf, i, &rec);\n-\t\tif (len > 0 &&\n-\t\t (isalpha((unsigned char)*rec) || /* identifier? */\n-\t\t *rec == '_' ||\t/* also identifier? */\n-\t\t *rec == '$')) {\t/* mysterious GNU diff's invention */\n-\t\t\tif (len > sz)\n-\t\t\t\tlen = sz;\n-\t\t\twhile (0 < len && isspace((unsigned char)rec[len - 1]))\n-\t\t\t\tlen--;\n-\t\t\tmemcpy(buf, rec, len);\n-\t\t\t*ll = len;\n+\t\tif ((*ll = ff(rec, len, buf, sz, ff_priv)) >= 0)\n \t\t\treturn;\n-\t\t}\n \t}\n+\t*ll = 0;\n }\n \n \n@@ -120,6 +127,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \txdchange_t *xch, *xche;\n \tchar funcbuf[80];\n \tlong funclen = 0;\n+\tfind_func_t ff = xecfg->find_func ? xecfg->find_func : def_ff;\n \n \tif (xecfg->flags & XDL_EMIT_COMMON)\n \t\treturn xdl_emit_common(xe, xscr, ecb, xecfg);\n@@ -143,7 +151,8 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \n \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n \t\t\txdl_find_func(&xe->xdf1, s1, funcbuf,\n-\t\t\t\t sizeof(funcbuf), &funclen);\n+\t\t\t\t sizeof(funcbuf), &funclen,\n+\t\t\t\t ff, xecfg->find_func_priv);\n \t\t}\n \t\tif (xdl_emit_hunk_hdr(s1 + 1, e1 - s1, s2 + 1, e2 - s2,\n \t\t\t\t funcbuf, funclen, ecb) < 0)\n-- \n1.5.3.rc0.822.g38609\n"},{"id":"46617","messageId":"Pine.LNX.4.64.0707061320180.4093@racer.site","threadId":"8866","inReplyTo":"7vejjmexm0.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Introduce diff_filespec_is_binary()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-06T12:20:47Z","receivedAt":"2007-07-06T12:20:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Jul 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > It is quite cute to hide this in the funcname patch...\n> \n> So this is the proper refactoring _before_ any of your topics.\n\nThanks. And sorry for bugging you.\n\nCiao,\nDscho\n"},{"id":"46619","messageId":"Pine.LNX.4.64.0707061320510.4093@racer.site","threadId":"8866","inReplyTo":"7v8x9uexji.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-06T12:38:42Z","receivedAt":"2007-07-06T12:38:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Jul 2007, Junio C Hamano wrote:\n\n> diff --git a/diff.c b/diff.c\n> @@ -1143,6 +1154,82 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n> \treturn one->is_binary;\n> }\n> \n> +static struct hunk_header_regexp {\n> +\tchar *name;\n> +\tchar *regexp;\n> +\tstruct hunk_header_regexp *next;\n> +} *hunk_header_regexp_list, **hunk_header_regexp_tail;\n> +\n> +static int hunk_header_config(const char *var, const char *value)\n> +{\n> +\tstatic const char funcname[] = \"funcname.\";\n> +\tstruct hunk_header_regexp *hh;\n> +\n> +\tif (prefixcmp(var, funcname))\n> +\t\treturn 0;\n> +\tvar += strlen(funcname);\n> +\tfor (hh = hunk_header_regexp_list; hh; hh = hh->next)\n> +\t\tif (!strcmp(var, hh->name)) {\n> +\t\t\tfree(hh->regexp);\n> +\t\t\thh->regexp = xstrdup(value);\n> +\t\t\treturn 0;\n> +\t\t}\n\nHeh. By reordering your code, you could say\n\n\tif ((hh = hunk_header_regexp(var))) {\n\t\tfree(hh->regexp);\n\t\thh->regexp = xstrdup(value);\n\t\treturn 0;\n\t}\n\n> +\thh = xcalloc(1, sizeof(*hh));\n> +\thh->name = xstrdup(var);\n> +\thh->regexp = xstrdup(value);\n> +\thh->next = NULL;\n> +\t*hunk_header_regexp_tail = hh;\n> +\treturn 0;\n> +}\n\nIs that tail expansion not overly complex? Why not just set \"hh->next = \nhunk_header_regexp_list; hunk_header_regexp_list = hh\";\n\nYes, your code seems correct, but I took some extra cycles to get at that \nimpression. A \"static int parsed_config_for_hunk_headers\" would have \nhelped, instead of reusing _tail for two purposes. And this variable could \nbe set at the beginning of hunk_header_config(), so that \nhunk_header_regexp() is usable from inside hunk_header_config().\n\n> +static const char *hunk_header_regexp(const char *ident)\n> +{\n> +\tstruct hunk_header_regexp *hh;\n> +\n> +\tif (!hunk_header_regexp_tail) {\n> +\t\thunk_header_regexp_tail = &hunk_header_regexp_list;\n> +\t\tgit_config(hunk_header_config);\n> +\t}\n> +\tfor (hh = hunk_header_regexp_list; hh; hh = hh->next)\n> +\t\tif (!strcmp(ident, hh->name))\n> +\t\t\treturn hh->regexp;\n> +\treturn NULL;\n> +}\n\nAnother thing. These long names are a bit inconsistent. In the config, you \nname it \"funcname\". In xdiff, we name them \"FUNCNAMES\". Yes, here they are \nhunk_headers.\n\nAlso, since the expressions are not strictly regular expressions, but \nlists of them, and with your idea they are even more different, why not \njust go for \"funcname_list\"? It's easier to read, and static anyway.\n\nRest looks fine to me...\n\nCiao,\nDscho\n"},{"id":"46650","messageId":"alpine.LFD.0.98.0707061051020.9434@woody.linux-foundation.org","threadId":"8866","inReplyTo":"7v8x9uexji.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-07-06T17:59:20Z","receivedAt":"2007-07-06T17:59:20Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 6 Jul 2007, Junio C Hamano wrote:\n>\n> This makes\"diff -p\" hunk headers customizable via gitattributes mechanism.\n> It is based on Johannes's earlier patch that allowed to define a single\n> regexp to be used for everything.\n\nOk, I think this is really nice, but I do wonder a bit about the syntax.\n\nIn particular, the \"funcname\" thing is really a pretty ugly special-case \napproach.\n\nWouldn't it be nicer to consider the \"funcname=java\" to be less of a \n\"special case for the built-in diff\", and instead think of it as a more \ngeneric issue of \"how do we want to generate diffs for java files?\"\n\nIOW, wouldn't this be much nicer to be thought about as a \"custon diff \ndriver\" issue?\n\nSo I like your patches, but dislike the config syntax, and would suggest \nsomething like\n\nIn .gitattributes:\n\n\t*.java diff=java\n\t*.perl diff=perl\n\t*.doc diff=doc\n\nIn .git/config\n\n\t[diff \"java\"]\n\t\tcommand = internal\n\t\tfuncname = ... # ugly and complicated regexp to override the built-in one.\n\n\t[diff \"perl\"]\n\t\tcommand = internal\n\t\tfuncname = ...\n\n\t[diff \"doc\"]\n\t\tcommand = ms-doc-diff\n\nDoesn't this make more sense and mesh much better with the already \nexisting custom diff driver?\n\n(And yeah, maybe we could instead of \"command=internal\" just have the rule \nthat \"internal\" is the default, and you'd not have a command at all when \nyou want to run the internal diff.\n\nJust an idea. I don't have any code.\n\n\t\t\tLinus\n"},{"id":"46670","messageId":"7v8x9tdlbv.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"alpine.LFD.0.98.0707061051020.9434@woody.linux-foundation.org","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-07T02:00:36Z","receivedAt":"2007-07-07T02:00:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> In .gitattributes:\n>\n> \t*.java diff=java\n> \t*.perl diff=perl\n> \t*.doc diff=doc\n>\n> In .git/config\n>\n> \t[diff \"java\"]\n> \t\tcommand = internal\n> \t\tfuncname = ... # ugly and complicated regexp to override the built-in one.\n>\n> \t[diff \"perl\"]\n> \t\tcommand = internal\n> \t\tfuncname = ...\n>\n> \t[diff \"doc\"]\n> \t\tcommand = ms-doc-diff\n>\n> Doesn't this make more sense and mesh much better with the already \n> existing custom diff driver?\n>\n> (And yeah, maybe we could instead of \"command=internal\" just have the rule \n> that \"internal\" is the default, and you'd not have a command at all when \n> you want to run the internal diff.\n>\n> Just an idea. I don't have any code.\n\nYeah, I'd be lying if I said that this did not cross my mind\nwhen I saw existing diff.*.command handling.\n\nAbout the comment from Johannes regarding hunk_header vs\nfuncname, I would actually prefer hunk_header, since that is\nwhat this is about (\"funcname\" and \"find_func\" were misnomer\nfrom the beginning), but I'd rename hunk_header to funcname for\nthe sake of consistency and minimizing the diff.\n\nWill find time to look at this over the weekend.\n"},{"id":"46681","messageId":"200707071011.l67AB9rg005792@mi0.bluebottle.com","threadId":"8866","inReplyTo":"7v8x9tdlbv.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"しらいしななこ","fromEmail":"nanako3@bluebottle.com","sentAt":"2007-07-07T10:08:25Z","receivedAt":"2007-07-07T10:08:25Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano :\n\n> Yeah, I'd be lying if I said that this did not cross my mind\n> when I saw existing diff.*.command handling.\n>\n> About the comment from Johannes regarding hunk_header vs\n> funcname, I would actually prefer hunk_header, since that is\n> what this is about (\"funcname\" and \"find_func\" were misnomer\n> from the beginning), but I'd rename hunk_header to funcname for\n> the sake of consistency and minimizing the diff.\n\nI would love to see \"diff=tex\" attribute to work on my manuscripts, but I\ndo not write C and do not understand the long length of backslashes very\nwell. I guessed in the source file a backslash needs to be doubled, and\nwhat I want to match is \"\\section{\", \"\\subsection{\", and \"\\subsubsection{\"\nat the beginning of lines, and attempted to do it like the patch at the\nend.\n\nIt does not work well, however. It shows only part of lines.\n\n\t@@ -8,7 +8,8 @@ \\section{\n\t@@ -224,7 +225,7 @@ sub\n\t@@ -240,7 +241,7 @@ subsub\n\nI have no idea what am I doing wrong (truthfully, I do not know what I am\ndoing, period).\n\n\ndiff --git a/diff.c b/diff.c\nindex 04e7e91..57f91b0 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1226,6 +1226,8 @@ static const char *diff_hunk_header_regexp(struct diff_filespec *one)\n \t\t\t\"^[ \t]*\\\\(\\\\([ \t]*\"\n \t\t\t\"[A-Za-z_][A-Za-z_0-9]*\\\\)\\\\{2,\\\\}\"\n \t\t\t\"[ \t]*([^;]*$\\\\)\";\n+\tif (!strcmp(ident, \"tex\"))\n+\t\treturn \"^\\\\\\\\\\\\(sub\\\\)*section{\";\n \n \treturn NULL;\n }\n\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n\n----------------------------------------------------------------------\nFinally - A spam blocker that actually works.\nhttp://www.bluebottle.com/tag/4\n"},{"id":"46682","messageId":"7vhcog8qv4.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"alpine.LFD.0.98.0707061051020.9434@woody.linux-foundation.org","subject":"[PATCH] Fix configuration syntax to specify customized hunk header patterns.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-07T10:12:15Z","receivedAt":"2007-07-07T10:12:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This updates the hunk header customization syntax. The special\ncase 'funcname' attribute is gone. It also uses the name\n'funcname\" throughout, instead of \"hunk_header\".\n\nYou assign the name of the type of contents to path's \"diff\"\nattribute as a string value in .gitattributes like this:\n\n\t*.java diff=java\n\t*.perl diff=perl\n\t*.doc diff=doc\n\nIf you supply \"diff..funcname\" variable via the\nconfiguration mechanism (e.g. in $HOME/.gitconfig), the value is\nused as the regexp set to find the line to use for the hunk\nheader (the variable is called \"funcname\" because such a line\ntypically is the one that has the name of the function in\nprogramming language source text).\n\nIf there is no such configuration, built-in default is used, if\nany. Currently there are two default patterns: default and java.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 147 +++++++++++++++++++++++++---------------------\n diffcore.h | 2 +-\n t/t4018-diff-funcname.sh | 6 +-\n 3 files changed, 84 insertions(+), 71 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 04e7e91..21e61af 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -56,6 +56,14 @@ static struct ll_diff_driver {\n \tchar *cmd;\n } *user_diff, **user_diff_tail;\n \n+static void read_config_if_needed(void)\n+{\n+\tif (!user_diff_tail) {\n+\t\tuser_diff_tail = &user_diff;\n+\t\tgit_config(git_diff_ui_config);\n+\t}\n+}\n+\n /*\n * Currently there is only \"diff..command\" variable;\n * because there are \"diff.color.\" variables, we are parsing\n@@ -94,6 +102,45 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val\n }\n \n /*\n+ * 'diff..funcname' attribute can be specified in the configuration\n+ * to define a customized regexp to find the beginning of a function to\n+ * be used for hunk header lines of \"diff -p\" style output.\n+ */\n+static struct funcname_pattern {\n+\tchar *name;\n+\tchar *pattern;\n+\tstruct funcname_pattern *next;\n+} *funcname_pattern_list;\n+\n+static int parse_funcname_pattern(const char *var, const char *ep, const char *value)\n+{\n+\tconst char *name;\n+\tint namelen;\n+\tstruct funcname_pattern *pp;\n+\n+\tname = var + 5; /* \"diff.\" */\n+\tnamelen = ep - name;\n+\n+\tfor (pp = funcname_pattern_list; pp; pp = pp->next)\n+\t\tif (!strncmp(pp->name, name, namelen) && !pp->name[namelen])\n+\t\t\tbreak;\n+\tif (!pp) {\n+\t\tchar *namebuf;\n+\t\tpp = xcalloc(1, sizeof(*pp));\n+\t\tnamebuf = xmalloc(namelen + 1);\n+\t\tmemcpy(namebuf, name, namelen);\n+\t\tnamebuf[namelen] = 0;\n+\t\tpp->name = namebuf;\n+\t\tpp->next = funcname_pattern_list;\n+\t\tfuncname_pattern_list = pp;\n+\t}\n+\tif (pp->pattern)\n+\t\tfree(pp->pattern);\n+\tpp->pattern = xstrdup(value);\n+\treturn 0;\n+}\n+\n+/*\n * These are to give UI layer defaults.\n * The core-level commands such as git-diff-files should\n * never be affected by the setting of diff.renames\n@@ -122,8 +169,12 @@ int git_diff_ui_config(const char *var, const char *value)\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \n-\t\tif (ep != var + 4 && !strcmp(ep, \".command\"))\n-\t\t\treturn parse_lldiff_command(var, ep, value);\n+\t\tif (ep != var + 4) {\n+\t\t\tif (!strcmp(ep, \".command\"))\n+\t\t\t\treturn parse_lldiff_command(var, ep, value);\n+\t\t\tif (!strcmp(ep, \".funcname\"))\n+\t\t\t\treturn parse_funcname_pattern(var, ep, value);\n+\t\t}\n \t}\n \tif (!prefixcmp(var, \"diff.color.\") || !prefixcmp(var, \"color.diff.\")) {\n \t\tint slot = parse_diff_color_slot(var, 11);\n@@ -1101,43 +1152,39 @@ static void emit_binary_diff(mmfile_t *one, mmfile_t *two)\n static void setup_diff_attr_check(struct git_attr_check *check)\n {\n \tstatic struct git_attr *attr_diff;\n-\tstatic struct git_attr *attr_diff_func_name;\n \n \tif (!attr_diff) {\n \t\tattr_diff = git_attr(\"diff\", 4);\n-\t\tattr_diff_func_name = git_attr(\"funcname\", 8);\n \t}\n \tcheck[0].attr = attr_diff;\n-\tcheck[1].attr = attr_diff_func_name;\n }\n \n static void diff_filespec_check_attr(struct diff_filespec *one)\n {\n-\tstruct git_attr_check attr_diff_check[2];\n+\tstruct git_attr_check attr_diff_check;\n \n \tif (one->checked_attr)\n \t\treturn;\n \n-\tsetup_diff_attr_check(attr_diff_check);\n+\tsetup_diff_attr_check(&attr_diff_check);\n \tone->is_binary = 0;\n-\tone->hunk_header_ident = NULL;\n+\tone->funcname_pattern_ident = NULL;\n \n-\tif (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) {\n+\tif (!git_checkattr(one->path, 1, &attr_diff_check)) {\n \t\tconst char *value;\n \n \t\t/* binaryness */\n-\t\tvalue = attr_diff_check[0].value;\n+\t\tvalue = attr_diff_check.value;\n \t\tif (ATTR_TRUE(value))\n \t\t\t;\n \t\telse if (ATTR_FALSE(value))\n \t\t\tone->is_binary = 1;\n \n-\t\t/* hunk header ident */\n-\t\tvalue = attr_diff_check[1].value;\n+\t\t/* funcname pattern ident */\n \t\tif (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))\n \t\t\t;\n \t\telse\n-\t\t\tone->hunk_header_ident = value;\n+\t\t\tone->funcname_pattern_ident = value;\n \t}\n \n \tif (!one->data && DIFF_FILE_VALID(one))\n@@ -1154,54 +1201,23 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n \treturn one->is_binary;\n }\n \n-static struct hunk_header_regexp {\n-\tchar *name;\n-\tchar *regexp;\n-\tstruct hunk_header_regexp *next;\n-} *hunk_header_regexp_list, **hunk_header_regexp_tail;\n-\n-static int hunk_header_config(const char *var, const char *value)\n+static const char *funcname_pattern(const char *ident)\n {\n-\tstatic const char funcname[] = \"funcname.\";\n-\tstruct hunk_header_regexp *hh;\n+\tstruct funcname_pattern *pp;\n \n-\tif (prefixcmp(var, funcname))\n-\t\treturn 0;\n-\tvar += strlen(funcname);\n-\tfor (hh = hunk_header_regexp_list; hh; hh = hh->next)\n-\t\tif (!strcmp(var, hh->name)) {\n-\t\t\tfree(hh->regexp);\n-\t\t\thh->regexp = xstrdup(value);\n-\t\t\treturn 0;\n-\t\t}\n-\thh = xcalloc(1, sizeof(*hh));\n-\thh->name = xstrdup(var);\n-\thh->regexp = xstrdup(value);\n-\thh->next = NULL;\n-\t*hunk_header_regexp_tail = hh;\n-\treturn 0;\n-}\n-\n-static const char *hunk_header_regexp(const char *ident)\n-{\n-\tstruct hunk_header_regexp *hh;\n-\n-\tif (!hunk_header_regexp_tail) {\n-\t\thunk_header_regexp_tail = &hunk_header_regexp_list;\n-\t\tgit_config(hunk_header_config);\n-\t}\n-\tfor (hh = hunk_header_regexp_list; hh; hh = hh->next)\n-\t\tif (!strcmp(ident, hh->name))\n-\t\t\treturn hh->regexp;\n+\tread_config_if_needed();\n+\tfor (pp = funcname_pattern_list; pp; pp = pp->next)\n+\t\tif (!strcmp(ident, pp->name))\n+\t\t\treturn pp->pattern;\n \treturn NULL;\n }\n \n-static const char *diff_hunk_header_regexp(struct diff_filespec *one)\n+static const char *diff_funcname_pattern(struct diff_filespec *one)\n {\n-\tconst char *ident, *regexp;\n+\tconst char *ident, *pattern;\n \n \tdiff_filespec_check_attr(one);\n-\tident = one->hunk_header_ident;\n+\tident = one->funcname_pattern_ident;\n \n \tif (!ident)\n \t\t/*\n@@ -1209,12 +1225,12 @@ static const char *diff_hunk_header_regexp(struct diff_filespec *one)\n \t\t * regexp is used; otherwise NULL is returned and xemit uses\n \t\t * the built-in default.\n \t\t */\n-\t\treturn hunk_header_regexp(\"default\");\n+\t\treturn funcname_pattern(\"default\");\n \n \t/* Look up custom \"funcname.$ident\" regexp from config. */\n-\tregexp = hunk_header_regexp(ident);\n-\tif (regexp)\n-\t\treturn regexp;\n+\tpattern = funcname_pattern(ident);\n+\tif (pattern)\n+\t\treturn pattern;\n \n \t/*\n \t * And define built-in fallback patterns here. Note that\n@@ -1304,11 +1320,11 @@ static void builtin_diff(const char *name_a,\n \t\txdemitconf_t xecfg;\n \t\txdemitcb_t ecb;\n \t\tstruct emit_callback ecbdata;\n-\t\tconst char *hunk_header_regexp;\n+\t\tconst char *funcname_pattern;\n \n-\t\thunk_header_regexp = diff_hunk_header_regexp(one);\n-\t\tif (!hunk_header_regexp)\n-\t\t\thunk_header_regexp = diff_hunk_header_regexp(two);\n+\t\tfuncname_pattern = diff_funcname_pattern(one);\n+\t\tif (!funcname_pattern)\n+\t\t\tfuncname_pattern = diff_funcname_pattern(two);\n \n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\tmemset(&ecbdata, 0, sizeof(ecbdata));\n@@ -1318,8 +1334,8 @@ static void builtin_diff(const char *name_a,\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\n \t\txecfg.flags = XDL_EMIT_FUNCNAMES;\n-\t\tif (hunk_header_regexp)\n-\t\t\txdiff_set_find_func(&xecfg, hunk_header_regexp);\n+\t\tif (funcname_pattern)\n+\t\t\txdiff_set_find_func(&xecfg, funcname_pattern);\n \t\tif (!diffopts)\n \t\t\t;\n \t\telse if (!prefixcmp(diffopts, \"--unified=\"))\n@@ -1862,10 +1878,7 @@ static const char *external_diff_attr(const char *name)\n \t\t !ATTR_UNSET(value)) {\n \t\t\tstruct ll_diff_driver *drv;\n \n-\t\t\tif (!user_diff_tail) {\n-\t\t\t\tuser_diff_tail = &user_diff;\n-\t\t\t\tgit_config(git_diff_ui_config);\n-\t\t\t}\n+\t\t\tread_config_if_needed();\n \t\t\tfor (drv = user_diff; drv; drv = drv->next)\n \t\t\t\tif (!strcmp(drv->name, value))\n \t\t\t\t\treturn drv->cmd;\ndiff --git a/diffcore.h b/diffcore.h\nindex 0598514..eef17c4 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -27,7 +27,7 @@ struct diff_filespec {\n \tchar *path;\n \tvoid *data;\n \tvoid *cnt_data;\n-\tconst void *hunk_header_ident;\n+\tconst char *funcname_pattern_ident;\n \tunsigned long size;\n \tint xfrm_flags;\t\t /* for use by the xfrm */\n \tunsigned short mode;\t /* file mode */\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex dc7a47b..f9db81d 100644\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -38,12 +38,12 @@ test_expect_success 'default behaviour' '\n '\n \n test_expect_success 'preset java pattern' '\n-\techo \"*.java funcname=java\" >.gitattributes &&\n+\techo \"*.java diff=java\" >.gitattributes &&\n \tgit diff Beer.java Beer-correct.java |\n \tgrep \"^@@.*@@ public static void main(\"\n '\n \n-git config funcname.java '!static\n+git config diff.java.funcname '!static\n !String\n [^ \t].*s.*'\n \n@@ -53,7 +53,7 @@ test_expect_success 'custom pattern' '\n '\n \n test_expect_success 'last regexp must not be negated' '\n-\tgit config diff.functionnameregexp \"!static\" &&\n+\tgit config diff.java.funcname \"!static\" &&\n \t! git diff Beer.java Beer-correct.java\n '\n \n-- \n1.5.3.rc0.837.g4574c\n"},{"id":"46684","messageId":"7vabu88qem.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"200707071011.l67AB9rg005792@mi0.bluebottle.com","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-07T10:22:09Z","receivedAt":"2007-07-07T10:22:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"しらいしななこ writes:\n\n> I would love to see \"diff=tex\" attribute to work on my manuscripts, but I\n> do not write C and do not understand the long length of backslashes very\n> well. I guessed in the source file a backslash needs to be doubled, and\n> what I want to match is \"\\section{\", \"\\subsection{\", and \"\\subsubsection{\"\n> at the beginning of lines, and attempted to do it like the patch at the\n> end.\n\nHeh, I do not speak TeX very well, so we are even ;-)\n\n> It does not work well, however. It shows only part of lines.\n>\n> \t@@ -8,7 +8,8 @@ \\section{\n> \t@@ -224,7 +225,7 @@ sub\n> \t@@ -240,7 +241,7 @@ subsub\n>\n> I have no idea what am I doing wrong (truthfully, I do not know what I am\n> doing, period).\n>\n> diff --git a/diff.c b/diff.c\n> index 04e7e91..57f91b0 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1226,6 +1226,8 @@ static const char *diff_hunk_header_regexp(struct diff_filespec *one)\n> \t\t\t\"^[ \t]*\\\\(\\\\([ \t]*\"\n> \t\t\t\"[A-Za-z_][A-Za-z_0-9]*\\\\)\\\\{2,\\\\}\"\n> \t\t\t\"[ \t]*([^;]*$\\\\)\";\n> +\tif (!strcmp(ident, \"tex\"))\n> +\t\treturn \"^\\\\\\\\\\\\(sub\\\\)*section{\";\n> \n> \treturn NULL;\n> }\n\nIt's getting late so I won't be testing this myself tonight, but\nI think\n\n\treturn \"^\\\\(\\\\\\\\\\\\(sub\\\\)*section{.*\\\\)$\";\n\nwould do the job.\n\nHOWEVER.\n\nJohannes, it strikes me that it is very odd having to add \".*$\"\nat the end and to surround the whole thing in a parentheses.\nShouldn't the ff_regexp() grabber simply pick the whole line?\nAfter all, that is what GNU \"diff -p -F RE\" does.\n\nAlso this makes me realize that not all contents in the world\nare not programming language source files, and \"funcname\" is\na misnomer. For this one, however, we _can_ blame GNU diff, as\nthey call this --show-function-line option ;-)\n \n"},{"id":"46685","messageId":"Pine.LNX.4.64.0707071317060.4093@racer.site","threadId":"8866","inReplyTo":"7vabu88qem.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-07T12:17:11Z","receivedAt":"2007-07-07T12:17:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 7 Jul 2007, Junio C Hamano wrote:\n\n> しらいしななこ writes:\n> \n> > I would love to see \"diff=tex\" attribute to work on my manuscripts, \n> > [...]\n> > It does not work well, however. It shows only part of lines.\n> >\n> > \t@@ -8,7 +8,8 @@ \\section{\n> > \t@@ -224,7 +225,7 @@ sub\n> > \t@@ -240,7 +241,7 @@ subsub\n> >\n> > I have no idea what am I doing wrong (truthfully, I do not know what I \n> > am doing, period).\n\nThose are regular expressions. Read more about them here:\n\n\thttp://en.wikipedia.org/wiki/Regular_expression\n\n> > +\tif (!strcmp(ident, \"tex\"))\n> > +\t\treturn \"^\\\\\\\\\\\\(sub\\\\)*section{\";\n\nIt is always easier, and will never require C skills, to put this into the \nconfig. With Junio's current version:\n\n\techo '*.tex funcname=tex' >> .gitattributes\n\techo '[funcname] tex = ^\\(\\\\\\(sub\\)*section{.*\\)' >> .git/config\n\nThe problem is of course that the backslashes have to be escaped _both_ in \nC and in regexps.\n\nYou could write that much simpler, though:\n\n\t\\\\[a-z]*section.*\n\nIt would work the same, in practice, because if something like \n\\supercoolsection is defined, you are likely wanting to match that, too.\n\n> Johannes, it strikes me that it is very odd having to add \".*$\" at the \n> end and to surround the whole thing in a parentheses. Shouldn't the \n> ff_regexp() grabber simply pick the whole line? After all, that is what \n> GNU \"diff -p -F RE\" does.\n\nYes, but then you can forget about your hierarchical idea.\n\nOr maybe not, since I am still awaiting a sane syntax for that, which \nwould probably have to solve just one more problem.\n\nHere is a patch for the rest of the line thing:\n\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex be866d1..9503dfb 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -156,7 +156,7 @@ static long ff_regexp(const char *line, long len,\n \t}\n \ti = pmatch[1].rm_so >= 0 ? 1 : 0;\n \tline += pmatch[i].rm_so;\n-\tresult = pmatch[i].rm_eo - pmatch[i].rm_so;\n+\tresult = (int)len - pmatch[i].rm_so;\n \tif (result > buffer_size)\n \t\tresult = buffer_size;\n \telse\n\n> Also this makes me realize that not all contents in the world are not \n> programming language source files, and \"funcname\" is a misnomer. For \n> this one, however, we _can_ blame GNU diff, as they call this \n> --show-function-line option ;-)\n\nOkay.\n\nHow about sorting out the other technical issues first \n(diff..funcname instead of funcname.), leaving the names \nalone, and then rename the remaining references of funcname to hunkheader?\n\nCiao,\nDscho\n"},{"id":"46714","messageId":"7vsl806jem.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"Pine.LNX.4.64.0707071317060.4093@racer.site","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-07T20:36:17Z","receivedAt":"2007-07-07T20:36:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Johannes, it strikes me that it is very odd having to add \".*$\" at the \n>> end and to surround the whole thing in a parentheses. Shouldn't the \n>> ff_regexp() grabber simply pick the whole line? After all, that is what \n>> GNU \"diff -p -F RE\" does.\n>\n> Yes, but then you can forget about your hierarchical idea.\n\nYeah, I've brought it up just for discussion, but I no longer\nthink the multi-line pattern that picks pieces and paste them\ntogether is worth it.\n"},{"id":"46738","messageId":"7v1wfj4auu.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"Pine.LNX.4.64.0707071317060.4093@racer.site","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T07:23:53Z","receivedAt":"2007-07-08T07:23:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> > +\tif (!strcmp(ident, \"tex\"))\n>> > +\t\treturn \"^\\\\\\\\\\\\(sub\\\\)*section{\";\n>\n> It is always easier, and will never require C skills, to put this into the \n> config. With Junio's current version:\n>\n> \techo '*.tex funcname=tex' >> .gitattributes\n> \techo '[funcname] tex = ^\\(\\\\\\(sub\\)*section{.*\\)' >> .git/config\n>\n> The problem is of course that the backslashes have to be escaped _both_ in \n> C and in regexps.\n\nI think giving a reasonable set of basic language support as\nbuilt-in patterns is important for usability. Otherwise the\nusers end up needing to have them in their $HOME/.gitconfig.\n\nI am not sure if Java and LaTeX qualify as the first two most\nimportant cases, but they are what we already have\ndemonstrated. How about doing something like this?\n\n\n-- >8 --\ndiff.c: make built-in hunk header pattern a separate table\n\nThis would hopefully make it easier to maintain. Initially we\nwould have \"java\" and \"tex\" defined, as they are the only ones\nwe already have.\n\nSigned-off-by: Junio C Hamano \n---\ndiff --git a/diff.c b/diff.c\nindex b8473f5..cd6b0c4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1216,9 +1216,22 @@ static const char *funcname_pattern(const char *ident)\n \treturn NULL;\n }\n \n+static struct builtin_funcname_pattern {\n+\tconst char *name;\n+\tconst char *pattern;\n+} builtin_funcname_pattern[] = {\n+\t{ \"java\", \"!^[ \t]*\\\\(catch\\\\|do\\\\|for\\\\|if\\\\|instanceof\\\\|\"\n+\t\t\t\"new\\\\|return\\\\|switch\\\\|throw\\\\|while\\\\)\\n\"\n+\t\t\t\"^[ \t]*\\\\(\\\\([ \t]*\"\n+\t\t\t\"[A-Za-z_][A-Za-z_0-9]*\\\\)\\\\{2,\\\\}\"\n+\t\t\t\"[ \t]*([^;]*$\\\\)\" },\n+\t{ \"tex\", \"^\\\\(\\\\\\\\\\\\(sub\\\\)*section{.*\\\\)$\" },\n+};\n+\n static const char *diff_funcname_pattern(struct diff_filespec *one)\n {\n \tconst char *ident, *pattern;\n+\tint i;\n \n \tdiff_filespec_check_attr(one);\n \tident = one->funcname_pattern_ident;\n@@ -1240,12 +1253,9 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)\n \t * And define built-in fallback patterns here. Note that\n \t * these can be overriden by the user's config settings.\n \t */\n-\tif (!strcmp(ident, \"java\"))\n-\t\treturn \"!^[ \t]*\\\\(catch\\\\|do\\\\|for\\\\|if\\\\|instanceof\\\\|\"\n-\t\t\t\"new\\\\|return\\\\|switch\\\\|throw\\\\|while\\\\)\\n\"\n-\t\t\t\"^[ \t]*\\\\(\\\\([ \t]*\"\n-\t\t\t\"[A-Za-z_][A-Za-z_0-9]*\\\\)\\\\{2,\\\\}\"\n-\t\t\t\"[ \t]*([^;]*$\\\\)\";\n+\tfor (i = 0; i < ARRAY_SIZE(builtin_funcname_pattern); i++)\n+\t\tif (!strcmp(ident, builtin_funcname_pattern[i].name))\n+\t\t\treturn builtin_funcname_pattern[i].pattern;\n \n \treturn NULL;\n }\n"},{"id":"46748","messageId":"Pine.LNX.4.64.0707081254450.4248@racer.site","threadId":"8866","inReplyTo":"7v1wfj4auu.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T11:58:21Z","receivedAt":"2007-07-08T11:58:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 8 Jul 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> > +\tif (!strcmp(ident, \"tex\"))\n> >> > +\t\treturn \"^\\\\\\\\\\\\(sub\\\\)*section{\";\n> >\n> > It is always easier, and will never require C skills, to put this into the \n> > config. With Junio's current version:\n> >\n> > \techo '*.tex funcname=tex' >> .gitattributes\n> > \techo '[funcname] tex = ^\\(\\\\\\(sub\\)*section{.*\\)' >> .git/config\n> >\n> > The problem is of course that the backslashes have to be escaped _both_ in \n> > C and in regexps.\n> \n> I think giving a reasonable set of basic language support as\n> built-in patterns is important for usability. Otherwise the\n> users end up needing to have them in their $HOME/.gitconfig.\n\nI agree.\n\n> I am not sure if Java and LaTeX qualify as the first two most important \n> cases, but they are what we already have demonstrated.\n\nJava and LaTeX are the two languages I use, for which the inbuilt funcname \ndefault is absolutely unusable. Given that Java is the only language with \na notable git-related code base, for which the default fails, Java is the \nprime (if not the only) reason I wanted this whole patch series in the \nfirst place.\n\n> How about doing something like this?\n\nI do not see how a table in diff.c could be easier to maintain than a \nsmall part in a small function in diff.c. It would make sense, though, to \nmove the table to xdiff-interface.[ch].\n\nCiao,\nDscho\n"},{"id":"46818","messageId":"alpine.LFD.0.999.0707082320480.26459@xanadu.home","threadId":"8866","inReplyTo":"7v8x9tdlbv.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-07-09T03:22:41Z","receivedAt":"2007-07-09T03:22:41Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 6 Jul 2007, Junio C Hamano wrote:\n\n> About the comment from Johannes regarding hunk_header vs\n> funcname, I would actually prefer hunk_header, since that is\n> what this is about (\"funcname\" and \"find_func\" were misnomer\n> from the beginning), but I'd rename hunk_header to funcname for\n> the sake of consistency and minimizing the diff.\n\nI think \"minimizing the diff\" in this case is a bad reason. Using \nhunk_header is so much better than funcname IMHO.\n\n\nNicolas\n"},{"id":"46823","messageId":"7vd4z2xj34.fsf@assigned-by-dhcp.cox.net","threadId":"8866","inReplyTo":"alpine.LFD.0.999.0707082320480.26459@xanadu.home","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-09T05:05:35Z","receivedAt":"2007-07-09T05:05:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> On Fri, 6 Jul 2007, Junio C Hamano wrote:\n>\n>> About the comment from Johannes regarding hunk_header vs\n>> funcname, I would actually prefer hunk_header, since that is\n>> what this is about (\"funcname\" and \"find_func\" were misnomer\n>> from the beginning), but I'd rename hunk_header to funcname for\n>> the sake of consistency and minimizing the diff.\n>\n> I think \"minimizing the diff\" in this case is a bad reason. Using \n> hunk_header is so much better than funcname IMHO.\n\nWell, even then it turns out to be a good reason, as the patch\nto rename function and field can be a separate patch. After\nadding that \"latex pattern\" stuff, I am even more inclined to\nrename them.\n"},{"id":"46855","messageId":"Pine.LNX.4.64.0707091236300.5546@racer.site","threadId":"8866","inReplyTo":"7vd4z2xj34.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Per-path attribute based hunk header selection.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-09T11:40:41Z","receivedAt":"2007-07-09T11:40:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 8 Jul 2007, Junio C Hamano wrote:\n\n> Nicolas Pitre writes:\n> \n> > On Fri, 6 Jul 2007, Junio C Hamano wrote:\n> >\n> >> About the comment from Johannes regarding hunk_header vs\n> >> funcname, I would actually prefer hunk_header, since that is\n> >> what this is about (\"funcname\" and \"find_func\" were misnomer\n> >> from the beginning), but I'd rename hunk_header to funcname for\n> >> the sake of consistency and minimizing the diff.\n> >\n> > I think \"minimizing the diff\" in this case is a bad reason. Using \n> > hunk_header is so much better than funcname IMHO.\n> \n> Well, even then it turns out to be a good reason, as the patch\n> to rename function and field can be a separate patch. After\n> adding that \"latex pattern\" stuff, I am even more inclined to\n> rename them.\n\nNot to mention that even the name \"hunk_header_pattern_ident\" would be a \nmisnomer to begin with. It is the diff attribute we are storing there. \nDid you have any chance to look at\n\n\thttp://thread.gmane.org/gmane.comp.version-control.git/51828\n\nyet? That should clarify things, and\n\n\thttp://article.gmane.org/gmane.comp.version-control.git/51829\n\non top of it should clarify things even more, besides making the code a \nlittle faster again.\n\nCiao,\nDscho\n"}]}