{"thread":{"id":"56990","subject":"git diff -I<regex> does not work with empty +/- lines","startedAt":"2021-11-28T09:19:42Z","lastAt":"2021-11-29T04:57:42Z","messageCount":3,"participants":["Johannes Altmanninger","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"442412","messageId":"20211128091521.7ysocurtt4qlgcf6@gmail.com","threadId":"56990","inReplyTo":null,"subject":"git diff -I<regex> does not work with empty +/- lines","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2021-11-28T09:15:21Z","receivedAt":"2021-11-28T09:19:42Z","isPatch":false,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"diff -I<regex> suppresses hunks where all +/- lines match <regex>.\nit is useful to filter away boilerplate changes.\n\nUnfortunately, it doesn't help if a hunk has a blank line, because the one\nobvious way to filter out blank lines (^$) match *every* line, surprisingly.\nThis is because for a line \"01\\n\"\nwe have a zero-width match here ^$ (offset 3).\n\nIOW, while we succesfully ignore deleted blank lines\n\n\tprintf '\\n' | git diff --no-index - /dev/null -I'^$'\n\tdiff --git a/- b/-\n\tdeleted file mode 100644\n\nwe also ignore non-blank lines (very surprising)\n\n\tprintf 'non-blank-line\\n' | git diff --no-index - /dev/null -I'^$'\n\tdiff --git a/- b/-\n\tdeleted file mode 100644\n\nunless they don't end in a newline (special case)\n\n\tprintf 'line without ending newline' | git diff --no-index - /dev/null -I'^$'\n\tdiff --git a/- b/-\n\tdeleted file mode 100644\n\t--- a/-\n\t+++ /dev/null\n\t@@ -1 +0,0 @@\n\t-line without ending newline\n\t\\ No newline at end of file\n\nThis patch fixes the second example. Is this the right direction?\nDo we want to honor core.eol, so we preserve the \\r when we have Unix endings?\n\nIn any case -I<regex> won't be able to discern between \"line\\n\" and \"line\"\nbut that's not important to me.\n\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex a4542c05b6..23325022b9 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -1016,10 +1016,17 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)\n static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {\n \tregmatch_t regmatch;\n \tint i;\n+\tconst char *end = rec->ptr + rec->size;\n+\n+\tif (rec->size >= 2 && end[-2] == '\\r' && end[-1] == '\\n') {\n+\t\tend -= 2;\n+\t} else if (rec->size && end[-1] == '\\n') {\n+\t\tend -= 1;\n+\t}\n \n \tfor (i = 0; i < xpp->ignore_regex_nr; i++)\n-\t\tif (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,\n-\t\t\t\t &regmatch, 0))\n+\t\tif (!regexec_buf(xpp->ignore_regex[i], rec->ptr,\n+\t\t\t\t end - rec->ptr, 1, &regmatch, 0))\n \t\t\treturn 1;\n \n \treturn 0;\n"},{"id":"442426","messageId":"xmqqtufv50bx.fsf@gitster.g","threadId":"56990","inReplyTo":"20211128091521.7ysocurtt4qlgcf6@gmail.com","subject":"Re: git diff -I<regex> does not work with empty +/- lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-28T23:26:10Z","receivedAt":"2021-11-28T23:28:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Altmanninger <aclopte@gmail.com> writes:\n\n> diff -I<regex> suppresses hunks where all +/- lines match <regex>.\n> it is useful to filter away boilerplate changes.\n>\n> Unfortunately, it doesn't help if a hunk has a blank line, because the one\n> obvious way to filter out blank lines (^$) match *every* line, surprisingly.\n> This is because for a line \"01\\n\"\n> we have a zero-width match here ^$ (offset 3).\n>\n> IOW, while we succesfully ignore deleted blank lines\n>\n> \tprintf '\\n' | git diff --no-index - /dev/null -I'^$'\n> \tdiff --git a/- b/-\n> \tdeleted file mode 100644\n>\n> we also ignore non-blank lines (very surprising)\n>\n> \tprintf 'non-blank-line\\n' | git diff --no-index - /dev/null -I'^$'\n> \tdiff --git a/- b/-\n> \tdeleted file mode 100644\n>\n> unless they don't end in a newline (special case)\n>\n> \tprintf 'line without ending newline' | git diff --no-index - /dev/null -I'^$'\n> \tdiff --git a/- b/-\n> \tdeleted file mode 100644\n> \t--- a/-\n> \t+++ /dev/null\n> \t@@ -1 +0,0 @@\n> \t-line without ending newline\n> \t\\ No newline at end of file\n>\n> This patch fixes the second example. Is this the right direction?\n\nI do not know where in the code the breakage in the first example\ncomes from.  It sounds like a bug if a pattern is not matched\nhonoring the anchor, whether the beginning-of-line \"^\" or the\nend-of-line \"$\" one.\n\n> Do we want to honor core.eol, so we preserve the \\r when we have Unix endings?\n\nAbsolutely.  I think the code paths that work on blob data (i.e. Git\ninternal, before the smudge operation is applied to externalize it\nto a working tree file) should only consider LF as the end of line\n(and any existing code that doesn't is a bug).\n\n> In any case -I<regex> won't be able to discern between \"line\\n\" and \"line\"\n> but that's not important to me.\n>\n> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\n> index a4542c05b6..23325022b9 100644\n> --- a/xdiff/xdiffi.c\n> +++ b/xdiff/xdiffi.c\n> @@ -1016,10 +1016,17 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)\n>  static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {\n>  \tregmatch_t regmatch;\n>  \tint i;\n> +\tconst char *end = rec->ptr + rec->size;\n> +\n> +\tif (rec->size >= 2 && end[-2] == '\\r' && end[-1] == '\\n') {\n> +\t\tend -= 2;\n> +\t} else if (rec->size && end[-1] == '\\n') {\n> +\t\tend -= 1;\n> +\t}\n>  \n>  \tfor (i = 0; i < xpp->ignore_regex_nr; i++)\n> -\t\tif (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,\n> -\t\t\t\t &regmatch, 0))\n> +\t\tif (!regexec_buf(xpp->ignore_regex[i], rec->ptr,\n> +\t\t\t\t end - rec->ptr, 1, &regmatch, 0))\n>  \t\t\treturn 1;\n>  \n>  \treturn 0;\n"},{"id":"442430","messageId":"20211129045422.fyowpxfabmyahaov@gmail.com","threadId":"56990","inReplyTo":"xmqqtufv50bx.fsf@gitster.g","subject":"Re: git diff -I<regex> does not work with empty +/- lines","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2021-11-29T04:54:22Z","receivedAt":"2021-11-29T04:57:42Z","isPatch":false,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"On Sun, Nov 28, 2021 at 03:26:10PM -0800, Junio C Hamano wrote:\n> Johannes Altmanninger <aclopte@gmail.com> writes:\n> \n> > diff -I<regex> suppresses hunks where all +/- lines match <regex>.\n> > it is useful to filter away boilerplate changes.\n> >\n> > Unfortunately, it doesn't help if a hunk has a blank line, because the one\n> > obvious way to filter out blank lines (^$) match *every* line, surprisingly.\n> > This is because for a line \"01\\n\"\n> > we have a zero-width match here ^$ (offset 3).\n> >\n> > IOW, while we succesfully ignore deleted blank lines\n> >\n> > \tprintf '\\n' | git diff --no-index - /dev/null -I'^$'\n> > \tdiff --git a/- b/-\n> > \tdeleted file mode 100644\n> >\n> > we also ignore non-blank lines (very surprising)\n> >\n> > \tprintf 'non-blank-line\\n' | git diff --no-index - /dev/null -I'^$'\n> > \tdiff --git a/- b/-\n> > \tdeleted file mode 100644\n> >\n> > unless they don't end in a newline (special case)\n> >\n> > \tprintf 'line without ending newline' | git diff --no-index - /dev/null -I'^$'\n> > \tdiff --git a/- b/-\n> > \tdeleted file mode 100644\n> > \t--- a/-\n> > \t+++ /dev/null\n> > \t@@ -1 +0,0 @@\n> > \t-line without ending newline\n> > \t\\ No newline at end of file\n> >\n> > This patch fixes the second example. Is this the right direction?\n> \n> I do not know where in the code the breakage in the first example\n> comes from.  It sounds like a bug if a pattern is not matched\n> honoring the anchor, whether the beginning-of-line \"^\" or the\n> end-of-line \"$\" one.\n\nTHe first example (printf '\\n' | ... -I'^$') works fine AFAICT.\nThe regex matches the empty line, so the hunk is ignored\n(but the file header is still printed).\nOnly the second example (printf 'non-blank-line\\n' | ... -I'^$')\nshows that we ignore too much, because ^$ (incorrectly) matches,\nbut it should only match blank lines.\n\nThanks\n"}]}