Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-11-29 Thread Chris Hillery
Review: Approve -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/83210 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe :

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-11-22 Thread Chris Hillery
Review: Approve I still don't understand the #include situation. Clearly dict_en.cpp is not *yet* using ZORBA_NO_FULL_TEXT or else it wouldn't compile. What I'm guessing is that there is some other change coming from somewhere else which depends on having that #include. If so, IMHO then the

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-11-22 Thread Zorba Build Bot
Attempt to merge into lp:zorba failed due to conflicts: text conflict in ChangeLog -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/79165 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to :

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-11-22 Thread Daniel Turcanu
Yes, it would compile, but with the wrong error messages. The way Paul implemented it, it doesn't raise compile errors if the error messages are incorrect. I don't know if it raises runtime errors. So that include is necessary right now, it was necessary for some time ago. --

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-11-15 Thread Matthias Brantner
Sorry, I still don't understand why you added the zorba/config.h include. I was able to fix the included test case locally by doing only the change in strings_impl.cpp; it doesn't seem like the dict_XX_cpp.xq change is related at all. And I still think that adding that #include is a bad idea

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-11-15 Thread Matthias Brantner
Review: Approve -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/79165 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe :

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-19 Thread Daniel Turcanu
True, that change is not related to fn:analyze-string. It only solves the error messages included inside #if defined. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/79165 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list:

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-18 Thread Chris Hillery
Sorry, I still don't understand why you added the zorba/config.h include. I was able to fix the included test case locally by doing only the change in strings_impl.cpp; it doesn't seem like the dict_XX_cpp.xq change is related at all. And I still think that adding that #include is a bad idea

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-12 Thread Matthias Brantner
Review: Approve I approve the fix but it has merge conflicts in the ChangeLog. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/79120 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to :

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-12 Thread Chris Hillery
Review: Needs Information Still wondering what the #include zorba/config.h additions are about. They don't seem related or necessary... -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/79120 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list:

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-12 Thread Daniel Turcanu
The #include is necessary to have access to zorba settings. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/79120 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-12 Thread Daniel Turcanu
There are a lot of #if defined and #if !defined in that file. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/79120 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-11 Thread Matthias Brantner
Review: Needs Information What is the file test/rbkt/Queries/zorba/resolving/path_to_uri.xq supposed to test. It imports the file module but doesn't use it. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78996 Your team Zorba Coders is subscribed to branch lp:zorba. --

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-07 Thread Chris Hillery
It looks like all the absolute path changes are still in there... -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78621 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-06 Thread Daniel Turcanu
Maybe W3C doesn't care about file systems, but URIs do. At least http, ftp and file all work with file system paths. We actually work with URL, not URI, we don't have general URIs so far. I tested now file:path-to-uri and it doesn't do what I want: if I give it an absolute path, then it returns

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-06 Thread Chris Hillery
HTTP and FTP URIs certainly don't work with filesystem paths. File: URIs only do by convention, and there are (somewhat hand-wavy) rules for mapping filesystem paths to file: URIs. But a filesystem path is-not-a file: URI, and so a filesystem path is never an appropriate argument for

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-06 Thread Daniel Turcanu
Please stop giving arguments from the spec. I also know the spec. I am trying to come with a solution to the problem. This problem cannot be solved by sticking to the spec. So I want like this: have a variable that contains an absolute or relative file system path, and resolve it to a base

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-05 Thread Chris Hillery
Review: Disapprove This isn't a good fix. The path of a URI isn't a filesystem path; it's a specific part of a URI (see RFC 3986). The path portion of a URI cannot be absolute or relative. So it doesn't make sense to have this functionality in the URI class. fn:resolve-uri() is not supposed

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-05 Thread Daniel Turcanu
For Windows, the absolute path for URI is still in the form C:/, only that it looks like C3A/. At least for file:// scheme. I guess I should add a check only for that scheme. About absolute and relative paths in URI, think about what fn:resolve-uri works with. It can receive a relative path, or

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-05 Thread Chris Hillery
The path component of a file: URI on Windows can contain C:/ (or C%3A/), but that's not directly relevant. fn:resolve-uri() works with exactly URIs. It is not defined to accept filesystem paths. Thus, if the input is C:/foo, it will interpret that strictly as a URI (and most likely fail since

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-05 Thread Daniel Turcanu
Ok, I guess you are right. I reread again the URI spec and it doesn't say anything about Windows paths. But how to transform Windows filepaths into file URI in a OS independent way? There is another function fn:encode-for-uri, which is supposed to encode a path to be used as uri, but that isn't

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-04 Thread Matthias Brantner
Please open two bugs for this and link the bug numbers from the ChangeLog. Also, please describe (or mention the bugs) in the commit message. We already went back and forth with URI fixes plenty of times and it's very confusing if it's unclear which scenario your commit is supposed to fix. --

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-10-04 Thread Matthias Brantner
Review: Needs Fixing See comment recently made. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78110 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe :

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-09-29 Thread Matthias Brantner
Daniel, which test does the URI change fix? I don't see a test for that and it's not mentioned in the ChangeLog. Same for analyze-string. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/76577 Your team Zorba Coders is requested to review the proposed merge of

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-09-22 Thread Zorba Build Bot
Voting does not meet specified criteria. Required: Approve 0, Disapprove 1. Got: 1 Pending. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/76556 Your team Zorba Coders is requested to review the proposed merge of lp:~danielturcanu/zorba/mytrunk into lp:zorba. -- Mailing

Re: [Zorba-coders] [Merge] lp:~danielturcanu/zorba/mytrunk into lp:zorba

2011-09-22 Thread Zorba Build Bot
Voting does not meet specified criteria. Required: Approve 0, Disapprove 1. Got: 1 Pending. -- https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/76577 Your team Zorba Coders is requested to review the proposed merge of lp:~danielturcanu/zorba/mytrunk into lp:zorba. -- Mailing