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

[Atsushi Eno]

> > -----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?

BTW it should not be the reason you could change code in advance without
further discussion. Don't mess svn history by doing things like that.

Atsushi Eno