Re: [PATCHv3] m4sugar: factor away _AS_ECHO_PREPARE.

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 :)
> 
> Okay to push?
> 
> "printf '%s\n' ..." has been a fine replacement for plain "echo"
> for at least 5 years (probably more like 10), even with most
> museum-piece shells.
> * lib/m4sugar/m4sh.m4 (_AS_ECHO_PREPARE): Remove.
> (_AS_SHELL_SANITIZE): Keep as_nl setting originally from
> _AS_ECHO_PREPARE here where it more properly belongs.
> (AS_ECHO, AS_ECHO_N): Use printf unconditionally.
> * doc/autoconf.texi (Limitations of Shell Builtins): Document
> preference for 'printf' over working around 'echo' bugs.
> * NEWS: Updated.
> Reported by Jim Meyering.
> ---
>  NEWS                |  4 ++++
>  doc/autoconf.texi   | 19 +++++++----------
>  lib/m4sugar/m4sh.m4 | 60 +++++++----------------------------------------------
>  3 files changed, 19 insertions(+), 64 deletions(-)
>
> [SNIP]
>
> diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
> index 25b4fe3..479c2d9 100644
> --- a/lib/m4sugar/m4sh.m4
> +++ b/lib/m4sugar/m4sh.m4
> @@ -453,7 +453,6 @@ m4_defun([_AS_SHELL_SANITIZE],
>  [m4_text_box([M4sh Initialization.])
>  
>  AS_BOURNE_COMPATIBLE
> -_AS_ECHO_PREPARE
>  _AS_PATH_SEPARATOR_PREPARE
>  
>  # IFS
> @@ -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?

>  IFS=" ""	$as_nl"
>  
>  # Find who we are.  Look in the path if we contain no directory separator.
> @@ -1023,63 +1025,15 @@ fi
>  # Output WORD followed by a newline.  WORD must be a single shell word
>  # (typically a quoted string).  The bytes of WORD are output as-is, even
>  # if it starts with "-" or contains "\".
> -m4_defun_init([AS_ECHO],
> -[AS_REQUIRE([_$0_PREPARE])],
> -[$as_echo $1])
> +m4_defun([AS_ECHO],
> +[printf '%s\n' $1])
>  
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.

> [SNIP]
>
> +m4_defun([AS_ECHO_N],
> +[printf %s $1])
>  
Similarly, with this, an invocation like:

  AS_ECHO_N([1 2 3])

will generate code that prints:

  123

rather than the (IMHO) expected:

  1 2 3

Again, this is *not* a regression, but would be nice to fix it
nonetheless with a follow-up.

Regards,
  Stefano