[Mono-dev] [PATCH] Validation for <xsl:output> attributes.

[Atsushi Eno]

Atsushi Eno wrote:
> Gert Driesen wrote:
>>  
>>
>>> -----Original Message-----
>>> From: Atsushi Eno [mailto:atsushi at ximian.com] Sent: vrijdag 23 
>>> december 2005 20:31
>>> To: Gert Driesen
>>> Cc: mono-devel-list at ximian.com
>>> Subject: Re: [Mono-dev] [PATCH] Validation for <xsl:output> attributes.
>>>
>>> Gert Driesen wrote:
>>>> Comments inline
>>>>> -----Original Message-----
>>>>> From: mono-devel-list-bounces at lists.ximian.com 
>>>>> [mailto:mono-devel-list-bounces at lists.ximian.com] On Behalf Of 
>>>>> Atsushi Eno
>>>>> Sent: dinsdag 20 december 2005 6:26
>>>>> To: Gert Driesen
>>>>> Cc: mono-devel-list at ximian.com
>>>>> Subject: Re: [Mono-dev] [PATCH] Validation for <xsl:output> 
>>>>> attributes.
>>>>>
>>>>> Hi,
>>>>>
>>>>>> The attached patch implements validation for <xsl:output> 
>>>>> attributes, and
>>>>>> adds unit tests.
>>>>> Thanks!
>>>>>
>>>>>> I've also added some unit tests for XsltCompileException 
>>>>> and XslException.
>>>>>> Some test are marked NotWorking, due to bugs in Mono (for 
>>>>> which I'll report
>>>>>> bug reports later).
>>>>>>
>>>>> Some comments:
>>>>>
>>>>>     - You can try Mainsoft XSLT standalone tests. Go to
>>>>>       Test/System.Xml.Xsl/standalone and run "make run-test", then
>>>>>       you can find some regressions.
>>>>>     - Your code that checks attributes is good.
>>>>>     - "indent" in xsl:output is "yes" by default when the output
>>>>>       method is "html", unlike when it is "xml" ("no"). That's why
>>>>>       we have string value instead of boolean in XslOutput class.
>>>> I now use an enum for this internally, which allows us to 
>>> continue exposing
>>>> Intend as a bool.
>>> Please checkin this change as well. Some of Mainsoft XSLT tests
>>> started to fail after r54780.
>>
>> Hmm, that change was part of r54780. I tried to run the Mainsoft XSLT 
>> tests,
>> but patching of testsuite/TESTS/catalog-fixed.xml failed for some 
>> reason :(
> 
> Then you could just ask me to see it, instead of ignoring it.
> 
>> I've just added two more tests for XML/HTML indentation (indent explictly
>> set to yes/no, and default value) that work fine on both Mono and MS.NET
>> 1.x/2.0.
>>
>> Can you tell me what failures you get in the Mainsoft XSLT tests ?
> 
> 1) output_output05 : Different.
> 2) BVTs_bvt098 : Different.
> 3) ForwardComp__91848 : Unexpected exception:
> 
> Especially 3) tells that previous solution was better (did you ever
> think about forward compatibility?):
> 
> System.Xml.Xsl.XsltCompileException: 
> file:///C:/cygwin/home/atsushi/svn/mcs/class/System.XML/Test/System.Xml.Xsl/s 
> 
> tandalone_tests/testsuite/TESTS/MSFT_Conformance_Tests/ForwardComp/91848.xsl(4,40) 
> :
>  ---> System.Xml.Xsl.XsltException: 'sure' is an invalid value for 
> 'indent' attribute.--- End of inner exception stack trace ---

Forgot to say: I'm suggesting not to have another enumeration, but to 
revert the code as it was.

Another reason that string is better than enumeration (like
Yes/No/Default/Other I guess) is that it becomes a mess when someone
or ourself want to reuse the code to implement his or her own XSLT
implementation that supports custom output.

Atsushi Eno