Skip to content

Commit be58e70

Browse files
peffgitster
peff
authored and
gitster
committed
diff: unify external diff and funcname parsing code
Both sets of code assume that one specifies a diff profile as a gitattribute via the "diff=foo" attribute. They then pull information about that profile from the config as diff.foo.*. The code for each is currently completely separate from the other, which has several disadvantages: - there is duplication as we maintain code to create and search the separate lists of external drivers and funcname patterns - it is difficult to add new profile options, since it is unclear where they should go - the code is difficult to follow, as we rely on the "check if this file is binary" code to find the funcname pattern as a side effect. This is the first step in refactoring the binary-checking code. This patch factors out these diff profiles into "userdiff" drivers. A file with "diff=foo" uses the "foo" driver, which is specified by a single struct. Note that one major difference between the two pieces of code is that the funcname patterns are always loaded, whereas external drivers are loaded only for the "git diff" porcelain; the new code takes care to retain that situation. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
1 parent e7881c3 commit be58e70

File tree

4 files changed

+212
-224
lines changed

4 files changed

+212
-224
lines changed

‎Makefile‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ LIB_H += transport.h
389389
LIB_H += tree.h
390390
LIB_H += tree-walk.h
391391
LIB_H += unpack-trees.h
392+
LIB_H += userdiff.h
392393
LIB_H += utf8.h
393394
LIB_H += wt-status.h
394395

@@ -485,6 +486,7 @@ LIB_OBJS += tree-diff.o
485486
LIB_OBJS += tree.o
486487
LIB_OBJS += tree-walk.o
487488
LIB_OBJS += unpack-trees.o
489+
LIB_OBJS += userdiff.o
488490
LIB_OBJS += usage.o
489491
LIB_OBJS += utf8.o
490492
LIB_OBJS += walker.o

‎diff.c‎

Lines changed: 27 additions & 224 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "attr.h"
1212
#include "run-command.h"
1313
#include "utf8.h"
14+
#include "userdiff.h"
1415

1516
#ifdef NO_FAST_WORKING_DIRECTORY
1617
#define FAST_WORKING_DIRECTORY 0
@@ -56,80 +57,6 @@ static int parse_diff_color_slot(const char *var, int ofs)
5657
die("bad config variable '%s'", var);
5758
}
5859

