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

[Gert Driesen]

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.

> Please commit the patch after fixing them.

Committed in svn (revision 54780).

Gert