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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 #include should be added in 
conjunction with that change, not this one.

Still, assuming for the moment that the #include will be necessary *someday*, 
I'm approving this change 'cause frankly this proposal has been beaten to death 
already...
-- 
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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.
-- 
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 unless it's
 vitally necessary, because Zorba already has far too many header dependencies.
Chris, I agree with Daniel that the include is necessary. For example, 
dict_en.cpp
uses guards such as ZORBA_NO_FULL_TEXT to enable or disabled certain errors. 
Those
guards are only available if config.h is included.
-- 
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 unless it's 
vitally necessary, because Zorba already has far too many header dependencies.
-- 
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 a complete URI, and if I give it a relative 
path, then it also returns a complete URI, with the path relative to my rbkt 
dir in the build directory. This seems like a wrong thing. If my rbkt dir is 
hardcoded in the file module, then it won't work for other users if I deploy 
the file module.

I still think that this is a job for fn:encode-for-uri. I need a function to 
encode a file system path into a path for URI. So if it is absolute path, make 
it absolute path for URI, not a full URI, and if it is relative path, make it 
relative path for URI.
And then this path I can apply to fn:resolve-uri.
-- 
https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78253
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 absolute URI. There should be no 
if(is_absolute_path) and no if(win32) in the xquery code. Right now this 
doesn't work, because our URI implementation considers the absolute Win32 path 
as a relative path, because it doesn't start with slash.

Maybe we shoud add another function in the file module, like 
file:path-to-uri-path, which should add a slash for absolute win32 paths, and 
leave the relative paths as they are.
-- 
https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78253
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 to accept a filesystem path, so the test case 
path-to-uri.xq is not valid. If it DOES work on Unix as written, that's 
probably a different bug.

I am commenting specifically on the fix to bug 868329. I have nothing to say 
about the fix for bug 868325 except that it should probably be in a separate 
commit.
-- 
https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78253
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 absolute path or entire URI. When 
receiving absolute paths, it doesn't work now in Windows for file://.
-- 
https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78253
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 the C: scheme probably isn't 
known).

fn:doc() is an explicit exception in Zorba - it is defined (by us) to accept 
either a URI *or* a filesystem path. This is technically not W3C-spec 
compliant, but it was decided that it was necessary for a good customer 
experience. But fn:resolve-uri() does not have the same magic. If you want, 
you could maybe make a case that it should have the same magic, but even if we 
agreed on that, it should use the same code from inside the implementation of 
fn:doc() to achieve it.
-- 
https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78253
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 prepared for Windows either.
fn:encode-for-uri(C:\path1\file.xml) gives C%3Apath1%2Ffile.xml. 
I would expect /C%3A/path1/file.xml , meaning detect that is an absolute 
Windows path, and replace backslashes with slashes.
I think the problem is in this function, it should be made to work differently 
for Windows.
-- 
https://code.launchpad.net/~danielturcanu/zorba/mytrunk/+merge/78253
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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.
-- 
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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 
lp:~danielturcanu/zorba/mytrunk into lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


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 list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp