Re: [PATCH] t9001: avoid not portable '\n' with sed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2014-06-04 20.13, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
> 
>> Torsten Bögershausen <tboegi@xxxxxx> writes:
>>
>>> t9001 used a '\n' in a sed expression to split one line into two lines.
>>> Some versions of sed simply ignore the '\' before the 'n', treating
>>> '\n' as 'n'.
>>>
>>> As the test already requires perl as a prerequisite, use perl instead of sed.
>>>
>>> Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
>>> ---
>>
>> Hmph.  I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
>>
>>     The escape sequence '\n' shall match a <newline> embedded in the
>>     pattern space.
>>
>> so it may be better to be a bit more explicit in the log message to
>> say whose implementation has this issue to warn people.
>>
>>>  t/t9001-send-email.sh | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
>>> index 64d9434..2bf48d1 100755
>>> --- a/t/t9001-send-email.sh
>>> +++ b/t/t9001-send-email.sh
>>> @@ -1342,7 +1342,7 @@ test_cover_addresses () {
>>>  	git format-patch --cover-letter -2 -o outdir &&
>>>  	cover=`echo outdir/0000-*.patch` &&
>>>  	mv $cover cover-to-edit.patch &&
>>> -	sed "s/^From:/$header: extra@xxxxxxxxxxx\nFrom:/" cover-to-edit.patch >"$cover" &&
>>> +	"$PERL_PATH" -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch | tr Q "$LF" >"$cover" &&
>>
>> We have a shell function "perl" in test-lib-function.sh these days
>> so that you do not have to write "$PERL_PATH" yourself in tests ;-)
> 
> Also, piping output from perl to tr feels somewhat suboptimal.  I do
> not see where in the test material we use "Q to LF", and we may want
> to remove that altogether, but without that removal, an updated
> patch may look like this.
> 
>  t/t9001-send-email.sh | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 64d9434..9f06b8c 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1342,7 +1342,10 @@ test_cover_addresses () {
>  	git format-patch --cover-letter -2 -o outdir &&
>  	cover=`echo outdir/0000-*.patch` &&
>  	mv $cover cover-to-edit.patch &&
> -	sed "s/^From:/$header: extra@xxxxxxxxxxx\nFrom:/" cover-to-edit.patch >"$cover" &&
> +	perl -pe "
> +		s/^From:/$header: extra\@address.com\nFrom:/;
> +		y/Q/\n/;
> +	" cover-to-edit.patch >"$cover" &&
>  	git send-email \
>  	  --force \
>  	  --from="Example <nobody@xxxxxxxxxxx>" \
Good catch, the "tr" should had been removed.
My first version used
sed "s/^From:/$header: extra@address.comQFrom:/"
and the Q was replaced by tr with a literal LF.
So I think the 'Q' -> '\n' conversion should be removed completely :-)

The sed in question is /usr/bin/sed under Mac OS X.
Then we have the question: What exactly is the pattern space?

>In default operation, sed cyclically shall append a line of input, less its terminating <newline> >character, into the pattern space....

Isn't that the stuff from the input?
But that doesn't make too much sence to me, since "input lines" are terminated by \n.
So pattern space seems to mean output when they talk about the \n
Anyway, the \n (to insert a newline into the output) works under Linux, but not Mac OS.




 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]