On 4 July 2012 12:32, Vincent Hennebert <vhenneb...@gmail.com> wrote:
> One thing I forgot to mention: this work deserves its entry in the
> status.xml file, with importance set to ‘high’.
> <snip/>
> Let me insist on that one. The inconsistency is actually introduced by
> Fop classes. Before the merge there were 14 classes having FOP in their
> names, vs 6 having Fop. Also, the product name is FOP, not Fop, and that
> should be reflected in the class names.
>
I still don't agree, it's not about the number of classes but rather the
importance and location of those classes; FopFactory.java, Fop.java. If you
feel very strongly about it, feel free to change it, I won't oppose it, but
when someone thinks "wtf!!! Why are there so many different ways to write
FOP!?!?!" and they do an svn blame, I don't want my name anywhere near it.
<snip/>
> I’m confused. It seems that I can call that method with an
> ‘application/pdf’ mimeType and a PSRendererConfigParser instance. In
> which case the parser would not be able to parse PDF-specific options,
> and would silently ignore them. Or did I miss something?
>
You missed something. For most the configurators, you are correct, but for
the Bitmap configurators it gets a little bit more involved, they re-use
the same code. It was a choice between either a) duplicating code, or b)
doing what we did. (There was a c) option, but as always, it involved a lot
more redesign than we had time for.)
> <snip/>
> 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 ;) )
> So when you remove it you will have to re-write the build method such
> that it doesn’t call buildConfig anymore. How will you do? (Admittedly
> I haven’t studied the new API in much detail.)
>
> And BTW, what is the recommended way to get a FopFactoryConfig? The
> javadoc of buildConfig doesn’t say. I think it should.
The client doesn't need access to the FopFactoryConfig object, it's a
wrapper for all the configuration objects that FOP uses. You instantiate a
FopFactory by using the FopFactoryBuilder.build(). I was going to write up
documentation suggesting as much, but I haven't had the chance to do so yet.
> I’m not sure I understand. How is that supposed to help? Wouldn’t it
> actually be more helpful if the FopFactory were now referring to the
> interface? That would indicate to users that something is going to
> change and they should take the habit of looking at FopFactoryConfig
> from now on.
I think the thought process was that users would be more used to accessing
those members via the FopFactory so linking back to that same object would
be more explicit. Maybe it doesn't make sense to anyone other than me/Pete.
>> src/java/org/apache/fop/apps/io/InternalResourceResolver
> Why is it a public class then if it’s supposed to be internal to FOP?
> IIC that apps/io package is meant to become part of the public API?
>
Well Java doesn't have a project only access modifier so... Where else
would we put it? This class is peculates through FOP so it has to be public.
> >> • getBaseURI: I don’t think adding a slash at the end of the URI if
> >> it’s
> >> not there is desirable, because that effectively changes the base URI.
> >> That may lead to very confusing errors.
> >>
> >
> > As the javadoc suggests this is for giving you a directory base URI, I've
> > changed the method signature to {getBaseURI, getBaseDirectoryURI}.
>
> Ah, I’m glad to hear that I was right to assume that the getBaseURI
> method returns a base URI :-)
>
> What I’m talking about is that, by adding a final ‘/’ to the given
> string, you are effectively changing the base URI.
> For example, that method is called by the AFPResourceAccessor class,
> which passes as a parameter the baseURI it’s been given, which that
> getBaseURI method may change into a different base URI! This is very
> misleading and may result into hard-to-find errors.
>
The behaviour is the same as it was previously. I wanted to have a single
base URI for ALL of FOPs URI resolution, but that would be devastating to
backwards compatibility. Don't hate the player, hate the game ;)
> >> src/java/org/apache/fop/apps/io/TempResourceResolver
>
>
> 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?
>
Uhmm... I dunno. I didn't really handle this part of the code, Pete?
Presumably, since you have only read OR write access, you can delete it
once it's written, but you'll have to check with Pete.
