Re: [VOTE] Merge Temp_URI_Unification
Hi Chris/Glenn/Anyone else, You say command-line options should override the fop.xconf values, which makes sense. But should not-given command-line options override fop.xconf values too? Bare with me here, there is sense in the folly of that sentence. Ok, so let's take the example above, with strict FO validation, from the command line you have two options: 1) fop -r ... other args or 2) fop ... other args Obviously in option 1, you'd want strict FO validation to be invoked, regardless of what's in the fop conf. But how do we treat option 2? We're not explicitly telling it NOT to validate strictly, so how would a user expect FOP to behave? On 4 July 2012 17:26, Chris Bowditch bowditch_ch...@hotmail.com wrote: On 04/07/2012 17:15, Glenn Adams wrote: On Wed, Jul 4, 2012 at 9:25 AM, mehdi houshmand med1...@gmail.commailto: med1...@gmail.com wrote: On 4 July 2012 12:32, Vincent Hennebert vhenneb...@gmail.com mailto:vhenneb...@gmail.com wrote: There does seem to be a regression. Before, the FopFactoryConfigurator object was getting the strict-validation element from the config file and calling FopFactory.setStrictValidation if it was set to true. This is no longer the case. I’ve just tried to render a table with table-footer after table-body, and now a validation error is thrown even if I have strict-validation set to false in my config file. No validation error was thrown before. I've fixed the underlying issue but this is an interesting one; it isn't obvious that settings from the fop conf override the settings from the command line options. (Pete probably wrote this part hahaha ;) ) They shouldn't. Command line options should override the conf file. Agreed, but IIUC, the team just copied the existing functionality due to time constraints. We should probably log a bug to track that as an issue. Thanks, Chris
[Bug 51843] Surrogate pairs not treated as single unicode codepoint for display purposes
https://issues.apache.org/bugzilla/show_bug.cgi?id=51843 --- Comment #33 from Shepard Lee shepardck...@gmail.com --- Hi All, I encountered the same issue on my applications using fop 1.0. Glad to see the issue is going to be fixed in the coming version. May I know if this bug will be fixed in version 1.1 only or it will be patched in version 1.0, too? Shepard -- You are receiving this mail because: You are the assignee for the bug.
Re: [VOTE] Merge Temp_URI_Unification
On Thu, Jul 5, 2012 at 12:41 AM, mehdi houshmand med1...@gmail.com wrote: Hi Chris/Glenn/Anyone else, You say command-line options should override the fop.xconf values, which makes sense. But should not-given command-line options override fop.xconf values too? Bare with me here, there is sense in the folly of that sentence. Ok, so let's take the example above, with strict FO validation, from the command line you have two options: 1) fop -r ... other args or 2) fop ... other args Obviously in option 1, you'd want strict FO validation to be invoked, regardless of what's in the fop conf. But how do we treat option 2? We're not explicitly telling it NOT to validate strictly, so how would a user expect FOP to behave? In the case of strict validation, if either configuration file or command line option says do strict validation, then strict validation should apply. We would need an option don't do strict validation in order to allow the command line to override a configuration file saying to perform strict validation.
Re: [Bug 51843] Surrogate pairs not treated as single unicode codepoint for display purposes
No, this is NOT going to be fixed in the upcoming version. I have made NO statements about when this will be addressed in FOP. In particular, it will NOT be patched in 1.0 and will NOT be addressed in 1.1. This is a POSSIBLE 1.2 (or later) fix. On Thu, Jul 5, 2012 at 1:23 AM, bugzi...@apache.org wrote: https://issues.apache.org/bugzilla/show_bug.cgi?id=51843 --- Comment #33 from Shepard Lee shepardck...@gmail.com --- Hi All, I encountered the same issue on my applications using fop 1.0. Glad to see the issue is going to be fixed in the coming version. May I know if this bug will be fixed in version 1.1 only or it will be patched in version 1.0, too? Shepard -- You are receiving this mail because: You are the assignee for the bug.
[Bug 51843] Surrogate pairs not treated as single unicode codepoint for display purposes
https://issues.apache.org/bugzilla/show_bug.cgi?id=51843 --- Comment #34 from Glenn Adams gad...@apache.org --- (In reply to comment #33) Hi All, I encountered the same issue on my applications using fop 1.0. Glad to see the issue is going to be fixed in the coming version. May I know if this bug will be fixed in version 1.1 only or it will be patched in version 1.0, too? No, this is NOT going to be fixed in the upcoming version. I have made NO statements about when this will be addressed in FOP. In particular, it will NOT be patched in 1.0 and will NOT be addressed in 1.1. This is a POSSIBLE 1.2 (or later) fix. -- You are receiving this mail because: You are the assignee for the bug.
Re: [VOTE] Merge Temp_URI_Unification
Hi Vincent, FOP Dev, On Wed, Jul 4, 2012 at 12:32 PM, Vincent Hennebert vhenneb...@gmail.com wrote: snip/ And BTW, what is the recommended way to get a FopFactoryConfig? The javadoc of buildConfig doesn’t say. I think it should. The need to obtain an instance of a FopFactoryConfig implementation is most probably unnecessary, however we have made it an option for two reasons: 1 The unit test org.apache.fop.fotreetest.FOTreeTestCase uses processing instructions in the parsed FO files to alter FopFactory configuration. We decided not to change this mechanism, and thus required a way to modify the configuration. By exposing the underling FopFactoryConfig we were able to wrap the immutable instance in a proxy that allows property modification. 2 If an existing FOP client relies on the modification of the FopFactory then there is a mechanism in place. Modifying the FopFactory after construction is discouraged which is why we have not advertised the practise in the javadocs or wiki. I have just noticed that the stativ method FopFactoryConfig FopFactory.newInstance(FopFactoryConfig) is not marked as depricated - I think the long term intention should be to make this method package private, along with FopFactoryConfig. src/java/org/apache/fop/apps/io/TempResourceResolver • unless I’m mistaken there’s no way of releasing the temporary resource once it’s no longer needed. The DefaultTempResourceResolver implementation deleteOnExits the file but this may not be enough if FOP is run on a server that is meant to (almost) never shut down. Or did I miss something? You missed something; this is the default implementation. This is pretty much only for the CLI use-case, if users wish to define their own implementation, all they have to do is implement the interface. OK, so presuming I want to write my own implementation that does a more fine-grained management of resources, how do I know that I can release a given resource? This is a good point. Currently, FOP assumes that temporary resources become unavailable for reading once the input stream is closed. FOP is expected to call InputStream.close(), and this call should be used to trigger he release of the resource, perhaps by wrapping the underlying InputStream in an implementation FilteredInputStream that does this cleanup. We intend to document in detail some recipes for implementing the ResourceResolver - there is currently some further work required for the handling of resource types (we envision that that these will consist of something like a superset of all MIME types) that we wish to handle first. Thanks, Peter
Re: git-svn doesn't update svn:mergeinfo
On 05/07/12 02:54, Glenn Adams wrote: On Wed, Jul 4, 2012 at 3:54 PM, Alexios Giotis alex.gio...@gmail.comwrote: Well actually git-svn has an option to update the svn:mergeinfo. It might be worth giving it a try. There are some restrictions and I suggest to test it first. The following is part of $ man git-svn --mergeinfo=mergeinfo Add the given merge information during the dcommit (e.g. --mergeinfo=/branches/foo:1-10). All svn server versions can store this information (as a property), and svn clients starting from version 1.5 can make use of it. To specify merge information from multiple branches, use a single space character between the branches (--mergeinfo=/branches/foo:1-10 /branches/bar:3,5-6,8) config key: svn.pushmergeinfo This option will cause git-svn to attempt to automatically populate the svn:mergeinfo property in the SVN repository when possible. Currently, this can only be done when dcommitting non-fast-forward merges where all parents but the first have already been pushed into SVN. Thanks for pointing that out. I had thought to use svn prop set manually as well, but after reviewing a number of threads on this issue, I've decided it would be safer to simply avoid merges on the git side, doing them on the svn side then using git svn rebase on the git side. A wise decision I think. Even after tweaking the svn:mergeinfo property, the commit would probably still not show up in the Apache Git mirror as a merge of two branches. I'll fix up the current fop-1_1 branch with a bit of manual rebuilding. Thanks for your efforts! Vincent
Re: [VOTE] Merge Temp_URI_Unification
Ok, so I've made the CLI options override when set, and not override when not set. I think that CommandLineOption class needs a little TLC, we can use Commons CLI or some such library ( http://java-source.net/open-source/command-line). No point reinventing the wheel. On 5 July 2012 08:27, Glenn Adams gl...@skynav.com wrote: On Thu, Jul 5, 2012 at 12:41 AM, mehdi houshmand med1...@gmail.comwrote: Hi Chris/Glenn/Anyone else, You say command-line options should override the fop.xconf values, which makes sense. But should not-given command-line options override fop.xconf values too? Bare with me here, there is sense in the folly of that sentence. Ok, so let's take the example above, with strict FO validation, from the command line you have two options: 1) fop -r ... other args or 2) fop ... other args Obviously in option 1, you'd want strict FO validation to be invoked, regardless of what's in the fop conf. But how do we treat option 2? We're not explicitly telling it NOT to validate strictly, so how would a user expect FOP to behave? In the case of strict validation, if either configuration file or command line option says do strict validation, then strict validation should apply. We would need an option don't do strict validation in order to allow the command line to override a configuration file saying to perform strict validation.
Comments on FopFactory.getRendererConfig (Temp_URI_Unification)
Hi,
I had a quick look on the work you are doing and I think that the newly
introduced methods
FopFactory.getRendererConfig(getRendererConfig(FOUserAgent userAgent,
Configuration cfg,
RendererConfigParser configCreator))
and the similar ones on FOUserAgent should all be replaced with a single
FopFactory.getRendererConfig(String mimeType).
1. Design reason: This is a public API that uses too many internal objects.
RendererConfigParser is not part of the public API [1] and I find no reason to
add. If this does not change, then it becomes automatically part of the public
API (it's a public method of the public FopFactory).
[1] http://xmlgraphics.apache.org/fop/trunk/embedding.html#API
2. Buggy: FopFactory is supposed to be reusable and a new FOUserAgent should be
created for every execution since it holds some rendering specific information.
But the existing FopFactory.getRendererConfig() uses the rendering specific
FOUserAgent to cache in (MapString, RendererConfig rendererConfig) a
RendererConfig. The cached object will be used by a future FOUserAgent with
possibly different user properties. Sounds scary to me. What if the
configuration of a renderer uses a FOUserAgent specific property ? (the
properties of the first FOUserAgent will be used).
Looking a little deeper, the renderer config implementations are currently
using firstly the FOUserAgent to get access to FopFactory (e.g. to check the
strict validation flag), secondly the event broadcaster. The first tells me
that they actually need to get the immutable FopFactory instead of the
FOUserAgent. The second (usage of the event broadcaster) makes my fears come
true. Scenario:
During the first rendering, a default FOUserAgent is used. Assuming the
configuration is problematic, the default event handlers will log a message and
continue. During the next rendering, an event broadcaster that aborts
processing on any error is registered. The user would expect to stop on any
error, but the process will simply continue.
I see a few options. I am in favor of the simpler one which is to throw an
exception instead of broadcasting an event when an error in found in the
renderer config. A second one is to add yet another flag on FopFactory that
will determine this behavior. Another one is to register event handlers related
to renderer config on FopFactory. It's flexible but a thread safe
implementation of the handler is required.
3. Buggy: FopFactory getRendererConfig() is not thread safe. Wrap the method
body in a synchronized (rendererConfig) {...}
Some quickies:
* FopFactory.java has unused members: private static Log log hyphPatNames.
* FopFactory.java javadoc has a typo renderingq
What do you think ?
Alexios Giotis
Re: Comments on FopFactory.getRendererConfig (Temp_URI_Unification)
Hi Alexios,
Thanks for taking a look at this.
On Thu, Jul 5, 2012 at 1:41 PM, Alexios Giotis alex.gio...@gmail.com wrote:
Hi,
I had a quick look on the work you are doing and I think that the newly
introduced methods
FopFactory.getRendererConfig(getRendererConfig(FOUserAgent userAgent,
Configuration cfg,
RendererConfigParser configCreator))
and the similar ones on FOUserAgent should all be replaced with a single
FopFactory.getRendererConfig(String mimeType).
1. Design reason: This is a public API that uses too many internal objects.
RendererConfigParser is not part of the public API [1] and I find no reason
to add. If this does not change, then it becomes automatically part of the
public API (it's a public method of the public FopFactory).
[1] http://xmlgraphics.apache.org/fop/trunk/embedding.html#API
Good point - a quick fix is to make the method can be made package
private. With a bit more work I agree that the method signature could
be simplified: The Configuration object is resolvable from the mime
type, as you suggest, however calls to getRendererConfig(String
mimeType, RendererConfigParser configCreator) can be made whereby
configCreator.getMimeType() differs from mimeType (see
BitmapRendererConfigurator). Ideally we should fix the configuration
hierarchy however this was out of scope for our initial
implementation. Removing the FOUserAgent parameter and calling
RendererConfigParser.build() with the FopFactory makes sense, however
the issue of the Event Broadcaster crops up - see below...
2. Buggy: FopFactory is supposed to be reusable and a new FOUserAgent should
be created for every execution since it holds some rendering specific
information. But the existing FopFactory.getRendererConfig() uses the
rendering specific FOUserAgent to cache in (MapString, RendererConfig
rendererConfig) a RendererConfig. The cached object will be used by a future
FOUserAgent with possibly different user properties. Sounds scary to me. What
if the configuration of a renderer uses a FOUserAgent specific property ?
(the properties of the first FOUserAgent will be used).
Looking a little deeper, the renderer config implementations are currently
using firstly the FOUserAgent to get access to FopFactory (e.g. to check the
strict validation flag), secondly the event broadcaster. The first tells me
that they actually need to get the immutable FopFactory instead of the
FOUserAgent. The second (usage of the event broadcaster) makes my fears come
true. Scenario:
During the first rendering, a default FOUserAgent is used. Assuming the
configuration is problematic, the default event handlers will log a message
and continue. During the next rendering, an event broadcaster that aborts
processing on any error is registered. The user would expect to stop on any
error, but the process will simply continue.
I see a few options. I am in favor of the simpler one which is to throw an
exception instead of broadcasting an event when an error in found in the
renderer config. A second one is to add yet another flag on FopFactory that
will determine this behavior. Another one is to register event handlers
related to renderer config on FopFactory. It's flexible but a thread safe
implementation of the handler is required.
Well spotted! We considered this problem and wondered whether we
should store the events and republish them upon subsequent calls to
FopFactory.getRendererConfig.
I think this is something to consider, along with your suggestions, if
it turns out to be a practical requirement.
3. Buggy: FopFactory getRendererConfig() is not thread safe. Wrap the method
body in a synchronized (rendererConfig) {...}
Well spotted!
Thanks,
Peter
