RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: zondag 25 december 2005 4:45 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: RE: [Mono-dev] [PATCH] Validation for xsl:output attributes. ... and here's the patch ;-) Sorry for the noise ! It looks good. Please apply exactly the same patch by the time you commit anything else. Yeah, sure. I'll be a good boy ;-) Gert ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: zondag 25 december 2005 4:44 To: Gert Driesen Cc: [EMAIL PROTECTED] 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. Mmm, did you ever mention it when you posted the patch and before committing your massive changes which were mostly just changing CRLF/LF and message format changes in SVN? Nope, I didn't know this then. Did you mention anything about forwards compatiblity back then ? 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 ? Am too lazy so I just paste it again. 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. You could have just run standalone tests and see what happens. Yes, but they didn't work. And the tests I added worked fine on both MS.NET 1.x/2.0 and Mono now, but I didn't know about forwards compatibility ... Gert ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Validation for xsl:output attributes.
Gert Driesen wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: zondag 25 december 2005 4:44 To: Gert Driesen Cc: [EMAIL PROTECTED] 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. Mmm, did you ever mention it when you posted the patch and before committing your massive changes which were mostly just changing CRLF/LF and message format changes in SVN? Nope, I didn't know this then. Did you mention anything about forwards compatiblity back then ? Yes: Can you tell me what failures you get in the Mainsoft XSLT tests ? 1) output_output05 : Different. 2) BVTs_bvt098 : Different. 3) ForwardComp__91848 : Unexpected exception: Especially 3) tells that previous solution was better (did you ever think about forward compatibility?): You could have just run standalone tests and see what happens. Yes, but they didn't work. And the tests I added worked fine on both MS.NET 1.x/2.0 and Mono now, but I didn't know about forwards compatibility ... It had already been fixed before the last reply. Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: zondag 25 december 2005 15:39 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output attributes. Gert Driesen wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: zondag 25 december 2005 4:44 To: Gert Driesen Cc: [EMAIL PROTECTED] 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. Mmm, did you ever mention it when you posted the patch and before committing your massive changes which were mostly just changing CRLF/LF and message format changes in SVN? Nope, I didn't know this then. Did you mention anything about forwards compatiblity back then ? Yes: Can you tell me what failures you get in the Mainsoft XSLT tests ? 1) output_output05 : Different. 2) BVTs_bvt098 : Different. 3) ForwardComp__91848 : Unexpected exception: That was after you had approved my changes, but well ... (and no you did not give approval for changes to Xslt(Compile)Exception, but that has nothing to do with the forwards compatibility regression). No need to keep going about this though ... Especially 3) tells that previous solution was better (did you ever think about forward compatibility?): You could have just run standalone tests and see what happens. Yes, but they didn't work. And the tests I added worked fine on both MS.NET 1.x/2.0 and Mono now, but I didn't know about forwards compatibility ... It had already been fixed before the last reply. I know, but after I had committed my changes. Gert ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Validation for xsl:output attributes.
Gert Driesen wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: zondag 25 december 2005 15:39 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output attributes. Gert Driesen wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: zondag 25 december 2005 4:44 To: Gert Driesen Cc: [EMAIL PROTECTED] 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. Mmm, did you ever mention it when you posted the patch and before committing your massive changes which were mostly just changing CRLF/LF and message format changes in SVN? Nope, I didn't know this then. Did you mention anything about forwards compatiblity back then ? Yes: Can you tell me what failures you get in the Mainsoft XSLT tests ? 1) output_output05 : Different. 2) BVTs_bvt098 : Different. 3) ForwardComp__91848 : Unexpected exception: That was after you had approved my changes, but well ... (and no you did not give approval for changes to Xslt(Compile)Exception, but that has nothing to do with the forwards compatibility regression). Of course when I wrote the first reply to you I couldn't expect that you jumped to svn commit despite my first comment, which should have led anyone to see the standalone test results (or report that he or she could not run it): - 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. Xslt(Compile)Exception stuff is nothing to do with it. I'm not wasting my time. If the community wants silly exception in the name of compatibility then I'll left it as is. No need to keep going about this though ... Yes, you don't have to write further reply. Especially 3) tells that previous solution was better (did you ever think about forward compatibility?): You could have just run standalone tests and see what happens. Yes, but they didn't work. And the tests I added worked fine on both MS.NET 1.x/2.0 and Mono now, but I didn't know about forwards compatibility ... It had already been fixed before the last reply. I know, but after I had committed my changes. Without asking anymore, yes. I'll go back to my vacation hacking rather than something that is done in my work-time. Bye. Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: zaterdag 24 december 2005 3:47
To: Gert Driesen
Cc: mono-devel-list@lists.ximian.com
Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output
attributes.
Gert Driesen wrote:
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: vrijdag 23 december 2005 19:29
To: Gert Driesen
Cc: mono-devel-list@lists.ximian.com
Subject: RE: [Mono-dev] [PATCH] Validation for xsl:output
attributes.
-Original Message-
From: Atsushi Eno [mailto:[EMAIL PROTECTED]
Sent: vrijdag 23 december 2005 18:54
To: Gert Driesen
Cc: [EMAIL PROTECTED]
Subject: RE: [Mono-dev] [PATCH] Validation for xsl:output
attributes.
Comments inline
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: dinsdag 20 december 2005 6:26
To: Gert Driesen
Cc: [EMAIL PROTECTED]
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.
You are saying the same idea with different words.
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 :(
You wrote a lot of helpful tests here and there, but it's
not. Also this
bug is usually not worthy of spending much words. You committed
changes without further discussion, which is a bad sign.
The only way to ensure compatibility, and to discover (undocumented) changes
between versions of .NET is by following the MS implementation as close as
possible. Only then can you have meaningful tests (that pass on both Mono
and MS.NET), which allow you to catch regressions/changes in either
implementation.
I don't want to waste more time of my vacation on this, so why don't you
just rollback all my changes ... I'm sure that would please you the most.
Regards
Gert
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
Comments inline
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: dinsdag 20 december 2005 6:26
To: Gert Driesen
Cc: [EMAIL PROTECTED]
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.
Please commit the patch after fixing them.
Committed in svn (revision 54780).
Gert
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message-
From: Atsushi Eno [mailto:[EMAIL PROTECTED]
Sent: vrijdag 23 december 2005 18:54
To: Gert Driesen
Cc: [EMAIL PROTECTED]
Subject: RE: [Mono-dev] [PATCH] Validation for xsl:output
attributes.
Comments inline
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: dinsdag 20 december 2005 6:26
To: Gert Driesen
Cc: [EMAIL PROTECTED]
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 ?
Gert
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Validation for xsl:output attributes.
Gert Driesen wrote: Comments inline -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: dinsdag 20 december 2005 6:26 To: Gert Driesen Cc: [EMAIL PROTECTED] 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. Please checkin this change as well. Some of Mainsoft XSLT tests started to fail after r54780. Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: vrijdag 23 december 2005 20:31 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output attributes. Gert Driesen wrote: Comments inline -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: dinsdag 20 december 2005 6:26 To: Gert Driesen Cc: [EMAIL PROTECTED] 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. Please checkin this change as well. Some of Mainsoft XSLT tests started to fail after r54780. Hmm, that change was part of r54780. I tried to run the Mainsoft XSLT tests, but patching of testsuite/TESTS/catalog-fixed.xml failed for some reason :( I've just added two more tests for XML/HTML indentation (indent explictly set to yes/no, and default value) that work fine on both Mono and MS.NET 1.x/2.0. Can you tell me what failures you get in the Mainsoft XSLT tests ? Sorry if I my changes caused regressions, but I really added lots of tests to make sure that they did not :(:( Gert ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Validation for xsl:output attributes.
Gert Driesen wrote: -Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: vrijdag 23 december 2005 20:31 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output attributes. Gert Driesen wrote: Comments inline -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: dinsdag 20 december 2005 6:26 To: Gert Driesen Cc: [EMAIL PROTECTED] 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. Please checkin this change as well. Some of Mainsoft XSLT tests started to fail after r54780. Hmm, that change was part of r54780. I tried to run the Mainsoft XSLT tests, but patching of testsuite/TESTS/catalog-fixed.xml failed for some reason :( Then you could just ask me to see it, instead of ignoring it. I've just added two more tests for XML/HTML indentation (indent explictly set to yes/no, and default value) that work fine on both Mono and MS.NET 1.x/2.0. Can you tell me what failures you get in the Mainsoft XSLT tests ? 1) output_output05 : Different. 2) BVTs_bvt098 : Different. 3) ForwardComp__91848 : Unexpected exception: Especially 3) tells that previous solution was better (did you ever think about forward compatibility?): System.Xml.Xsl.XsltCompileException: file:///C:/cygwin/home/atsushi/svn/mcs/class/System.XML/Test/System.Xml.Xsl/s tandalone_tests/testsuite/TESTS/MSFT_Conformance_Tests/ForwardComp/91848.xsl(4,40) : --- System.Xml.Xsl.XsltException: 'sure' is an invalid value for 'indent' attribute.--- End of inner exception stack trace --- Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
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. 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:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 14:48 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output attributes. Gert Driesen wrote: -Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 11:16 To: Gert Driesen Cc: [EMAIL PROTECTED] 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
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.) 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 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. (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.) Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 11:16 To: Gert Driesen Cc: [EMAIL PROTECTED] 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 :( 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. 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). Gert ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 17:27 To: Gert Driesen Cc: [EMAIL PROTECTED] 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:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 14:48 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output attributes. Gert Driesen wrote: -Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 11:16 To: Gert Driesen Cc: [EMAIL PROTECTED] 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
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 14:48 To: Gert Driesen Cc: [EMAIL PROTECTED] Subject: Re: [Mono-dev] [PATCH] Validation for xsl:output attributes. Gert Driesen wrote: -Original Message- From: Atsushi Eno [mailto:[EMAIL PROTECTED] Sent: zaterdag 24 december 2005 11:16 To: Gert Driesen Cc: [EMAIL PROTECTED] 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
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. Mmm, did you ever mention it when you posted the patch and before committing your massive changes which were mostly just changing CRLF/LF and message format changes in SVN? 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 ? Am too lazy so I just paste it again. 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. You could have just run standalone tests and see what happens. Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
... and here's the patch ;-) Sorry for the noise ! It looks good. Please apply exactly the same patch by the time you commit anything else. Thanks, Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
RE: [Mono-dev] [PATCH] Validation for xsl:output attributes.
-Original Message-
From: Atsushi Eno [mailto:[EMAIL PROTECTED]
Sent: vrijdag 23 december 2005 18:54
To: Gert Driesen
Cc: [EMAIL PROTECTED]
Subject: RE: [Mono-dev] [PATCH] Validation for xsl:output
attributes.
Comments inline
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: dinsdag 20 december 2005 6:26
To: Gert Driesen
Cc: [EMAIL PROTECTED]
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?
BTW it should not be the reason you could change code in advance without
further discussion. Don't mess svn history by doing things like that.
Atsushi Eno
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Validation for xsl:output attributes.
Gert Driesen wrote:
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: vrijdag 23 december 2005 19:29
To: Gert Driesen
Cc: mono-devel-list@lists.ximian.com
Subject: RE: [Mono-dev] [PATCH] Validation for xsl:output
attributes.
-Original Message-
From: Atsushi Eno [mailto:[EMAIL PROTECTED]
Sent: vrijdag 23 december 2005 18:54
To: Gert Driesen
Cc: [EMAIL PROTECTED]
Subject: RE: [Mono-dev] [PATCH] Validation for xsl:output
attributes.
Comments inline
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf
Of Atsushi Eno
Sent: dinsdag 20 december 2005 6:26
To: Gert Driesen
Cc: [EMAIL PROTECTED]
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.
You are saying the same idea with different words.
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 :(
You wrote a lot of helpful tests here and there, but it's not. Also this
bug is usually not worthy of spending much words. You committed
changes without further discussion, which is a bad sign.
Atsushi Eno
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
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.
- unindent cases in switches, i.e.
switch (foo) {
case bar:
...
- 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.
Please commit the patch after fixing them.
Atsushi Eno
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