59-
static struct ll_diff_driver {
60-
const char *name;
61-
struct ll_diff_driver *next;
62-
const char *cmd;
63-
} *user_diff, **user_diff_tail;
64-
65-
/*
66-
* Currently there is only "diff.<drivername>.command" variable;
67-
* because there are "diff.color.<slot>" variables, we are parsing
68-
* this in a bit convoluted way to allow low level diff driver
69-
* called "color".
70-
*/
71-
static int parse_lldiff_command(const char *var, const char *ep, const char *value)
72-
{
73-
const char *name;
74-
int namelen;
75-
struct ll_diff_driver *drv;
76-
77-
name = var + 5;
78-
namelen = ep - name;
79-
for (drv = user_diff; drv; drv = drv->next)
80-
if (!strncmp(drv->name, name, namelen) && !drv->name[namelen])
81-
break;
82-
if (!drv) {
83-
drv = xcalloc(1, sizeof(struct ll_diff_driver));
84-
drv->name = xmemdupz(name, namelen);
85-
if (!user_diff_tail)
86-
user_diff_tail = &user_diff;
87-
*user_diff_tail = drv;
88-
user_diff_tail = &(drv->next);
89-
}
90-
91-
return git_config_string(&(drv->cmd), var, value);
92-
}
93-
94-
/*
95-
* 'diff.<what>.funcname' attribute can be specified in the configuration
96-
* to define a customized regexp to find the beginning of a function to
97-
* be used for hunk header lines of "diff -p" style output.
98-
*/
99-
struct funcname_pattern_entry {
100-
char *name;
101-
char *pattern;
102-
int cflags;
103-
};
104-
static struct funcname_pattern_list {
105-
struct funcname_pattern_list *next;
106-
struct funcname_pattern_entry e;
107-
} *funcname_pattern_list;
108-
109-
static int parse_funcname_pattern(const char *var, const char *ep, const char *value, int cflags)
110-
{
111-
const char *name;
112-
int namelen;
113-
struct funcname_pattern_list *pp;
114-
115-
name = var + 5; /* "diff." */
116-
namelen = ep - name;
117-
118-
for (pp = funcname_pattern_list; pp; pp = pp->next)
119-
if (!strncmp(pp->e.name, name, namelen) && !pp->e.name[namelen])
120-
break;
121-
if (!pp) {
122-
pp = xcalloc(1, sizeof(*pp));
123-
pp->e.name = xmemdupz(name, namelen);
124-
pp->next = funcname_pattern_list;
125-
funcname_pattern_list = pp;
126-
}
127-
free(pp->e.pattern);
128-
pp->e.pattern = xstrdup(value);
129-
pp->e.cflags = cflags;
130-
return 0;
131-
}
132-
13360
/*
13461
* These are to give UI layer defaults.
13562
* The core-level commands such as git-diff-files should
@@ -162,11 +89,11 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
16289
}
16390
if (!strcmp(var, "diff.external"))
16491
return git_config_string(&external_diff_cmd_cfg, var, value);
165-
if (!prefixcmp(var, "diff.")) {
166-
const char *ep = strrchr(var, '.');
16792

168-
if (ep != var + 4 && !strcmp(ep, ".command"))
169-
return parse_lldiff_command(var, ep, value);
93+
switch (userdiff_config_porcelain(var, value)) {
94+
case 0: break;
95+
case -1: return -1;
96+
default: return 0;
17097
}
17198

17299
return git_diff_basic_config(var, value, cb);
@@ -193,21 +120,10 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
193120
return 0;
194121
}
195122

196-
if (!prefixcmp(var, "diff.")) {
197-
const char *ep = strrchr(var, '.');
198-
if (ep != var + 4) {
199-
if (!strcmp(ep, ".funcname")) {
200-
if (!value)
201-
return config_error_nonbool(var);
202-
return parse_funcname_pattern(var, ep, value,
203-
0);
204-
} else if (!strcmp(ep, ".xfuncname")) {
205-
if (!value)
206-
return config_error_nonbool(var);
207-
return parse_funcname_pattern(var, ep, value,
208-
REG_EXTENDED);
209-
}
210-
}
123+
switch (userdiff_config_basic(var, value)) {
124+
case 0: break;
125+
case -1: return -1;
126+
default: return 0;
211127
}
212128

213129
return git_color_default_config(var, value, cb);
@@ -1352,46 +1268,24 @@ static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two)
13521268
emit_binary_diff_body(file, two, one);
13531269
}
13541270

1355-
static void setup_diff_attr_check(struct git_attr_check *check)
1356-
{
1357-
static struct git_attr *attr_diff;
1358-
1359-
if (!attr_diff) {
1360-
attr_diff = git_attr("diff", 4);
1361-
}
1362-
check[0].attr = attr_diff;
1363-
}
1364-
13651271
static void diff_filespec_check_attr(struct diff_filespec *one)
13661272
{
1367-
struct git_attr_check attr_diff_check;
1273+
struct userdiff_driver *drv;
13681274
int check_from_data = 0;
13691275

13701276
if (one->checked_attr)
13711277
return;
13721278

1373-
setup_diff_attr_check(&attr_diff_check);
1279+
drv = userdiff_find_by_path(one->path);
13741280
one->is_binary = 0;
1375-
one->funcname_pattern_ident = NULL;
13761281

1377-
if (!git_checkattr(one->path, 1, &attr_diff_check)) {
1378-
const char *value;
1379-
1380-
/* binaryness */
1381-
value = attr_diff_check.value;
1382-
if (ATTR_TRUE(value))
1383-
;
1384-
else if (ATTR_FALSE(value))
1385-
one->is_binary = 1;
1386-
else
1387-
check_from_data = 1;
1388-
1389-
/* funcname pattern ident */
1390-
if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))
1391-
;
1392-
else
1393-
one->funcname_pattern_ident = value;
1394-
}
1282+
/* binaryness */
1283+
if (drv == USERDIFF_ATTR_TRUE)
1284+
;
1285+
else if (drv == USERDIFF_ATTR_FALSE)
1286+
one->is_binary = 1;
1287+
else
1288+
check_from_data = 1;
13951289

13961290
if (check_from_data) {
13971291
if (!one->data && DIFF_FILE_VALID(one))
@@ -1408,80 +1302,12 @@ int diff_filespec_is_binary(struct diff_filespec *one)
14081302
return one->is_binary;
14091303
}
14101304

