threads / discuss / 56990

git diff -I<regex> does not work with empty +/- lines

Subject: git diff -I<regex> does not work with empty +/- lines

## tl;dr

3 messages between Nov 28, 2021 and Nov 29, 2021.

replies: 2people: 2as markdown or json

Johannes Altmanninger· Nov 28, 2021, 09:15 UTC · lore

diff -I<regex> suppresses hunks where all +/- lines match <regex>. it is useful to filter away boilerplate changes.

Unfortunately, it doesn't help if a hunk has a blank line, because the one obvious way to filter out blank lines (^$) match *every* line, surprisingly. This is because for a line "01\n" we have a zero-width match here ^$ (offset 3).

IOW, while we succesfully ignore deleted blank lines

	printf '\n' | git diff --no-index - /dev/null -I'^$'
	diff --git a/- b/-
	deleted file mode 100644

we also ignore non-blank lines (very surprising)

	printf 'non-blank-line\n' | git diff --no-index - /dev/null -I'^$'
	diff --git a/- b/-
	deleted file mode 100644

unless they don't end in a newline (special case)

	printf 'line without ending newline' | git diff --no-index - /dev/null -I'^$'
	diff --git a/- b/-
	deleted file mode 100644
	--- a/-
	+++ /dev/null
	@@ -1 +0,0 @@
	-line without ending newline
	\ No newline at end of file

This patch fixes the second example. Is this the right direction? Do we want to honor core.eol, so we preserve the \r when we have Unix endings?

In any case -I<regex> won't be able to discern between "line\n" and "line" but that's not important to me.

diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index a4542c05b6..23325022b9 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -1016,10 +1016,17 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)
 static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {
 	regmatch_t regmatch;
 	int i;
+	const char *end = rec->ptr + rec->size;
+
+	if (rec->size >= 2 && end[-2] == '\r' && end[-1] == '\n') {
+		end -= 2;
+	} else if (rec->size && end[-1] == '\n') {
+		end -= 1;
+	}
 
 	for (i = 0; i < xpp->ignore_regex_nr; i++)
-		if (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,
-				 &regmatch, 0))
+		if (!regexec_buf(xpp->ignore_regex[i], rec->ptr,
+				 end - rec->ptr, 1, &regmatch, 0))
 			return 1;
 
 	return 0;

Junio C Hamano· Nov 28, 2021, 23:26 UTC · re: Johannes Altmanninger · lore

Re: git diff -I<regex> does not work with empty +/- lines

Johannes Altmanninger <aclopte@gmail.com> writes:

Show 32 quoted lines

> diff -I<regex> suppresses hunks where all +/- lines match <regex>.
> it is useful to filter away boilerplate changes.
>
> Unfortunately, it doesn't help if a hunk has a blank line, because the one
> obvious way to filter out blank lines (^$) match *every* line, surprisingly.
> This is because for a line "01\n"
> we have a zero-width match here ^$ (offset 3).
>
> IOW, while we succesfully ignore deleted blank lines
>
> 	printf '\n' | git diff --no-index - /dev/null -I'^$'
> 	diff --git a/- b/-
> 	deleted file mode 100644
>
> we also ignore non-blank lines (very surprising)
>
> 	printf 'non-blank-line\n' | git diff --no-index - /dev/null -I'^$'
> 	diff --git a/- b/-
> 	deleted file mode 100644
>
> unless they don't end in a newline (special case)
>
> 	printf 'line without ending newline' | git diff --no-index - /dev/null -I'^$'
> 	diff --git a/- b/-
> 	deleted file mode 100644
> 	--- a/-
> 	+++ /dev/null
> 	@@ -1 +0,0 @@
> 	-line without ending newline
> 	\ No newline at end of file
>
> This patch fixes the second example. Is this the right direction?

I do not know where in the code the breakage in the first example comes from. It sounds like a bug if a pattern is not matched honoring the anchor, whether the beginning-of-line "^" or the end-of-line "$" one.

> Do we want to honor core.eol, so we preserve the \r when we have Unix endings?

Absolutely. I think the code paths that work on blob data (i.e. Git internal, before the smudge operation is applied to externalize it to a working tree file) should only consider LF as the end of line (and any existing code that doesn't is a bug).

Show 27 quoted lines

> In any case -I<regex> won't be able to discern between "line\n" and "line"
> but that's not important to me.
>
> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
> index a4542c05b6..23325022b9 100644
> --- a/xdiff/xdiffi.c
> +++ b/xdiff/xdiffi.c
> @@ -1016,10 +1016,17 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)
>  static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {
>  	regmatch_t regmatch;
>  	int i;
> +	const char *end = rec->ptr + rec->size;
> +
> +	if (rec->size >= 2 && end[-2] == '\r' && end[-1] == '\n') {
> +		end -= 2;
> +	} else if (rec->size && end[-1] == '\n') {
> +		end -= 1;
> +	}
>  
>  	for (i = 0; i < xpp->ignore_regex_nr; i++)
> -		if (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,
> -				 &regmatch, 0))
> +		if (!regexec_buf(xpp->ignore_regex[i], rec->ptr,
> +				 end - rec->ptr, 1, &regmatch, 0))
>  			return 1;
>  
>  	return 0;

Johannes Altmanninger· Nov 29, 2021, 04:54 UTC · re: Junio C Hamano · lore

Re: git diff -I<regex> does not work with empty +/- lines

On Sun, Nov 28, 2021 at 03:26:10PM -0800, Junio C Hamano wrote:

Show 39 quoted lines

> Johannes Altmanninger <aclopte@gmail.com> writes:
> 
> > diff -I<regex> suppresses hunks where all +/- lines match <regex>.
> > it is useful to filter away boilerplate changes.
> >
> > Unfortunately, it doesn't help if a hunk has a blank line, because the one
> > obvious way to filter out blank lines (^$) match *every* line, surprisingly.
> > This is because for a line "01\n"
> > we have a zero-width match here ^$ (offset 3).
> >
> > IOW, while we succesfully ignore deleted blank lines
> >
> > 	printf '\n' | git diff --no-index - /dev/null -I'^$'
> > 	diff --git a/- b/-
> > 	deleted file mode 100644
> >
> > we also ignore non-blank lines (very surprising)
> >
> > 	printf 'non-blank-line\n' | git diff --no-index - /dev/null -I'^$'
> > 	diff --git a/- b/-
> > 	deleted file mode 100644
> >
> > unless they don't end in a newline (special case)
> >
> > 	printf 'line without ending newline' | git diff --no-index - /dev/null -I'^$'
> > 	diff --git a/- b/-
> > 	deleted file mode 100644
> > 	--- a/-
> > 	+++ /dev/null
> > 	@@ -1 +0,0 @@
> > 	-line without ending newline
> > 	\ No newline at end of file
> >
> > This patch fixes the second example. Is this the right direction?
> 
> I do not know where in the code the breakage in the first example
> comes from.  It sounds like a bug if a pattern is not matched
> honoring the anchor, whether the beginning-of-line "^" or the
> end-of-line "$" one.

THe first example (printf '\n' | ... -I'^$') works fine AFAICT. The regex matches the empty line, so the hunk is ignored (but the file header is still printed). Only the second example (printf 'non-blank-line\n' | ... -I'^$') shows that we ignore too much, because ^$ (incorrectly) matches, but it should only match blank lines.

Thanks