Re: [PATCHv3] m4sugar: factor away _AS_ECHO_PREPARE.

On 01/29/2013 08:59 PM, Eric Blake wrote:
> On 01/29/2013 11:26 AM, Stefano Lattarini wrote:
>> Hello everybody, sorry for the late review.
>>
>> On 01/29/2013 07:17 AM, Gary V. Vaughan wrote:
>>> Incorporating feedback from Paul and Paul.  Thank you both :)
>>>
> 
>>> @@ -461,6 +460,9 @@ _AS_PATH_SEPARATOR_PREPARE
>>>  # there to prevent editors from complaining about space-tab.
>>>  # (If _AS_PATH_WALK were called with IFS unset, it would disable word
>>>  # splitting by setting IFS to empty value.)
>>> +as_nl='
>>> +'
>>> +export as_nl
>>>
>> Why this export?
> 
> Because _AS_ECHO_PREPARE used to export it as well.
>
OK, I missed that.  Sorry.

> Removing the export might break behavior.
>
Indeed.  Adding a comment stating this non-obvious [1] rationale
might be worthwhile, I think; I'll send a patch tomorrow if nobody
beats me.

[1] Non-obvious for someone that will read the code in the
    feature, without seeing this patch or going through the
    Git history.

>>>  
>> This won't work as expected with some invocation like:
>>
>>   AS_ECHO([1 2 3])
>>
>> as the generated code will print:
>>
>>   1
>>   2
>>   3
>>
>> rather than the (IMHO) expected:
>>
>>   1 2 3
>>
>> This is *not* a regression, since this issue was already in the
>> existing code; but it would be nice to have it fixed in a follow-up
>> patch.
> 
> No, I don't see any reason to change long-standing behavior,
>
Note that the old behavior was actually undefined, since the exact output
would have depended on whether print, printf or echo was selected to be
run by AS_ECHO.  But then ...

> since no
> one should be relying on the behavior when passing more than one shell
> word anyway.
> 
.. this is a valid objection (and as Nick pointed out, this limitation
was already documented in the manual); so we have no bug actually.
Sorry for the noise.

Regards,
  Stefano