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

[Atsushi Eno]

It indeed is "indent" part matter. Anyways you don't have to touch
the code anymore. I fixed the bug. There is no failing NUnit tests
and no more regressions in standalone tests now.

As I said, checking attributes on xsl:output and reject extraneous
ones is indeed good, so I didn't want to revert the entire changes.

So, now that all problems should have gone, I hope you have a merry
X'mas (it's a bit too late for me and Santa does not seem coming to
this bad boy ;-)

Atsushi Eno

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