From 392809702016cde59d50a7b07e8c27f6d0ec3c3f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 27 Aug 2008 19:48:01 -0700 Subject: diff: Help "less" hide ^M from the output When the tracked contents have CRLF line endings, colored diff output shows "^M" at the end of output lines, which is distracting, even though the pager we use by default ("less") knows to hide them. The problem is that "less" hides a carriage-return only at the end of the line, immediately before a line feed. The colored diff output does not take this into account, and emits four element sequence for each line: - force this color; - the line up to but not including the terminating line feed; - reset color - line feed. By including the carriage return at the end of the line in the second item, we are breaking the smart our pager has in order not to show "^M". This can be fixed by changing the sequence to: - force this color; - the line up to but not including the terminating end-of-line; - reset color - end-of-line. where end-of-line is either a single linefeed or a CRLF pair. When the output is not colored, "force this color" and "reset color" sequences are both empty, so we won't have this problem with or without this patch. Signed-off-by: Junio C Hamano --- diff.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 7b4300a74a..6d56c69810 100644 --- a/diff.c +++ b/diff.c @@ -511,13 +511,20 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix) static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len) { - int has_trailing_newline = (len > 0 && line[len-1] == '\n'); + int has_trailing_newline, has_trailing_carriage_return; + + has_trailing_newline = (len > 0 && line[len-1] == '\n'); if (has_trailing_newline) len--; + has_trailing_carriage_return = (len > 0 && line[len-1] == '\r'); + if (has_trailing_carriage_return) + len--; fputs(set, file); fwrite(line, len, 1, file); fputs(reset, file); + if (has_trailing_carriage_return) + fputc('\r', file); if (has_trailing_newline) fputc('\n', file); } -- cgit v1.2.3 From 45e7ca0f0e1042c26d56b578165365c3f70c0121 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 18 Sep 2008 17:40:48 -0500 Subject: diff.c: return pattern entry pointer rather than just the hunk header pattern This is in preparation for associating a flag with each pattern which will control how the pattern is interpreted. For example, as a basic or extended regular expression. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- diff.c | 55 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 27 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 5e01b2bb27..406a76a203 100644 --- a/diff.c +++ b/diff.c @@ -94,32 +94,35 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val * to define a customized regexp to find the beginning of a function to * be used for hunk header lines of "diff -p" style output. */ -static struct funcname_pattern { +struct funcname_pattern_entry { char *name; char *pattern; - struct funcname_pattern *next; +}; +static struct funcname_pattern_list { + struct funcname_pattern_list *next; + struct funcname_pattern_entry e; } *funcname_pattern_list; static int parse_funcname_pattern(const char *var, const char *ep, const char *value) { const char *name; int namelen; - struct funcname_pattern *pp; + struct funcname_pattern_list *pp; name = var + 5; /* "diff." */ namelen = ep - name; for (pp = funcname_pattern_list; pp; pp = pp->next) - if (!strncmp(pp->name, name, namelen) && !pp->name[namelen]) + if (!strncmp(pp->e.name, name, namelen) && !pp->e.name[namelen]) break; if (!pp) { pp = xcalloc(1, sizeof(*pp)); - pp->name = xmemdupz(name, namelen); + pp->e.name = xmemdupz(name, namelen); pp->next = funcname_pattern_list; funcname_pattern_list = pp; } - free(pp->pattern); - pp->pattern = xstrdup(value); + free(pp->e.pattern); + pp->e.pattern = xstrdup(value); return 0; } @@ -1377,20 +1380,17 @@ int diff_filespec_is_binary(struct diff_filespec *one) return one->is_binary; } -static const char *funcname_pattern(const char *ident) +static const struct funcname_pattern_entry *funcname_pattern(const char *ident) { - struct funcname_pattern *pp; + struct funcname_pattern_list *pp; for (pp = funcname_pattern_list; pp; pp = pp->next) - if (!strcmp(ident, pp->name)) - return pp->pattern; + if (!strcmp(ident, pp->e.name)) + return &pp->e; return NULL; } -static struct builtin_funcname_pattern { - const char *name; - const char *pattern; -} builtin_funcname_pattern[] = { +static const struct funcname_pattern_entry builtin_funcname_pattern[] = { { "java", "!^[ ]*\\(catch\\|do\\|for\\|if\\|instanceof\\|" "new\\|return\\|switch\\|throw\\|while\\)\n" "^[ ]*\\(\\([ ]*" @@ -1407,9 +1407,10 @@ static struct builtin_funcname_pattern { { "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$" }, }; -static const char *diff_funcname_pattern(struct diff_filespec *one) +static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_filespec *one) { - const char *ident, *pattern; + const char *ident; + const struct funcname_pattern_entry *pe; int i; diff_filespec_check_attr(one); @@ -1424,9 +1425,9 @@ static const char *diff_funcname_pattern(struct diff_filespec *one) return funcname_pattern("default"); /* Look up custom "funcname.$ident" regexp from config. */ - pattern = funcname_pattern(ident); - if (pattern) - return pattern; + pe = funcname_pattern(ident); + if (pe) + return pe; /* * And define built-in fallback patterns here. Note that @@ -1434,7 +1435,7 @@ static const char *diff_funcname_pattern(struct diff_filespec *one) */ for (i = 0; i < ARRAY_SIZE(builtin_funcname_pattern); i++) if (!strcmp(ident, builtin_funcname_pattern[i].name)) - return builtin_funcname_pattern[i].pattern; + return &builtin_funcname_pattern[i]; return NULL; } @@ -1512,11 +1513,11 @@ static void builtin_diff(const char *name_a, xdemitconf_t xecfg; xdemitcb_t ecb; struct emit_callback ecbdata; - const char *funcname_pattern; + const struct funcname_pattern_entry *pe; - funcname_pattern = diff_funcname_pattern(one); - if (!funcname_pattern) - funcname_pattern = diff_funcname_pattern(two); + pe = diff_funcname_pattern(one); + if (!pe) + pe = diff_funcname_pattern(two); memset(&xecfg, 0, sizeof(xecfg)); memset(&ecbdata, 0, sizeof(ecbdata)); @@ -1528,8 +1529,8 @@ static void builtin_diff(const char *name_a, xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts; xecfg.ctxlen = o->context; xecfg.flags = XDL_EMIT_FUNCNAMES; - if (funcname_pattern) - xdiff_set_find_func(&xecfg, funcname_pattern); + if (pe) + xdiff_set_find_func(&xecfg, pe->pattern); if (!diffopts) ; else if (!prefixcmp(diffopts, "--unified=")) -- cgit v1.2.3 From a013585b20ac757b0e75a72181ffa44674f35235 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 18 Sep 2008 17:42:48 -0500 Subject: diff.c: associate a flag with each pattern and use it for compiling regex This is in preparation for allowing extended regular expression patterns. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- diff.c | 21 ++++++++++++--------- xdiff-interface.c | 4 ++-- xdiff-interface.h | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 406a76a203..6881cf4efa 100644 --- a/diff.c +++ b/diff.c @@ -97,13 +97,14 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val struct funcname_pattern_entry { char *name; char *pattern; + int cflags; }; static struct funcname_pattern_list { struct funcname_pattern_list *next; struct funcname_pattern_entry e; } *funcname_pattern_list; -static int parse_funcname_pattern(const char *var, const char *ep, const char *value) +static int parse_funcname_pattern(const char *var, const char *ep, const char *value, int cflags) { const char *name; int namelen; @@ -123,6 +124,7 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v } free(pp->e.pattern); pp->e.pattern = xstrdup(value); + pp->e.cflags = cflags; return 0; } @@ -185,7 +187,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) if (!strcmp(ep, ".funcname")) { if (!value) return config_error_nonbool(var); - return parse_funcname_pattern(var, ep, value); + return parse_funcname_pattern(var, ep, value, + 0); } } } @@ -1395,16 +1398,16 @@ static const struct funcname_pattern_entry builtin_funcname_pattern[] = { "new\\|return\\|switch\\|throw\\|while\\)\n" "^[ ]*\\(\\([ ]*" "[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}" - "[ ]*([^;]*\\)$" }, + "[ ]*([^;]*\\)$", 0 }, { "pascal", "^\\(\\(procedure\\|function\\|constructor\\|" "destructor\\|interface\\|implementation\\|" "initialization\\|finalization\\)[ \t]*.*\\)$" "\\|" - "^\\(.*=[ \t]*\\(class\\|record\\).*\\)$" - }, - { "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$" }, - { "tex", "^\\(\\\\\\(\\(sub\\)*section\\|chapter\\|part\\)\\*\\{0,1\\}{.*\\)$" }, - { "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$" }, + "^\\(.*=[ \t]*\\(class\\|record\\).*\\)$", + 0 }, + { "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$", 0 }, + { "tex", "^\\(\\\\\\(\\(sub\\)*section\\|chapter\\|part\\)\\*\\{0,1\\}{.*\\)$", 0 }, + { "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$", 0 }, }; static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_filespec *one) @@ -1530,7 +1533,7 @@ static void builtin_diff(const char *name_a, xecfg.ctxlen = o->context; xecfg.flags = XDL_EMIT_FUNCNAMES; if (pe) - xdiff_set_find_func(&xecfg, pe->pattern); + xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags); if (!diffopts) ; else if (!prefixcmp(diffopts, "--unified=")) diff --git a/xdiff-interface.c b/xdiff-interface.c index 61dc5c5470..2c81f40cb6 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -206,7 +206,7 @@ static long ff_regexp(const char *line, long len, return result; } -void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value) +void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags) { int i; struct ff_regs *regs; @@ -231,7 +231,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value) expression = buffer = xstrndup(value, ep - value); else expression = value; - if (regcomp(®->re, expression, 0)) + if (regcomp(®->re, expression, cflags)) die("Invalid regexp to look for hunk header: %s", expression); free(buffer); value = ep + 1; diff --git a/xdiff-interface.h b/xdiff-interface.h index f7f791d96b..33cab9dd59 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -21,6 +21,6 @@ int parse_hunk_header(char *line, int len, int read_mmfile(mmfile_t *ptr, const char *filename); int buffer_is_binary(const char *ptr, unsigned long size); -extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line); +extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags); #endif -- cgit v1.2.3 From 45d9414fa5599b41578625961b53e18a9b9148c7 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 18 Sep 2008 17:44:33 -0500 Subject: diff.*.xfuncname which uses "extended" regex's for hunk header selection Currently, the hunk headers produced by 'diff -p' are customizable by setting the diff.*.funcname option in the config file. The 'funcname' option takes a basic regular expression. This functionality was designed using the GNU regex library which, by default, allows using backslashed versions of some extended regular expression operators, even in Basic Regular Expression mode. For example, the following characters, when backslashed, are interpreted according to the extended regular expression rules: ?, +, and |. As such, the builtin funcname patterns were created using some extended regular expression operators. Other platforms which adhere more strictly to the POSIX spec do not interpret the backslashed extended RE operators in Basic Regular Expression mode. This causes the pattern matching for the builtin funcname patterns to fail on those platforms. Introduce a new option 'xfuncname' which uses extended regular expressions, and advertise it _instead_ of funcname. Since most users are on GNU platforms, the majority of funcname patterns are created and tested there. Advertising only xfuncname should help to avoid the creation of non-portable patterns which work with GNU regex but not elsewhere. Additionally, the extended regular expressions may be less ugly and complicated compared to the basic RE since many common special operators do not need to be backslashed. For example, the GNU Basic RE: ^[ ]*\\(\\(public\\|static\\).*\\)$ becomes the following Extended RE: ^[ ]*((public|static).*)$ Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- Documentation/gitattributes.txt | 4 ++-- diff.c | 5 +++++ t/t4018-diff-funcname.sh | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) (limited to 'diff.c') diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index 94e6752aa2..9259637609 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -288,13 +288,13 @@ for paths. *.tex diff=tex ------------------------ -Then, you would define "diff.tex.funcname" configuration to +Then, you would define "diff.tex.xfuncname" configuration to specify a regular expression that matches a line that you would want to appear as the hunk header, like this: ------------------------ [diff "tex"] - funcname = "^\\(\\\\\\(sub\\)*section{.*\\)$" + xfuncname = "^(\\\\(sub)*section\\{.*)$" ------------------------ Note. A single level of backslashes are eaten by the diff --git a/diff.c b/diff.c index 6881cf4efa..dabb4b4a02 100644 --- a/diff.c +++ b/diff.c @@ -189,6 +189,11 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) return config_error_nonbool(var); return parse_funcname_pattern(var, ep, value, 0); + } else if (!strcmp(ep, ".xfuncname")) { + if (!value) + return config_error_nonbool(var); + return parse_funcname_pattern(var, ep, value, + REG_EXTENDED); } } } diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 18bcd9713d..602d68f092 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -58,7 +58,7 @@ test_expect_success 'last regexp must not be negated' ' ' test_expect_success 'alternation in pattern' ' - git config diff.java.funcname "^[ ]*\\(\\(public\\|static\\).*\\)$" + git config diff.java.xfuncname "^[ ]*((public|static).*)$" && git diff --no-index Beer.java Beer-correct.java | grep "^@@.*@@ public static void main(" ' -- cgit v1.2.3