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

On Thu, May 15, 2008 at 02:46:17PM +0200, Nick Wellnhofer wrote:
> Daniel Veillard wrote:
> >  okay, thanks a lot for doing this. This sounds a reasonnable solution
> >to that long-standing problem. The patch looks fine but there is just
> >one thing i would like to see changed: we should not use 
> >is*/tolower/toupper
> >in libxslt, because they are user locale dependant. Even if they are used
> >for test a priori in the ASCII range, there is a risk of confusion and
> >i would prefer to see the checks and normalizations done on the lang
> >attribute to use explicit 'a', 'z', 'A', 'Z' comparisons like in other
> >places of libxml2/libxslt.
> Attached is another version of the patch that addresses these issues. 
> This time against 1.1.24.

  Okay, thanks !
I finally took the time to apply your patch (no I hadn't forgotten :-)
it's now in SVN !
Just one thing, it would be nice to add a couple of simple example
to test taht this is working correctly in the regression tests, if you
have some that would be a good addition.

> >>The changes to namespaces.c and transform.c have nothing to do with this 
> >>patch. They are because of bug #377440: 
> >>
> >
> >  Okay that need to be looked at separately. And hence as a separate patch.
> >I think the behaviour given in comment #2 is normal. If in the input 
> >stylesheet
> >you have an explicit namespace definition, and no exclude-result-prefixes,
> >you have to put them back in scope (and the reason is that the namespace 
> >prefix may be used inside attribute values or inside content, so it may
> >be used by the result iven if you can't guess that just by the analysis
> >of the output structure, it's bad design but legal).
> >  Apparently based on comment #3 the behaviour is actually correct for
> >both #2 and #3 examples.
> >  The question in the comment block of #4 is an open one. xmlSearchNsByHref
> >may be a bit too strong, and your suggestion on the default namespace 
> >sounds
> >more reasonnable to me than the full recursive search.
> I know that the current behavior of libxslt is completely per spec, but 
> it would be nice if libxslt could check the default namespace at least. 
> I removed my changes from the patch.

  Okay, can you provide a separate patch with just the test for the
default namespace, I will apply this too,

  thanks again !


Red Hat Virtualization group
Daniel Veillard      | virtualization library
veillard redhat com  | libxml GNOME XML XSLT toolkit | Rpmfind RPM search engine

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