[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 17:27
> To: Gert Driesen
> Cc: mono-devel-list at ximian.com
> Subject: RE: [Mono-dev] [PATCH] Validation for <xsl:output> 
> attributes.
> 
> 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.

It also applies to "standalone" and "omit-xml-declaration" attributes (see
the new tests I added to XslTransformTests), and extranous attributes.

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

We still needed some additional changes to achieve forwards compatibility.
Patch attached.

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

Lol

One more question: apart from lack of support for forwards compatibility
(which is not directly related/limited to "indent" attribute), what was
buggy about my change for indent attribute ?

Gert

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