>>>>> "KR" == Keven Ring <[EMAIL PROTECTED]> writes:
>> Could you please provide additional tests for absolute JAR sub
>> URL's? This will help ensure that you have covered all cases and
>> that we don't break anything you are depending on in the future.
KR> Sure. How would you like me to supply additional tests? Via this
KR> reflector?
Yes, that would be great.
KR> I'll let you know when I have the tests up and working.
Thanks.
>> Comments on the code:
>>
>> I'm a little disappointed with the use of java.net.URL here (as
>> opposed to directly handling the absolute and relative cases),
>> although I've most convinced myself that this doesn't introduce a
>> significant compatability issue (given the data protocol URL
>> requiremenst of SVG, java.net.URL makes me twichy :).
KR> I fully understand [ yes, I did read the javadoc comments
KR> regarding ParsedURL, and why it exists ]. However, I was in a bit
KR> of a dilema - I could parse it myself, and construct a suitable
KR> URI, but since the JAR URL specification is really Sun's
KR> specification, I would rather 'trust' their implementation of
KR> constructing a new JAR URL [irrespective of the type of URL being
KR> constructed, whether it is absolute, absolute in the same JAR, or
KR> relative in the same JAR] than attempting to write a [possibly
KR> poor] substitute, and at the same time, not have to worry about
KR> possible format specification changes by Sun.
KR> Because ParsedURL performs the same type of functionality as the
KR> java URL class, if ParsedURL was modified to support these three
KR> classes of modifications to an existing ParsedURL, then, I agree,
KR> it would be the cleanest approach. However, because other
KR> protocols require ParsedURL to operate correctly, I was hesitant
KR> to make the necessary modifications, somewhat due to the
KR> unwillingness (at the time) to fully understand what ParsedURL was
KR> doing in the plethora of cases.
As I said I don't think it's a huge problem (I don't think a
'jar:data:xxxx!foo.svg' is something that we really need to deal with
at this point).
>> As you seem to suspect I might, I really don't like the openStream
>> call. Do you have a case that you expect to encounter here? If not
>> I would rather drop it.
KR> The case that I wanted to support, which, in the long run, isn't
KR> very important, is the ability to fall back to the original URL
KR> merging algorithm. Assuming that the overall issue is resolved,
KR> then the opening and closing of the resource is certainly
KR> superflous.
I guess I was asking if you had a specific type of input that you
thought the original code handled and the new code wouldn't.
I ran the regard tests with your update any except for improved
results from one of the relative cases
("jar:file:dir/file.jar!/p/a/t/h/../base.svg" ->
"jar:file:dir/file.jar!/p/a/t/base.svg" - which means that relative
references will work now :) everything gave the same results. So the
patch is looking pretty good without this check.
>> Finally, you are better off using 'constructParsedURLData(url)'
>> rather than calling 'parseURL(url.toExternalForm())'.
KR> Great! If there is a preferred/more efficient way of constructing
KR> a ParsedURL instance given a URL, then, by all means, make
KR> changes.
I made this change.
KR> My purpose for posting the original message, and the submitted
KR> change, was simply to identify an issue, and to provide a possible
KR> solution. The solution is based on my knowledge and understand of
KR> Batik, as well as the time available to investigate the matter. I
KR> welcome the inclusion, or modification of the solution, more to
KR> further extend the capabilities of the entire system.
Sure I understand, of course part of my job is to get the people
who "feel the itch" to finish the job (a few tests please ;) so everyone
can use it.
Thanks for the help!
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
