{"thread":{"id":"61508","subject":"[PATCH v2] Switch grep from non-portable BRE to portable ERE","startedAt":"2024-05-17T19:01:52Z","lastAt":"2024-05-22T10:52:57Z","messageCount":3,"participants":["Marcel Telka","Patrick Steinhardt"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"494996","messageId":"ZkepnZhGEhSveN00@telcontar","threadId":"61508","inReplyTo":null,"subject":"[PATCH v2] Switch grep from non-portable BRE to portable ERE","fromName":"Marcel Telka","fromEmail":"marcel@telka.sk","sentAt":"2024-05-17T19:01:49Z","receivedAt":"2024-05-17T19:01:52Z","isPatch":true,"sender":{"key":"marcel@telka.sk","avatar":"https://avatars.githubusercontent.com/u/3000173?v=4"},"body":"This makes the grep usage fully POSIX compliant.  The ability to\nenable ERE features in BRE using backslash is a GNU extension.\n\nSigned-off-by: Marcel Telka <marcel@telka.sk>\n---\n mergetools/vimdiff           | 2 +-\n t/t1404-update-ref-errors.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex 734d15a03b..f8ad6b35d4 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -325,7 +325,7 @@ gen_cmd () {\n \t\tfi\n \n \t\t# If this is a single window diff with all the buffers\n-\t\tif ! echo \"$tab\" | grep \",\\|/\" >/dev/null\n+\t\tif ! echo \"$tab\" | grep -E \",|/\" >/dev/null\n \t\tthen\n \t\t\tCMD=\"$CMD | silent execute 'bufdo diffthis'\"\n \t\tfi\ndiff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh\nindex 98e9158bd2..67ebd81a4c 100755\n--- a/t/t1404-update-ref-errors.sh\n+++ b/t/t1404-update-ref-errors.sh\n@@ -100,7 +100,7 @@ df_test() {\n \t\tprintf \"%s\\n\" \"delete $delname\" \"create $addname $D\"\n \tfi >commands &&\n \ttest_must_fail git update-ref --stdin <commands 2>output.err &&\n-\tgrep \"fatal:\\( cannot lock ref $SQ$addname$SQ:\\)\\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ\" output.err &&\n+\tgrep -E \"fatal:( cannot lock ref $SQ$addname$SQ:)? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ\" output.err &&\n \tprintf \"%s\\n\" \"$C $delref\" >expected-refs &&\n \tgit for-each-ref --format=\"%(objectname) %(refname)\" $prefix/r >actual-refs &&\n \ttest_cmp expected-refs actual-refs\n"},{"id":"495140","messageId":"Zkw0ty8gOMS3Opzk@tanuki","threadId":"61508","inReplyTo":"ZkepnZhGEhSveN00@telcontar","subject":"Re: [PATCH v2] Switch grep from non-portable BRE to portable ERE","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-21T05:44:23Z","receivedAt":"2024-05-21T05:44:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 17, 2024 at 09:01:49PM +0200, Marcel Telka wrote:\n> This makes the grep usage fully POSIX compliant.  The ability to\n\nNit: we typically don't say \"This commit\", of which \"This\" is another\nversion. Instead, we use imperative style as if instructing the code to\nchange. Also, we typically first explain what the problem is before we\nsay how we fix it.\n\nI don't think this is worth a reroll though, it's only a hint for the\nnext patch you may be sending :)\n\n> enable ERE features in BRE using backslash is a GNU extension.\n> \n> Signed-off-by: Marcel Telka <marcel@telka.sk>\n\nIt would have been nice if this thread was connected to the thread of\nyour first version so that it's easier to follow the discussion, e.g. by\nusing `--in-reply-to=` in git-send-email(1) or git-format-patch(1).\n\nBut other than that the changes look good to me, thanks!\n\nPatrick\n\n> ---\n>  mergetools/vimdiff           | 2 +-\n>  t/t1404-update-ref-errors.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/mergetools/vimdiff b/mergetools/vimdiff\n> index 734d15a03b..f8ad6b35d4 100644\n> --- a/mergetools/vimdiff\n> +++ b/mergetools/vimdiff\n> @@ -325,7 +325,7 @@ gen_cmd () {\n>  \t\tfi\n>  \n>  \t\t# If this is a single window diff with all the buffers\n> -\t\tif ! echo \"$tab\" | grep \",\\|/\" >/dev/null\n> +\t\tif ! echo \"$tab\" | grep -E \",|/\" >/dev/null\n>  \t\tthen\n>  \t\t\tCMD=\"$CMD | silent execute 'bufdo diffthis'\"\n>  \t\tfi\n> diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh\n> index 98e9158bd2..67ebd81a4c 100755\n> --- a/t/t1404-update-ref-errors.sh\n> +++ b/t/t1404-update-ref-errors.sh\n> @@ -100,7 +100,7 @@ df_test() {\n>  \t\tprintf \"%s\\n\" \"delete $delname\" \"create $addname $D\"\n>  \tfi >commands &&\n>  \ttest_must_fail git update-ref --stdin <commands 2>output.err &&\n> -\tgrep \"fatal:\\( cannot lock ref $SQ$addname$SQ:\\)\\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ\" output.err &&\n> +\tgrep -E \"fatal:( cannot lock ref $SQ$addname$SQ:)? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ\" output.err &&\n>  \tprintf \"%s\\n\" \"$C $delref\" >expected-refs &&\n>  \tgit for-each-ref --format=\"%(objectname) %(refname)\" $prefix/r >actual-refs &&\n>  \ttest_cmp expected-refs actual-refs\n> \n"},{"id":"495296","messageId":"Zk3OfqglVgPYlvXU@telcontar","threadId":"61508","inReplyTo":"Zkw0ty8gOMS3Opzk@tanuki","subject":"Re: [PATCH v2] Switch grep from non-portable BRE to portable ERE","fromName":"Marcel Telka","fromEmail":"marcel@telka.sk","sentAt":"2024-05-22T10:52:46Z","receivedAt":"2024-05-22T10:52:57Z","isPatch":true,"sender":{"key":"marcel@telka.sk","avatar":"https://avatars.githubusercontent.com/u/3000173?v=4"},"body":"On Tue, May 21, 2024 at 07:44:23AM +0200, Patrick Steinhardt wrote:\n> On Fri, May 17, 2024 at 09:01:49PM +0200, Marcel Telka wrote:\n> > This makes the grep usage fully POSIX compliant.  The ability to\n> \n> Nit: we typically don't say \"This commit\", of which \"This\" is another\n> version. Instead, we use imperative style as if instructing the code to\n> change. Also, we typically first explain what the problem is before we\n> say how we fix it.\n> \n> I don't think this is worth a reroll though, it's only a hint for the\n> next patch you may be sending :)\n> \n> > enable ERE features in BRE using backslash is a GNU extension.\n> > \n> > Signed-off-by: Marcel Telka <marcel@telka.sk>\n> \n> It would have been nice if this thread was connected to the thread of\n> your first version so that it's easier to follow the discussion, e.g. by\n> using `--in-reply-to=` in git-send-email(1) or git-format-patch(1).\n\nThank you for hints.  Really appreciated.  This was my first attempt to\nuse the `git send-email` workflow so it is not surprising I missed the\n--in-reply-to= option :-).\n\n\nBest regards.\n\n-- \n+-------------------------------------------+\n| Marcel Telka   e-mail:   marcel@telka.sk  |\n|                homepage: http://telka.sk/ |\n+-------------------------------------------+\n"}]}