[Mono-devel-list] Re: Fixes to XSLT

[Atsushi Eno]

Hi,

Thanks to Kazuki, the bug you first posted was already fixed in svn.
Feel free to report another one which is reproducible (I wish one
which seems practical), with this patch, and I'll checkin it.

Thanks,
Atsushi Eno

Andrew Skiba wrote:
> Atsushi Eno wrote:
> 
>>> I agree with you that XPathResultType.Navigator is buggy. It must 
>>> have been called XPathResultType.NavigatorOrString from the beginning.
>>
>>
>> I never thought such enumeration could ever exist, while XSLT spec
>> explicitly shows there is a result tree fragment type.
> 
> Of course if we were at stage of designing framework, I would agree with 
> you from the beginning. But we are in situation where MS for some reason 
> (let's call it design bug) made those values same. It's very expensive 
> to fix this bug, as it will give you regressions like that temperature 
> sample, so MS will never do that. To stay incompatible with them in 
> long-term is strategically wrong. After all what we are doing here is 
> implementing THEIR framework, not defining what we like. If they will 
> listen to you and fix that in NET 2.0 i will be happy. But we have at 
> least a year with 1.1, so I think we should change Navigator to 1.
> 
>>> No, and that's the IMPORTANT moment. MS.NET users don't have to 
>>> handle Navigator separetely, as it's done automagically by the String 
>>> case. 
>>
>>
>> XSLT spec never automagically regards result tree fragment as string.
>> I think you are mixing specific code/stylesheet case and general spec
>> difference.
> 
> Correct me if I'm wrong, for XSLT author the next two fragments look the 
> same:
> 
> <xsl:variable name="foo" select=".//temperature[1]"/>
> 
> and
> 
> <xsl:variable name="foo">
>     <xsl:copy-of select=".//temperature[1]"/>
> </xsl:variable>
> 
> But the first one is Navigator and the second is String. May be this was 
> reason to unite them. I don't like such design solution, again, but it's 
> what we have.
> 
>> That is not my point. Note that I get that result without your patch.
>> (If I got the same result _with your patch applied_, what do you
>> think is my point?) I mean, even with XPathResultType.Navigator = 4,
>> the example code shown in the URL above did not show the actual
>> difference.
> 
> Sorry for confusion, here I talked about navigator.patch It will change 
> the result, try it ;-)
> 
>> What if users wrote custom functions that expects RTF and string
>> strictly different things? Won't they be caught in hard-to-find bugs?
> 
> In this case thay can use "is" operator to see if expression is 
> XPathNavigator, that's what I did on lines 54 and 57 of navigator.patch
> 
>>> The thing is, this bug disappears if you apply my navigator patch. 
>>> And you can only imagine, how many such bugs you have to find yet. 
>>
>>
>> Again, that happens without your patch, and that still happens even
>> with your patch.
> 
> I'm talking about navigator.patch, again.
> 
>> Ok, am really tiredof this niche discussion. If we continue, I'll just
>> change XPathResultType.Navigator to 1. It consumes my productivity
>> extraneously and I myself is not strict W3C believer. But when people
>> start to say that mono users want buggy implementation than W3C
>> conformant implementation, I'll just say "I did resist."
> 
> I'm agree. Let you accept navigator.patch and blame me if anybody 
> complains.
> 
> Thank you.
> Andrew Skiba.
> 
> 
> ------------------------------------------------------------------------
> 
> diff -Naur ./System.Xml.XPath/Expression.cs ../monopatched/System.Xml.XPath/Expression.cs
> --- ./System.Xml.XPath/Expression.cs	2005-02-06 17:27:16.000000000 +0200
> +++ ../monopatched/System.Xml.XPath/Expression.cs	2005-02-08 18:20:53.000000000 +0200
> @@ -463,9 +463,7 @@
>  				case XPathResultType.NodeSet:
>  					return XPathFunctions.ToNumber (EvaluateString (iter));
>  				case XPathResultType.String:
> -					return XPathFunctions.ToNumber ((string) result);
> -				case XPathResultType.Navigator:
> -					return XPathFunctions.ToNumber (((XPathNavigator) (result)).Value);
> +					return XPathFunctions.ToNumber (result.ToString ());
>  				default:
>  					throw new XPathException ("invalid node type");
>  			}
> @@ -484,7 +482,7 @@
>  				case XPathResultType.Boolean:
>  					return ((bool) result) ? "true" : "false";
>  				case XPathResultType.String:
> -					return (string) result;
> +					return result.ToString ();
>  				case XPathResultType.NodeSet:
>  				{
>  					BaseIterator iterResult = (BaseIterator) result;
> @@ -492,8 +490,6 @@
>  						return "";
>  					return iterResult.Current.Value;
>  				}
> -				case XPathResultType.Navigator:
> -					return ((XPathNavigator) result).Value;
>  				default:
>  					throw new XPathException ("invalid node type");
>  			}
> @@ -515,14 +511,12 @@
>  				case XPathResultType.Boolean:
>  					return (bool) result;
>  				case XPathResultType.String:
> -					return ((string) result).Length != 0;
> +					return (result.ToString ()).Length != 0;
>  				case XPathResultType.NodeSet:
>  				{
>  					BaseIterator iterResult = (BaseIterator) result;
>  					return (iterResult != null && iterResult.MoveNext ());
>  				}
> -				case XPathResultType.Navigator:
> -					return ((string) ((XPathNavigator) result).Value).Length != 0;
>  				default:
>  					throw new XPathException ("invalid node type");
>  			}
> @@ -640,9 +634,9 @@
>  				typeR = GetReturnType (_right.Evaluate (iter));
>  
>  			// Regard RTF as nodeset
> -			if (typeL == XPathResultType.Navigator)
> +			if (typeL == XPathResultType.Navigator && _left is XPathNavigator)
>  				typeL = XPathResultType.NodeSet;
> -			if (typeR == XPathResultType.Navigator)
> +			if (typeR == XPathResultType.Navigator && _right is XPathNavigator)
>  				typeR = XPathResultType.NodeSet;
>  
>  			if (typeL == XPathResultType.NodeSet || typeR == XPathResultType.NodeSet)
> diff -Naur ./System.Xml.XPath/XPathResultType.cs ../monopatched/System.Xml.XPath/XPathResultType.cs
> --- ./System.Xml.XPath/XPathResultType.cs	2004-09-12 11:57:49.000000000 +0300
> +++ ../monopatched/System.Xml.XPath/XPathResultType.cs	2005-02-08 18:21:08.000000000 +0200
> @@ -39,8 +39,7 @@
>  		String = 1,
>  		Boolean = 2,
>  		NodeSet = 3,
> -		[MonoFIX ("MS.NET: 1")]
> -		Navigator = 4,
> +		Navigator = 1,
>  		Any = 5,
>  		Error = 6,
>  	}