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

[Gert Driesen]

> -----Original Message-----
> From: Atsushi Eno [mailto:atsushi at ximian.com] 
> Sent: zaterdag 24 december 2005 3:54
> To: Gert Driesen
> Cc: mono-devel-list at ximian.com
> Subject: Re: [Mono-dev] [PATCH] Validation for <xsl:output> 
> attributes.
> 
> 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/S
> ystem.Xml.Xsl/s 
> > 
> > 
> tandalone_tests/testsuite/TESTS/MSFT_Conformance_Tests/Forward
> Comp/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.

I submitted my initial patch the the mono-dev list, and you definitely
reviewed this part.

Our behaviour now matches that of MSFT, is that bad ? We now have unit tests
that validate our behaviour and that of MSFT (as these unit tests now pass
on both implementations).

You're saying that the MSFT implementation is not forward compatible, so I'd
suggest filing a bug report with them. If they ever change the
implementation, you'll immediately know as the unit tests will start
failing.

If our implementation does not match that of MSFT, then you can't have any
unit tests that allow you to discover this.

> 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.µ

Again, I just followed the behaviour of the MS implementation here, and got
your approval for the validation changes.

I really must be missing something here. If you don't want me to work on
this, you could've said so from the start ...

Gert