Hi.
> Hi, and thanks for your comments :)
>
> > I have -Xinheritance plug-in for JAXB, implemented similar to yours. ;)
>
> Is it public somewhere?
I've forgotten to say that it's JAXB 1.
It's from jaxbcommons.dev.java.net
Here's the link:
https://jaxbcommons.dev.java.net/source/browse/jaxbcommons/core/src/main/java/org/jvnet/jaxbcommons/inheritance/addon/AddOn.java?rev=1.2&view=markup
> > Really nice to see people writing plugins. ;)
> > Would you like to host it on jaxb2-commons.dev.java.net?
>
> It's probably more convenient to have it on my own domain, if I am to do
> work more on it. Others are of course free to put it up there, possibly
> with their own improvements, or mirroring whatever I put out.
Ok, I think I'll double it there.
> classOutline.implClass._implements(getClass().getClassLoader().loadClass(interfaceName));
>
> >
> > ...
> >
> > 1. Don't do getClass().getClassLoader().loadClass(interfaceName). This
> > will require you to make the interface class available to XJC during the
> > code generation time. This will have consequences in the build order and
> > so on.
>
> This was intentional; my idea was to actually check if the class
> implemented the interface. I'm not comfortable outputting generated code
> that might not compile. Same with your point 2, I originally planned to
> do this.
Understand. I think it is very hard to reliably check if the class does actually
satisfy the interface. Nice thing to have but complex to implement.
> > Here are some code snippets from hyperjaxb3 ClassUtils:
>
> Thanks, I've never actually worked with the code model XJC uses, so this
> helps me in what to look for.
You are welcome to welcome to browse the code/participate.
> > throw new InterfacePluginException("Unable to find named interface
> > '"+interfaceName+"'",cnfe);
> >
> > 3. (well, you won't have any cnfe if you use codeModel.ref(...) but
> anyway)
> > Errors should be reported via the errorHandler.
>
> I was wondering about this, it seemed such a klunky thing to do;
>
> a) IMO plugins shouldn't know anything about how an error is handled
> outside itself. Knowing about error handlers and such is the job of
> whatever calls the plugin.
>
> b) Having a reference to the handler opens up for using it in unintended
> ways. Probably not a big problem in this case, but elsewhere it could
> cause unwanted entanglement, and it's an unnecesarry smell.
>
> c) SAXParseException seems like the wrong thing to pass; we're not
> really parsing XML files here, but rather interpreting and acting on the
> information that is already parsed. The exception seems also to want a
> Location (tho I see now in the doc that you can pass null), something
> that isn't made available anywhere I can find. If CPluginCustomization
> had a getLocation method for instance, the exception would make some sense.
>
> d) The methods for passing an error to the error handler can again throw
> SAXException, a checked exception that the run method does not throw,
> and what then do you do with it? Wrap it and pass it to the error
> handler again? Ignore it? Ignore it only for certain severities? Throw a
> runtime exception?
I agree, ErrorHandler is not that perfect. It's more a historic thing in there.
The intention of ErrorHandler is that you may have various severity levels of
errors (like, warn or fatal) which are interpreted differently by the upper
level. In some cases you need to halt, sometimes you just show a warning and
continue.
But in any case it's the existing interface and we have to respect it, no matter
how bad it is.
I agree with c) and d).
b) does not seem to be a problem - you don't have to pass the handler around.
It's enough to catch an exception on the top level (say, run method) and pass it
to the handler with the corresponding severity level.
> It would make a lot more sense if the run method were declared to throw
> a checked XJCPluginException or such that you could subclass, possibly
> holding a reference to the customization or whatever that caused the
> problem.
Yes, this would be better. Let's see what Kohsuke says.
I'm not sure if the plugin API could be changed.
Bye.
/lexi