* [PATCH v2] Switch grep from non-portable BRE to portable ERE
@ 2024-05-17 19:01 Marcel Telka
2024-05-21 5:44 ` Patrick Steinhardt
0 siblings, 1 reply; 3+ messages in thread
From: Marcel Telka @ 2024-05-17 19:01 UTC (permalink / raw
To: git
This makes the grep usage fully POSIX compliant. The ability to
enable ERE features in BRE using backslash is a GNU extension.
Signed-off-by: Marcel Telka <marcel@telka.sk>
---
mergetools/vimdiff | 2 +-
t/t1404-update-ref-errors.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 734d15a03b..f8ad6b35d4 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -325,7 +325,7 @@ gen_cmd () {
fi
# If this is a single window diff with all the buffers
- if ! echo "$tab" | grep ",\|/" >/dev/null
+ if ! echo "$tab" | grep -E ",|/" >/dev/null
then
CMD="$CMD | silent execute 'bufdo diffthis'"
fi
diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 98e9158bd2..67ebd81a4c 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -100,7 +100,7 @@ df_test() {
printf "%s\n" "delete $delname" "create $addname $D"
fi >commands &&
test_must_fail git update-ref --stdin <commands 2>output.err &&
- grep "fatal:\( cannot lock ref $SQ$addname$SQ:\)\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
+ grep -E "fatal:( cannot lock ref $SQ$addname$SQ:)? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
printf "%s\n" "$C $delref" >expected-refs &&
git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
test_cmp expected-refs actual-refs
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Switch grep from non-portable BRE to portable ERE
2024-05-17 19:01 [PATCH v2] Switch grep from non-portable BRE to portable ERE Marcel Telka
@ 2024-05-21 5:44 ` Patrick Steinhardt
2024-05-22 10:52 ` Marcel Telka
0 siblings, 1 reply; 3+ messages in thread
From: Patrick Steinhardt @ 2024-05-21 5:44 UTC (permalink / raw
To: Marcel Telka; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]
On Fri, May 17, 2024 at 09:01:49PM +0200, Marcel Telka wrote:
> This makes the grep usage fully POSIX compliant. The ability to
Nit: we typically don't say "This commit", of which "This" is another
version. Instead, we use imperative style as if instructing the code to
change. Also, we typically first explain what the problem is before we
say how we fix it.
I don't think this is worth a reroll though, it's only a hint for the
next patch you may be sending :)
> enable ERE features in BRE using backslash is a GNU extension.
>
> Signed-off-by: Marcel Telka <marcel@telka.sk>
It would have been nice if this thread was connected to the thread of
your first version so that it's easier to follow the discussion, e.g. by
using `--in-reply-to=` in git-send-email(1) or git-format-patch(1).
But other than that the changes look good to me, thanks!
Patrick
> ---
> mergetools/vimdiff | 2 +-
> t/t1404-update-ref-errors.sh | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
> index 734d15a03b..f8ad6b35d4 100644
> --- a/mergetools/vimdiff
> +++ b/mergetools/vimdiff
> @@ -325,7 +325,7 @@ gen_cmd () {
> fi
>
> # If this is a single window diff with all the buffers
> - if ! echo "$tab" | grep ",\|/" >/dev/null
> + if ! echo "$tab" | grep -E ",|/" >/dev/null
> then
> CMD="$CMD | silent execute 'bufdo diffthis'"
> fi
> diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
> index 98e9158bd2..67ebd81a4c 100755
> --- a/t/t1404-update-ref-errors.sh
> +++ b/t/t1404-update-ref-errors.sh
> @@ -100,7 +100,7 @@ df_test() {
> printf "%s\n" "delete $delname" "create $addname $D"
> fi >commands &&
> test_must_fail git update-ref --stdin <commands 2>output.err &&
> - grep "fatal:\( cannot lock ref $SQ$addname$SQ:\)\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
> + grep -E "fatal:( cannot lock ref $SQ$addname$SQ:)? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
> printf "%s\n" "$C $delref" >expected-refs &&
> git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
> test_cmp expected-refs actual-refs
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Switch grep from non-portable BRE to portable ERE
2024-05-21 5:44 ` Patrick Steinhardt
@ 2024-05-22 10:52 ` Marcel Telka
0 siblings, 0 replies; 3+ messages in thread
From: Marcel Telka @ 2024-05-22 10:52 UTC (permalink / raw
To: Patrick Steinhardt; +Cc: git
On Tue, May 21, 2024 at 07:44:23AM +0200, Patrick Steinhardt wrote:
> On Fri, May 17, 2024 at 09:01:49PM +0200, Marcel Telka wrote:
> > This makes the grep usage fully POSIX compliant. The ability to
>
> Nit: we typically don't say "This commit", of which "This" is another
> version. Instead, we use imperative style as if instructing the code to
> change. Also, we typically first explain what the problem is before we
> say how we fix it.
>
> I don't think this is worth a reroll though, it's only a hint for the
> next patch you may be sending :)
>
> > enable ERE features in BRE using backslash is a GNU extension.
> >
> > Signed-off-by: Marcel Telka <marcel@telka.sk>
>
> It would have been nice if this thread was connected to the thread of
> your first version so that it's easier to follow the discussion, e.g. by
> using `--in-reply-to=` in git-send-email(1) or git-format-patch(1).
Thank you for hints. Really appreciated. This was my first attempt to
use the `git send-email` workflow so it is not surprising I missed the
--in-reply-to= option :-).
Best regards.
--
+-------------------------------------------+
| Marcel Telka e-mail: marcel@telka.sk |
| homepage: http://telka.sk/ |
+-------------------------------------------+
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-22 10:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-17 19:01 [PATCH v2] Switch grep from non-portable BRE to portable ERE Marcel Telka
2024-05-21 5:44 ` Patrick Steinhardt
2024-05-22 10:52 ` Marcel Telka
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).