diff mbox series

[v3,02/22] apply-one-time-sed.sh: modernize style

Message ID 1fddaab7011dae7cdd15dee83a94b021085f6703.1574449072.git.liu.denton@gmail.com (mailing list archive)
State New, archived
Headers show
Series t: test cleanup stemming from experimentally enabling pipefail | expand

Commit Message

Denton Liu Nov. 22, 2019, 6:59 p.m. UTC
Convert `[ ... ]` to use `test`.

Move the `then`s onto their own lines so that it conforms with the
general test style.

Instead of redirecting input into sed, allow it to open its own input.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/lib-httpd/apply-one-time-sed.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Junio C Hamano Nov. 23, 2019, 1:32 a.m. UTC | #1
Denton Liu <liu.denton@gmail.com> writes:

> Convert `[ ... ]` to use `test`.
>
> Move the `then`s onto their own lines so that it conforms with the
> general test style.
>
> Instead of redirecting input into sed, allow it to open its own input.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  t/lib-httpd/apply-one-time-sed.sh | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

This one is new in this round.  As a conversion to match the style
guidelines, it looks OK, but the original feels a bit substandard in
other ways, e.g. I would have said "if one-time-sed is a file",
instead of "if one-time-sed exists as any kind of filesystem
entity", and used "cmp -s out out_modified" instead of "diff", which
is an overkill if you want to merely learn if two things are equal.

Wait.  If we are to see if A and B are the same, and show A when
they are the same and otherwise show B, wouldn't it be much simpler
to do without comparison and always show B unconditionally instead?

What am I missing?

Ah, there is one extra command in the "else" clause that we cannot
see in the post-context.  So, sorry for the noise---don't wait ;-)

But all the other things before the "Wait" still stands.

Thanks.

> diff --git a/t/lib-httpd/apply-one-time-sed.sh b/t/lib-httpd/apply-one-time-sed.sh
> index fcef728925..3e9a615311 100644
> --- a/t/lib-httpd/apply-one-time-sed.sh
> +++ b/t/lib-httpd/apply-one-time-sed.sh
> @@ -7,11 +7,13 @@
>  #
>  # This can be used to simulate the effects of the repository changing in
>  # between HTTP request-response pairs.
> -if [ -e one-time-sed ]; then
> +if test -e one-time-sed
> +then
>  	"$GIT_EXEC_PATH/git-http-backend" >out
> -	sed "$(cat one-time-sed)" <out >out_modified
> +	sed "$(cat one-time-sed)" out >out_modified
>  
> -	if diff out out_modified >/dev/null; then
> +	if diff out out_modified >/dev/null
> +	then
>  		cat out
>  	else
>  		cat out_modified
diff mbox series

Patch

diff --git a/t/lib-httpd/apply-one-time-sed.sh b/t/lib-httpd/apply-one-time-sed.sh
index fcef728925..3e9a615311 100644
--- a/t/lib-httpd/apply-one-time-sed.sh
+++ b/t/lib-httpd/apply-one-time-sed.sh
@@ -7,11 +7,13 @@ 
 #
 # This can be used to simulate the effects of the repository changing in
 # between HTTP request-response pairs.
-if [ -e one-time-sed ]; then
+if test -e one-time-sed
+then
 	"$GIT_EXEC_PATH/git-http-backend" >out
-	sed "$(cat one-time-sed)" <out >out_modified
+	sed "$(cat one-time-sed)" out >out_modified
 
-	if diff out out_modified >/dev/null; then
+	if diff out out_modified >/dev/null
+	then
 		cat out
 	else
 		cat out_modified