Subject: Re: [PATCH] t/t9902-completion.sh: backslashes in echo

From: Marcel Telka <[email-masked]>
Date: Fri, 24 May 2024 01:21:41 +0200

On Thu, May 23, 2024 at 04:03:54PM -0700, Junio C Hamano wrote:
> Marcel Telka <[email-masked]> 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.

Agreed.

> 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 ;-).

The commit message does not say that this change is to make the code
POSIX portable ;-).  It silently assumes the portability is related to
bash only.

-- 
+-------------------------------------------+
| Marcel Telka   e-mail:   [email-masked]  |
|                homepage: http://telka.sk/ |
+-------------------------------------------+