[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: 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.

> 
> 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 :(

> Don't mess svn history by doing things like that.

Ok.

Gert