Re: [xslt] Patch for xsl:sort lang support



Nick Wellnhofer wrote:
Roumen Petrov wrote:
Roumen Petrov wrote:

Daniel Veillard wrote:
On Tue, Jun 10, 2008 at 02:05:12AM +0200, Nick Wellnhofer wrote:
Roumen Petrov wrote:
Nick Wellnhofer wrote:
Roumen Petrov wrote:
Please find patch that is first attempt to avoid build problem after recent commits related to xsltlocale.

I'm not convinced that xsltlocale.c is correct for msvcrt:
The manuals show that rfc3066 language tags are supported but the current code ignore all of them.
I took the language codes from here:
http://msdn.microsoft.com/en-us/library/39cwe7zf(VS.80).aspx
10x. I see that the _create_locale is related to setlocale.
Yes, _create_locale accepts the same parameters as setocale.

On this page http://msdn.microsoft.com/en-us/library/ms776260(VS.85).aspx
is listed another information but for other functions.
May be win32 locale has to be based on this.
No, the CRT functions don't seem to accept those language codes.

Also I think that _create_locale is missing before msvcrt80. The proposed patch don't resolve this in makefile based win32 build.
You're right, it seems it was introduced with VC8. Maybe it would be best to have a configure switch to disable locale support in case you want a build that runs without msvcr80.dll?
In the proposed patch configure will try detect presence of _create_locale if don't exist header xlocale.h , but not in makefile based build ( win32/Makefile.msvc ). May a new makefile win32/Makefile.msvcXX can ersolve problem.
Your patch looks good. See the attached version (also against current trunk) that additionally moves the defines to libxslt/xsltconfig.h and adds a switch to win32/configure.js.

I only wonder which setup uses GNU make and MSVCRT.

I wonder why the implementation assume that if exist xlocale.h the charmap is ascii.
Which charmap do you mean?
EBCDIC
I didn't think about that. Maybe one should add an appropriate #ifdef somewhere...

  Okay I applied and commited this patch which seems to integrate ideas
from both of you (i.e. i assume it superseeds Roumen's own libxslt-trunk-20080608.patch , right ?)
yes, it is fine with me (only code review).
(sorry for late reply but I'm too busy with other task and i don't have time to test).

  Is there anything still pending on this ?

I'm thinking about more backward compatible w32 implementation since the current require msvcrt80.
The xlocale based sort works fine.
But the related bug can be closed.

Daniel

Roumen


Please find attached file "libxslt-trunk-20080625.diff.gz" with w32 implementation based on winapi functions. It has to work on all NTs(nt4/w2k/xp/..). With some link quirks(not in the patch) it has to work on 9x(95/98/me).

Roumen

Looks very nice. I needed one little fix to get it to compile with the Microsoft C compiler and nmake. See the attached patch (on top of your patch) that also fixes two compiler warnings.

We might also want to keep the following checks in xsltLocaleStrCmp

if (str1 == str2) return(0);
if (str1 == NULL) return(-1);
if (str2 == NULL) return(1);

so that it behaves like xmlStrcmp with regard to NULL pointers.

Nick



Please find attached "libxslt-trunk-20080702.diff.gz" with suggested by Nick changes.

Roumen

Attachment: libxslt-trunk-20080702.diff.gz
Description: application/gzip



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]