The proposal to merge lp:~zorba-coders/zorba/bug1123835 into lp:zorba has been
updated.
Status: Needs review => Rejected
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug1123835/+merge/158978
--
https://code.launchpad.net/~zorba-coders/zorba/bug1123835/+merge/158978
Rejecting this proposal as it stands. If we want to fix this bug in future, we
need to revisit the plan.
--
https://code.launchpad.net/~zorba-coders/zorba/bug1123835/+merge/158978
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post
Review: Needs Fixing
Sorry, but this implementation isn't right. You're still sucking the entire
contents of the istream into memory via that stringstream. That's not
acceptable. (You also have a potential memory leak since you don't use an
auto_ptr<> for the stringstream you allocate on the st
3 of the failing test-cases are correct (fn-unparsed-text-lines-039,
fn-unparsed-text-039, fn-unparsed-text-available-038).
Here is why:
The 3 test-cases use fn/unparsed-text/non-xml-character.txt that contains BOM
followed by NULL character.
Here is the catch: NULL is not a valid XML 1.0 nor X
Review: Needs Fixing
Also, we should not have a check in our code for a base URI "#UNDEFINED". That
comes from FOTS; it is not part of Zorba or XQuery. I do not know how to tell
whether the base URI of a static context is actually "undefined" or not. In
fact I'm not even 100% sure what that mea
Review: Needs Fixing
Unfortunately, I don't think the proposed change is safe. istream::unget() is
not guaranteed to work, and in particular it probably won't work if the stream
is coming via HTTP.
However, there is a function StreamResource::isStreamSeekable(). Perhaps you
could check that fl
> Ok, maybe ignore point #1 - that function is there after all. I'm not sure how
> I missed it; must have been a typo in my search. Anyway, I'll review the code
> more thoroughly a bit later tonight. Points #2 and #3 still need to be fixed.
Points #2 and #3 should be fixed.
The proposed changes, s
Ok, maybe ignore point #1 - that function is there after all. I'm not sure how
I missed it; must have been a typo in my search. Anyway, I'll review the code
more thoroughly a bit later tonight. Points #2 and #3 still need to be fixed.
--
https://code.launchpad.net/~zorba-coders/zorba/bug1123835/
Review: Needs Fixing
1. It looks like you didn't check in some changes? sequences_impl.cpp refers to
a method URI::get_encoded_fragment() that doesn't exist.
2. sequences_impl.cpp now shows up on my system as "ISO-8859 English text"
rather than "ASCII English text" when I use the "file" command
This branch doesn't solve all the errors yet this is the list of errors missing
solution with a brief description of the current problem.
The missing errors are caused by 3 problems
* utf-8 encoding: missing the stream suggested to Paul that can handle errors
when invalid utf-8 are found.
* unkn
Juan Zacarias has proposed merging lp:~zorba-coders/zorba/bug1123835 into
lp:zorba.
Commit message:
Fixes for FOTS errors in fn:unparsed-text* functions
Requested reviews:
Chris Hillery (ceejatec)
Related bugs:
Bug #1123835 in Zorba: "fn-unparsed-text* failures (at least 20 failures)"
http
11 matches
Mail list logo
