| Message ID | 20240305212533.12947-20-dev+git@drbeat.li (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | avoid redundant pipelines | expand |
"Beat Bolli" <bb@drbeat.li> writes: > Signed-off-by: Beat Bolli <dev+git@drbeat.li> > --- > t/t8013-blame-ignore-revs.sh | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh > index 9a03b0f361ff..05213d13f30f 100755 > --- a/t/t8013-blame-ignore-revs.sh > +++ b/t/t8013-blame-ignore-revs.sh > @@ -25,11 +25,11 @@ test_expect_success setup ' > > git blame --line-porcelain file >blame_raw && > > - grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && > + sed -Ene "/^[0-9a-f]+ [0-9]+ 1/s/ .*//p" blame_raw >actual && Isn't -E a GNUism? At least, https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html does not seem to have it (we may need to fix t6030 to rid its only existing use).
Junio C Hamano wrote: > Isn't -E a GNUism? > > At least, > > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html > > does not seem to have it (we may need to fix t6030 to rid its only > existing use). I _thought_ that -r was the GNUism. The GNU sed-4.8 manpage says: -E, -r, --regexp-extended use extended regular expressions in the script (for portability use POSIX -E). That doesn't mean the man page is right, of course. :) https://www.austingroupbugs.net/view.php?id=528 suggests that -E has been adopted and, importanly, is more widely supported than -r (if we were considering using that rather than rewriting this to not use ERE syntax). MacOS in particular supports -E but not -r, according to that link. It seems like the documentation hasn't quite caught up to reality yet, perhaps?
Todd Zullinger <tmz@pobox.com> writes: > Junio C Hamano wrote: >> Isn't -E a GNUism? > ... > https://www.austingroupbugs.net/view.php?id=528 suggests > that -E has been adopted Then that is OK. Thanks for a good news. > and, importanly, is more widely > supported than -r (if we were considering using that rather > than rewriting this to not use ERE syntax). At least I wasn't, so it is irrelevant to this review, but it still is nice to know about it [*]. Thanks. [Footnote] * or is the knowledge of '-r' itself also irrelevant, now '-E' is kosher and widely usable?
On 06.03.24 03:17, Todd Zullinger wrote: > Junio C Hamano wrote: >> Isn't -E a GNUism? >> >> At least, >> >> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html >> >> does not seem to have it (we may need to fix t6030 to rid its only >> existing use). > > I _thought_ that -r was the GNUism. The GNU sed-4.8 manpage > says: > > -E, -r, --regexp-extended > use extended regular expressions in the script > (for portability use POSIX -E). > > That doesn't mean the man page is right, of course. :) > > https://www.austingroupbugs.net/view.php?id=528 suggests > that -E has been adopted and, importanly, is more widely > supported than -r (if we were considering using that rather > than rewriting this to not use ERE syntax). MacOS in > particular supports -E but not -r, according to that link. > > It seems like the documentation hasn't quite caught up to > reality yet, perhaps? > At least macOS Ventura and later supports "sed -E". I can't say what the more exotic platforms (NonStop?) have.
Beat Bolli <dev+git@drbeat.li> writes: > On 06.03.24 03:17, Todd Zullinger wrote: >> Junio C Hamano wrote: >>> Isn't -E a GNUism? >>> >>> At least, >>> >>> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html >>> >>> does not seem to have it (we may need to fix t6030 to rid its only >>> existing use). >> I _thought_ that -r was the GNUism. The GNU sed-4.8 manpage >> says: >> -E, -r, --regexp-extended >> use extended regular expressions in the script >> (for portability use POSIX -E). >> That doesn't mean the man page is right, of course. :) >> https://www.austingroupbugs.net/view.php?id=528 suggests >> that -E has been adopted and, importanly, is more widely > ... > At least macOS Ventura and later supports "sed -E". I can't say what > the more exotic platforms (NonStop?) have. More exotic platforms may lag behind, and it is a very valid concern. Also, the bug #528 does talk about -E being accepted, but wasn't it accepted for Issue 8, which is not finalized yet, no? So it seems it may be prudent to stick to BRE if we were to squash these pipelines into a single sed invocation, at least for a few years. Thanks.
diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh index 9a03b0f361ff..05213d13f30f 100755 --- a/t/t8013-blame-ignore-revs.sh +++ b/t/t8013-blame-ignore-revs.sh @@ -25,11 +25,11 @@ test_expect_success setup ' git blame --line-porcelain file >blame_raw && - grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 1/s/ .*//p" blame_raw >actual && git rev-parse X >expect && test_cmp expect actual && - grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 2/s/ .*//p" blame_raw >actual && git rev-parse X >expect && test_cmp expect actual ' @@ -53,11 +53,11 @@ do test_expect_success "ignore_rev_changing_lines ($I)" ' git blame --line-porcelain --ignore-rev $I file >blame_raw && - grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 1/s/ .*//p" blame_raw >actual && git rev-parse A >expect && test_cmp expect actual && - grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 2/s/ .*//p" blame_raw >actual && git rev-parse B >expect && test_cmp expect actual ' @@ -79,10 +79,10 @@ test_expect_success ignore_rev_adding_unblamable_lines ' git rev-parse Y >expect && git blame --line-porcelain file --ignore-rev Y >blame_raw && - grep -E "^[0-9a-f]+ [0-9]+ 3" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 3/s/ .*//p" blame_raw >actual && test_cmp expect actual && - grep -E "^[0-9a-f]+ [0-9]+ 4" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 4/s/ .*//p" blame_raw >actual && test_cmp expect actual ' @@ -92,11 +92,11 @@ test_expect_success ignore_revs_from_files ' git rev-parse Y >ignore_y && git blame --line-porcelain file --ignore-revs-file ignore_x --ignore-revs-file ignore_y >blame_raw && - grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 1/s/ .*//p" blame_raw >actual && git rev-parse A >expect && test_cmp expect actual && - grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 2/s/ .*//p" blame_raw >actual && git rev-parse B >expect && test_cmp expect actual ' @@ -106,11 +106,11 @@ test_expect_success ignore_revs_from_configs_and_files ' git config --add blame.ignoreRevsFile ignore_x && git blame --line-porcelain file --ignore-revs-file ignore_y >blame_raw && - grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 1/s/ .*//p" blame_raw >actual && git rev-parse A >expect && test_cmp expect actual && - grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 2/s/ .*//p" blame_raw >actual && git rev-parse B >expect && test_cmp expect actual ' @@ -121,10 +121,10 @@ test_expect_success override_ignore_revs_file ' git blame --line-porcelain file --ignore-revs-file "" --ignore-revs-file ignore_y >blame_raw && git rev-parse X >expect && - grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 1/s/ .*//p" blame_raw >actual && test_cmp expect actual && - grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 2/s/ .*//p" blame_raw >actual && test_cmp expect actual ' test_expect_success bad_files_and_revs ' @@ -279,11 +279,11 @@ test_expect_success ignore_merge ' test_merge M B && git blame --line-porcelain file --ignore-rev M >blame_raw && - grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 1/s/ .*//p" blame_raw >actual && git rev-parse B >expect && test_cmp expect actual && - grep -E "^[0-9a-f]+ [0-9]+ 9" blame_raw | sed -e "s/ .*//" >actual && + sed -Ene "/^[0-9a-f]+ [0-9]+ 9/s/ .*//p" blame_raw >actual && git rev-parse C >expect && test_cmp expect actual '
Signed-off-by: Beat Bolli <dev+git@drbeat.li> --- t/t8013-blame-ignore-revs.sh | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)