patcht/t9902-completion.sh: backslashes in echo

> The usage of backslashes in echo is not portable.  Since some tests
> tries to output strings containing '\b' it is safer to use printf
> here.  The usage of printf instead of echo is also preferred by POSIX.
>
> Signed-off-by: Marcel Telka <marcel@telka.sk>
> ---
>  t/t9902-completion.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 963f865f27..ed3d03367e 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -73,7 +73,7 @@ _get_comp_words_by_ref ()
>  print_comp ()
>  {
>  	local IFS=$'\n'
> -	echo "${COMPREPLY[*]}" > out
> +	printf '%s\n' "${COMPREPLY[*]}" > out
>  }

> Marcel Telka <marcel@telka.sk> writes:
> 
> > The usage of backslashes in echo is not portable.  Since some tests
> > tries to output strings containing '\b' it is safer to use printf
> > here.  The usage of printf instead of echo is also preferred by POSIX.
> >
> > Signed-off-by: Marcel Telka <marcel@telka.sk>
> > ---
> >  t/t9902-completion.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> > index 963f865f27..ed3d03367e 100755
> > --- a/t/t9902-completion.sh
> > +++ b/t/t9902-completion.sh
> > @@ -73,7 +73,7 @@ _get_comp_words_by_ref ()
> >  print_comp ()
> >  {
> >  	local IFS=$'\n'
> > -	echo "${COMPREPLY[*]}" > out
> > +	printf '%s\n' "${COMPREPLY[*]}" > out
> >  }
> 
> This has cooked in 'next' for some time already, and I'll merge this
> down to 'master' anyway, but this being a script very much speicific
> to bash whose built-in echo we are using, the portability argument
> of "echo" made in the proposed log message does not quite apply to
> this patch.

> On Thu, May 23, 2024 at 10:59:13AM -0700, Junio C Hamano wrote:
> > Marcel Telka <marcel@telka.sk> writes:
> > > The usage of backslashes in echo is not portable.  Since some tests
> > > tries to output strings containing '\b' it is safer to use printf
> > > here.  The usage of printf instead of echo is also preferred by POSIX.
> >
> > This has cooked in 'next' for some time already, and I'll merge this
> > down to 'master' anyway, but this being a script very much speicific
> > to bash whose built-in echo we are using, the portability argument
> > of "echo" made in the proposed log message does not quite apply to
> > this patch.
>
> Could you please help me to understand how the bash is used to run the
> script?
>
> $ head -n 1 t/t9902-completion.sh
> #!/bin/sh

> On Thu, May 23, 2024 at 4:32 PM Marcel Telka <marcel@telka.sk> wrote:
> > On Thu, May 23, 2024 at 10:59:13AM -0700, Junio C Hamano wrote:
> > > Marcel Telka <marcel@telka.sk> writes:
> > > > The usage of backslashes in echo is not portable.  Since some tests
> > > > tries to output strings containing '\b' it is safer to use printf
> > > > here.  The usage of printf instead of echo is also preferred by POSIX.
> > >
> > > This has cooked in 'next' for some time already, and I'll merge this
> > > down to 'master' anyway, but this being a script very much speicific
> > > to bash whose built-in echo we are using, the portability argument
> > > of "echo" made in the proposed log message does not quite apply to
> > > this patch.
> >
> > Could you please help me to understand how the bash is used to run the
> > script?
> >
> > $ head -n 1 t/t9902-completion.sh
> > #!/bin/sh
> 
> Looking a bit farther in t9902, you'll find:
> 
>     . ./lib-bash.sh
> 
> And if you look inside `lib-bash.sh`, you'll see that, if the
> currently-running shell is not Bash, it aborts the current script and
> re-runs it under Bash.

> But Junio's point was that t9902 is dedicated to testing Bash-specific
> functionality, so the commit message's justification to avoid this
> non-POSIX behavior isn't necessarily a good justification for the
> change. (He wasn't saying that the change itself was unwelcome, just
> that the commit message wasn't convincing.)

> On Thu, May 23, 2024 at 11:47:13PM +0200, Marcel Telka wrote:
> > Interesting.  I came to this issue because the test failed here due to
> > the echo.  So apparently the re-run under bash didn't happen here for
> > some reason or bash's echo behaved differently?  I need to dig into it
> > again to find what's going on.
> 
> Okay, I found the cause.  Bash on OpenIndiana is compiled with
> --enable-xpg-echo-default so escape sequencies are expanded by default.
> 
> > OTOH, it seems that the fix is maybe not needed because it is solving a
> > non-problem.
> 
> There is a problem, but definitely the justification in the commit
> message is not accurate because we do not care about POSIX here at all.
> Also maybe it would be better/simpler to use `echo -E` instead of
> `printf`, but I'm not sure here.

> Okay, I found the cause.  Bash on OpenIndiana is compiled with
> --enable-xpg-echo-default so escape sequencies are expanded by default.
>
>> OTOH, it seems that the fix is maybe not needed because it is solving a
>> non-problem.
>
> There is a problem, but definitely the justification in the commit
> message is not accurate because we do not care about POSIX here at all.
> Also maybe it would be better/simpler to use `echo -E` instead of
> `printf`, but I'm not sure here.

> Marcel Telka <marcel@telka.sk> writes:
> > There is a problem, but definitely the justification in the commit
> > message is not accurate because we do not care about POSIX here at all.
> > Also maybe it would be better/simpler to use `echo -E` instead of
> > `printf`, but I'm not sure here.
>
> How "portable" is "echo -E"?  It apparently is not listd in [*], but
> it should probably not matter as we are doing this in bash.
>
> printf is a kosher way whose behaviour is pretty well standardized
> especially with respect to "%s".  As I said that I was going to
> merge it down to 'master' already, it is now part of 'master'.
> Use of printf there may hopefully educate folks to think twice
> before using 'echo' on unknown data.

> Marcel Telka <marcel@telka.sk> writes:
> 
> > Okay, I found the cause.  Bash on OpenIndiana is compiled with
> > --enable-xpg-echo-default so escape sequencies are expanded by default.
> >
> >> OTOH, it seems that the fix is maybe not needed because it is solving a
> >> non-problem.
> >
> > There is a problem, but definitely the justification in the commit
> > message is not accurate because we do not care about POSIX here at all.
> > Also maybe it would be better/simpler to use `echo -E` instead of
> > `printf`, but I'm not sure here.
> 
> How "portable" is "echo -E"?  It apparently is not listd in [*], but
> it should probably not matter as we are doing this in bash.

> printf is a kosher way whose behaviour is pretty well standardized
> especially with respect to "%s".  As I said that I was going to
> merge it down to 'master' already, it is now part of 'master'.
> Use of printf there may hopefully educate folks to think twice
> before using 'echo' on unknown data.
> 
> By the way, it makes me feel funny that we are talking about "POSIX
> portability" when reviewing a change like this:
> 
>          print_comp ()
>          {
>                 local IFS=$'\n'
>         -	echo "${COMPREPLY[*]}" > out
>         +	printf '%s\n' "${COMPREPLY[*]}" > out
>          }
> 
> With "${ARRAY[*]}", POSIX portability is totally out of the water
> anyway, regardless of the echo/printf issue ;-).