[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 14:48
> To: Gert Driesen
> Cc: mono-devel-list at ximian.com
> Subject: Re: [Mono-dev] [PATCH] Validation for <xsl:output> 
> attributes.
> 
> Gert Driesen wrote:
> >  
> > 
> >> -----Original Message-----
> >> From: Atsushi Eno [mailto:atsushi at ximian.com] 
> >> Sent: zaterdag 24 december 2005 11:16
> >> To: Gert Driesen
> >> Cc: mono-devel-list at ximian.com
> >> Subject: Re: [Mono-dev] [PATCH] Validation for <xsl:output> 
> >> attributes.
> >>
> >> Gert Driesen wrote:
> >>> I submitted my initial patch the the mono-dev list, and you 
> >> definitely
> >>> reviewed this part.
> >> It is incorrect. It was the first reply I precisely 
> pointed out that
> >> HTML output is broken here, and after my reply you committed what
> >> you haven't posted.
> >>
> >>> 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.
> >> The standalone tests deny what you say. Note that our 
> standalone tests
> >> are using whatever MS.NET outputs. Thus, there is 
> something your code
> >> does not match with MS.NET, or MS.NET has changed their 
> >> implementation.
> >>
> >> (BTW I never said that MS implementation is not forward 
> compatible.)
> > 
> > I guess MS does perform the same level of validation if the 
> version is not
> > equal to 1.0.
> > 
> > Problem is that I cannot seem to succeed in executing the 
> standalone XSLT
> > test suite :(
> 
> Yes; Sorry for the inconvenience. It just doesn't build fine under
> LF environment (you could still try cygwin environment as I as well
> as Mainsoft guys used to do).

No problem, I'll tests it later (after x-mas).

> I made a quick fix (attached) which is however untested under
> Windows (this time) since the latest svn seems broken to run
> NUnit tests.

I'm working on linux, so that's not a problem.

> After this patch there are still some failing tests
> which incorrectly expects Plants.xml and Outputtest.xml (you will
> understand what am saying here after seeing the test results).
> 
> >>> 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.
> >> What I asked is to fix the problem and commit. You might 
> have thought
> >> you *fixed* it, but it is not right. Thus am asking you to 
> revert it.
> >> I don't see any reason that you should stick to the broken changes.
> > 
> > I'll see if I can find time to look into the broken 
> changes. With broken I
> > mean broken compared to the MSFT implementation. 
> > 
> > Is that ok for you ? If not, it might be best to revert all 
> validation
> > changes.
> 
> Unless you find something soon, no, please just revert "indent"
> part from enum to string (it is the best that everyone would agree.)

It doesn't make sense to just revert the "indent" part. Forward
compatibility does not just apply to the intend attribute.

I've added some tests to XslTransformTests that "prove" this.  

Can you tell me how I can access the XSLT version (as defined on the
<stylesheet> document element) from XslOutput to make the validations
optional if the version is different from "1.0" ?
 
> >>> 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 ...
> >> There is no reason you should feel you are rejected. I just keep
> >> pointing out that your fix is not right.
> > 
> > Ok. Point taken.
> > 
> >> (I, of course, don't like the altitude that compatibility 
> is God and
> >> it can throw better behavior away. I'm not here to 
> reinvent MS bugs.)
> > 
> > I agree with you. We should not implement MS bugs, but we 
> should stick to
> > their implementation as close as possible (as this will 
> allow tests to pass
> > on both implementations, therefore allowing us to catch 
> regressions/changes
> > in both implementations).
> 
> I don't think you agreed with me on this case. You are still 
> saying that
> showing empty message is better than "XSLT compile error" 
> because it is
> MS compatible. Am not interested in "general" thoughts since I am sure
> that we will never agree.

You might be right on that one ! just kidding ;-)

I agree that an empty message is not good, but you must also agree that
creating an Exception with an empty message is not good (and should not be
done anyway). So, I really think this is an academical discussion. You don't
want an XsltCompileException with no message ? Ok, then just pass in a
message in the ctor. Easy, no ?

I agree that we should not duplicate bugs, but I don't think this is really
a bug ... but lets not start the discussion all over again ;-) lol

Gert