| Message ID | 20240305212533.12947-5-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/annotate-tests.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh > index 5e21e84f3884..87572459e4b8 100644 > --- a/t/annotate-tests.sh > +++ b/t/annotate-tests.sh > @@ -532,7 +532,7 @@ test_expect_success 'blame -L :funcname with userdiff driver' ' > "$(cat file.template)" && > test_commit --author "B <B@test.git>" \ > "change" "$fortran_file" \ > - "$(cat file.template | sed -e s/ChangeMe/IWasChanged/)" && > + "$(sed -e s/ChangeMe/IWasChanged/ file.template)" && Obviously correct, but "$(sed -e s/ChangeMe/IWasChanged/ <file.template)" && might be a more faithful conversion (when "sed" looks at its ARGV[], it did not find anything before, and it would not find anything after this patch). Not worth a reroll, of course, though.
On Tue, Mar 05, 2024 at 02:28:15PM -0800, Junio C Hamano wrote: > "Beat Bolli" <bb@drbeat.li> writes: > > > Signed-off-by: Beat Bolli <dev+git@drbeat.li> > > --- > > t/annotate-tests.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh > > index 5e21e84f3884..87572459e4b8 100644 > > --- a/t/annotate-tests.sh > > +++ b/t/annotate-tests.sh > > @@ -532,7 +532,7 @@ test_expect_success 'blame -L :funcname with userdiff driver' ' > > "$(cat file.template)" && > > test_commit --author "B <B@test.git>" \ > > "change" "$fortran_file" \ > > - "$(cat file.template | sed -e s/ChangeMe/IWasChanged/)" && > > + "$(sed -e s/ChangeMe/IWasChanged/ file.template)" && > > Obviously correct, but > > "$(sed -e s/ChangeMe/IWasChanged/ <file.template)" && > > might be a more faithful conversion (when "sed" looks at its ARGV[], > it did not find anything before, and it would not find anything > after this patch). Good point. Thank you for being careful. > > Not worth a reroll, of course, though.
Rubén Justo <rjusto@gmail.com> writes: > On Tue, Mar 05, 2024 at 02:28:15PM -0800, Junio C Hamano wrote: >> "Beat Bolli" <bb@drbeat.li> writes: >> >> > Signed-off-by: Beat Bolli <dev+git@drbeat.li> >> > --- >> > t/annotate-tests.sh | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh >> > index 5e21e84f3884..87572459e4b8 100644 >> > --- a/t/annotate-tests.sh >> > +++ b/t/annotate-tests.sh >> > @@ -532,7 +532,7 @@ test_expect_success 'blame -L :funcname with userdiff driver' ' >> > "$(cat file.template)" && >> > test_commit --author "B <B@test.git>" \ >> > "change" "$fortran_file" \ >> > - "$(cat file.template | sed -e s/ChangeMe/IWasChanged/)" && >> > + "$(sed -e s/ChangeMe/IWasChanged/ file.template)" && >> >> Obviously correct, but >> >> "$(sed -e s/ChangeMe/IWasChanged/ <file.template)" && >> >> might be a more faithful conversion (when "sed" looks at its ARGV[], >> it did not find anything before, and it would not find anything >> after this patch). > > Good point. Thank you for being careful. Heh, I actually consider it the most irrelevant one among my comments. I actally do not think there is a way tell if your "sed" invocation is reading from one of the files listed on the command line, or reading from the standard input, from your sed script, unlike say Perl that has access to @ARGV. Certainly a simple s/A/B/ would not care. Compared to that, rewriting $(cat file | wc -l) to $(wc -l <file) does matter, which was done in [05/22].
On Tue, Mar 05, 2024 at 04:26:27PM -0800, Junio C Hamano wrote: > Rubén Justo <rjusto@gmail.com> writes: > > > On Tue, Mar 05, 2024 at 02:28:15PM -0800, Junio C Hamano wrote: > >> "Beat Bolli" <bb@drbeat.li> writes: > >> > >> > Signed-off-by: Beat Bolli <dev+git@drbeat.li> > >> > --- > >> > t/annotate-tests.sh | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh > >> > index 5e21e84f3884..87572459e4b8 100644 > >> > --- a/t/annotate-tests.sh > >> > +++ b/t/annotate-tests.sh > >> > @@ -532,7 +532,7 @@ test_expect_success 'blame -L :funcname with userdiff driver' ' > >> > "$(cat file.template)" && > >> > test_commit --author "B <B@test.git>" \ > >> > "change" "$fortran_file" \ > >> > - "$(cat file.template | sed -e s/ChangeMe/IWasChanged/)" && > >> > + "$(sed -e s/ChangeMe/IWasChanged/ file.template)" && > >> > >> Obviously correct, but > >> > >> "$(sed -e s/ChangeMe/IWasChanged/ <file.template)" && > >> > >> might be a more faithful conversion (when "sed" looks at its ARGV[], > >> it did not find anything before, and it would not find anything > >> after this patch). > > > > Good point. Thank you for being careful. > > Heh, I actually consider it the most irrelevant one among my > comments. I actally do not think there is a way tell if your "sed" > invocation is reading from one of the files listed on the command > line, or reading from the standard input, from your sed script, > unlike say Perl that has access to @ARGV. Certainly a simple s/A/B/ > would not care. > > Compared to that, rewriting $(cat file | wc -l) to $(wc -l <file) > does matter, which was done in [05/22]. Yeah, that is needed; faithfulness is appreciated.
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index 5e21e84f3884..87572459e4b8 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -532,7 +532,7 @@ test_expect_success 'blame -L :funcname with userdiff driver' ' "$(cat file.template)" && test_commit --author "B <B@test.git>" \ "change" "$fortran_file" \ - "$(cat file.template | sed -e s/ChangeMe/IWasChanged/)" && + "$(sed -e s/ChangeMe/IWasChanged/ file.template)" && check_count -f "$fortran_file" -L:RIGHT A 3 B 1 '
Signed-off-by: Beat Bolli <dev+git@drbeat.li> --- t/annotate-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)