Re: [gettext] a trivial patch

[Bruno Haible]

Stepan Kasal wrote:
> would you accept the following tiny simplification?

> @@ -37,15 +37,10 @@
>    dnl If the AC_CONFIG_AUX_DIR macro for autoconf is used we possibly
>    dnl find the mkinstalldirs script in another subdir but $(top_srcdir).
>    dnl Try to locate it.
> -  MKINSTALLDIRS=
> -  if test -n "$ac_aux_dir"; then
> -    case "$ac_aux_dir" in
> -      /*) MKINSTALLDIRS="$ac_aux_dir/mkinstalldirs" ;;
> -      *) MKINSTALLDIRS="\$(top_builddir)/$ac_aux_dir/mkinstalldirs" ;;
> -    esac
> -  fi
> -  if test -z "$MKINSTALLDIRS"; then
> -    MKINSTALLDIRS="\$(top_srcdir)/mkinstalldirs"
> -  fi
> +  case "$ac_aux_dir" in
> +    "") MKINSTALLDIRS="\$(top_srcdir)/mkinstalldirs" ;;
> +    /*) MKINSTALLDIRS="$ac_aux_dir/mkinstalldirs" ;;
> +    *)  MKINSTALLDIRS="\$(top_builddir)/$ac_aux_dir/mkinstalldirs" ;;
> +  esac
>    AC_SUBST(MKINSTALLDIRS)
>  ])

Your patch reduces the number of lines and is technically correct. But it
hampers the clarity of the shell code. In this place - autoconf macros
which have to satisfy constraints of multiple platforms, multiple versions
of autoconf, and backward compatibility - maintainability is key.

Here I have a comment "If the AC_CONFIG_AUX_DIR macro for autoconf is used ...".
If the code that follows it doesn't have an "if" statement that corresponds
to this comment, it is confusing. The code, as it is, represents the
intent more accurately: an "if", followed by the setting of a default value.

Bruno