1411-
static const struct funcname_pattern_entry *funcname_pattern(const char *ident)
1412-
{
1413-
struct funcname_pattern_list *pp;
1414-
1415-
for (pp = funcname_pattern_list; pp; pp = pp->next)
1416-
if (!strcmp(ident, pp->e.name))
1417-
return &pp->e;
1418-
return NULL;
1419-
}
1420-
1421-
static const struct funcname_pattern_entry builtin_funcname_pattern[] = {
1422-
{ "bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#}{~%]*).*$",
1423-
REG_EXTENDED },
1424-
{ "html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$", REG_EXTENDED },
1425-
{ "java",
1426-
"!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
1427-
"^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$",
1428-
REG_EXTENDED },
1429-
{ "objc",
1430-
/* Negate C statements that can look like functions */
1431-
"!^[ \t]*(do|for|if|else|return|switch|while)\n"
1432-
/* Objective-C methods */
1433-
"^[ \t]*([-+][ \t]*\\([ \t]*[A-Za-z_][A-Za-z_0-9* \t]*\\)[ \t]*[A-Za-z_].*)$\n"
1434-
/* C functions */
1435-
"^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$\n"
1436-
/* Objective-C class/protocol definitions */
1437-
"^(@(implementation|interface|protocol)[ \t].*)$",
1438-
REG_EXTENDED },
1439-
{ "pascal",
1440-
"^((procedure|function|constructor|destructor|interface|"
1441-
"implementation|initialization|finalization)[ \t]*.*)$"
1442-
"\n"
1443-
"^(.*=[ \t]*(class|record).*)$",
1444-
REG_EXTENDED },
1445-
{ "php", "^[\t ]*((function|class).*)", REG_EXTENDED },
1446-
{ "python", "^[ \t]*((class|def)[ \t].*)$", REG_EXTENDED },
1447-
{ "ruby", "^[ \t]*((class|module|def)[ \t].*)$",
1448-
REG_EXTENDED },
1449-
{ "tex",
1450-
"^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
1451-
REG_EXTENDED },
1452-
};
1453-
1454-
static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_filespec *one)
1305+
static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespec *one)
14551306
{
1456-
const char *ident;
1457-
const struct funcname_pattern_entry *pe;
1458-
int i;
1459-
1460-
diff_filespec_check_attr(one);
1461-
ident = one->funcname_pattern_ident;
1462-
1463-
if (!ident)
1464-
/*
1465-
* If the config file has "funcname.default" defined, that
1466-
* regexp is used; otherwise NULL is returned and xemit uses
1467-
* the built-in default.
1468-
*/
1469-
return funcname_pattern("default");
1470-
1471-
/* Look up custom "funcname.$ident" regexp from config. */
1472-
pe = funcname_pattern(ident);
1473-
if (pe)
1474-
return pe;
1475-
1476-
/*
1477-
* And define built-in fallback patterns here. Note that
1478-
* these can be overridden by the user's config settings.
1479-
*/
1480-
for (i = 0; i < ARRAY_SIZE(builtin_funcname_pattern); i++)
1481-
if (!strcmp(ident, builtin_funcname_pattern[i].name))
1482-
return &builtin_funcname_pattern[i];
1483-
1484-
return NULL;
1307+
struct userdiff_driver *drv = userdiff_find_by_path(one->path);
1308+
if (!drv)
1309+
drv = userdiff_find_by_name("default");
1310+
return drv && drv->funcname.pattern ? &drv->funcname : NULL;
14851311
}
14861312

14871313
void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b)
@@ -1579,7 +1405,7 @@ static void builtin_diff(const char *name_a,
15791405
xdemitconf_t xecfg;
15801406
xdemitcb_t ecb;
15811407
struct emit_callback ecbdata;
1582-
const struct funcname_pattern_entry *pe;
1408+
const struct userdiff_funcname *pe;
15831409

15841410
pe = diff_funcname_pattern(one);
15851411
if (!pe)
@@ -2117,29 +1943,6 @@ static void run_external_diff(const char *pgm,
21171943
}
21181944
}
21191945

2120-
static const char *external_diff_attr(const char *name)
2121-
{
2122-
struct git_attr_check attr_diff_check;
2123-
2124-
if (!name)
2125-
return NULL;
2126-
2127-
setup_diff_attr_check(&attr_diff_check);
2128-
if (!git_checkattr(name, 1, &attr_diff_check)) {
2129-
const char *value = attr_diff_check.value;
2130-
if (!ATTR_TRUE(value) &&
2131-
!ATTR_FALSE(value) &&
2132-
!ATTR_UNSET(value)) {
2133-
struct ll_diff_driver *drv;
2134-
2135-
for (drv = user_diff; drv; drv = drv->next)
2136-
if (!strcmp(drv->name, value))
2137-
return drv->cmd;
2138-
}
2139-
}
2140-
return NULL;
2141-
}
2142-
21431946
static void run_diff_cmd(const char *pgm,
21441947
const char *name,
21451948
const char *other,
@@ -2153,9 +1956,9 @@ static void run_diff_cmd(const char *pgm,
21531956
if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
21541957
pgm = NULL;
21551958
else {
2156-
const char *cmd = external_diff_attr(attr_path);
2157-
if (cmd)
2158-
pgm = cmd;
1959+
struct userdiff_driver *drv = userdiff_find_by_path(attr_path);
1960+
if (drv && drv->external)
1961+
pgm = drv->external;
21591962
}
21601963

21611964
if (pgm) {

0 commit comments

Comments
 (0)