Sorry for late response.
KO Myung-Hun <kom...@gmail.com> 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
