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

Gert Driesen gert.driesen at telenet.be
Sat Dec 24 13:25:56 EST 2005 

... and here's the patch ;-) 

Sorry for the noise !

> -----Original Message-----
> From: Gert Driesen [mailto:gert.driesen at telenet.be] 
> Sent: zaterdag 24 december 2005 19:24
> To: 'Atsushi Eno'
> Cc: 'mono-devel-list at ximian.com'
> Subject: RE: [Mono-dev] [PATCH] Validation for <xsl:output> 
> attributes.
> 
>  
> 
> > -----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
> > > 
> > 
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: xsl.diff
Type: application/octet-stream
Size: 3071 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20051224/f51b7f2d/attachment.obj

More information about the Mono-devel-list mailing list