Re: [bug-gettext] [PATCH] OS/2 patches

[Daiki Ueno]

Sorry for late response.

KO Myung-Hun <address@hidden> writes:

>>> [PATCH 1/8] moop: Replace CR as well as NL with a space
>> 
>> I'm not sure who adds CR here.  Can't we assume that all source files
>> use NL line termination?  Or some other tools add CR on OS/2?  Also, I
>> worry about the portability of '\r' as an argument of tr.
>
> sed. On OS/2, many utils such as sed, coreutils and sh use CR+LF as a
> EOL marker instead of LF only.
>
> I replaced '\r' with '\015' in according to John Cowan

Thanks, but I'm still not sure if this is the right solution.

I generally avoid special handling of CR in scripts, to make the
maintenance easier.

So, couldn't the corresponding code:

  methods=`sed -e "$sed_remove_comments" < "$file" | sed -e 
"$sed_extract_methods" | tr '\n' ' ' | tr ';' '\n' | sed -e 's,[    ]*$,,'`

be translated into:

  methods=`sed -e "$sed_remove_comments" < "$file" | sed -e 
"$sed_extract_methods" | tr -d ';' | sed -e 's,[    ]*$,,'`

?

The results should be equivalent except empty lines, which will be
removed later on.

>>> [PATCH 5/8] intl: Support UNIXROOT in BINDTEXTDOMAIN () on EMX
>> 
>> Doesn't this conflict with the UNIXROOT resolution in os2compat.c?
>
> I removed the dependencies on os2compat.[ch] from kLIBC code paths at
> With [PATCH 3/8]. So this will not conflict with os2compat.c
>
> Even if EMX is used, it will be no problem because this patch check the
> condition strictly. That is, once os2compat.c resolved UNIXROOT, this
> patch does not prepend UNIXROOT.

That's good.  Maybe it would be worth mentioning as a comment?

>>> [PATCH 7/8] gettext-tools: Use a short name for DLLs on OS/2

+AM_CONDITIONAL([OS2], [test "${host_os#os2}" != "${host_os}"])

I guess it would be a little more portable to use a 'case "$host_os"
...' statement (like WOE32), instead of ${VAR#WORD} substitution.

Regards,
-- 
Daiki Ueno