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

[Gert Driesen]

> -----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: zaterdag 24 december 2005 3:47
> To: Gert Driesen
> Cc: mono-devel-list at lists.ximian.com
> Subject: Re: [Mono-dev] [PATCH] Validation for <xsl:output> 
> attributes.
> 
> Gert Driesen wrote:
> >  
> > 
> >> -----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: vrijdag 23 december 2005 19:29
> >> To: Gert Driesen
> >> Cc: mono-devel-list at lists.ximian.com
> >> Subject: RE: [Mono-dev] [PATCH] Validation for <xsl:output> 
> >> attributes.
> >>
> >>
> >>>> -----Original Message-----
> >>>> From: Atsushi Eno [mailto:atsushi at ximian.com] 
> >>>> Sent: vrijdag 23 december 2005 18:54
> >>>> To: Gert Driesen
> >>>> Cc: mono-devel-list at ximian.com
> >>>> Subject: RE: [Mono-dev] [PATCH] Validation for <xsl:output> 
> >>>> attributes.
> >>>>
> >>>>> 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.
> >>>>>
> >>>>>> 	- unindent cases in switches, i.e.
> >>>>>>
> >>>>>> 		switch (foo) {
> >>>>>> 		case bar:
> >>>>>> 			...
> >>>>> Done.
> >>>>>
> >>>>>> 	- The reason why you marked [NotWorking] on
> >>>>>> 	  XsltExceptionTests.Constructor2() is because
> >>>>>>
> >>>>>> 		xsltException = new XsltException 
> >> ((string) null,cause);
> >>>>>> 		Assert.AreEqual (string.Empty, 
> >> xsltException.Message);
> >>>>>> 	  "fails", right? Hmm, It's still okay to keep 
> >> this test, but
> >>>>>> 	  I don't think it is kind of thing we should 
> >> fix. Having empty
> >>>>>> 	  message for an exception does not make sense.
> >>>>>>
> >>>>>> 	  I guess, most of the reason in NotWorking are 
> >> like that. If
> >>>>>> 	  so, you don't have to file bugs for them. 
> >> Just add some
> >>>>>> 	  comments in the sources.
> >>>>> I've fixes these "bugs", and all tests now pass.
> >>>> Lemme say again, "it is not kind of thing we should fix". 
> >> After your
> >>>> fix,
> >>>> no one can understand what is going on from the Message property.
> >>> If you actually specify a zero-length or null message, then 
> >> this not really
> >>> that odd behaviour. No ?
> >> No. Your test reports like
> >>
> >> expected <null>
> >> but actually <"XSLT compile error ...">
> >>
> >> that is, it expects not to explain that it is XSLT compile 
> error, but
> >> not to tell
> >> anything. Why on earth it could be better?
> > 
> > I'm not saying that an empty (or null) message is better. 
> But I'm saying
> > that it makes sense that if you construct an exception with 
> a null or
> > zero-length message, you actually get a zero-length message.
> 
> You are saying the same idea with different words.
> 
> >> BTW it should not be the reason you could change code in 
> >> advance without
> >> further discussion. 
> > 
> > All I did was, instead of reporting a bug report, I fixed 
> the "issue"
> > myself. I got the impression that you did not consider it 
> to be an important
> > bug (and I agree that its not), and that you wanted me to 
> file a bug report
> > as a reminder (for when you'd have time to fix it). I 
> thought I'd help you
> > by "fixing" it myself ... But well, guess not :(
> 
> You wrote a lot of helpful tests here and there, but it's 
> not. Also this
> "bug" is usually not worthy of spending much words. You committed 
> changes without further discussion, which is a bad sign.

The only way to ensure compatibility, and to discover (undocumented) changes
between versions of .NET is by following the MS implementation as close as
possible. Only then can you have meaningful tests (that pass on both Mono
and MS.NET), which allow you to catch regressions/changes in either
implementation.

I don't want to waste more time of my vacation on this, so why don't you
just rollback all my changes ... I'm sure that would please you the most.

Regards

Gert