On Wed, Nov 18, 2009 at 09:04, Vincent Massol <[email protected]> wrote:

On Nov 17, 2009, at 7:16 PM, Thomas Mortagne wrote:

...

Hi devs,

Currently it's not possible to list support BlockRenderer syntaxes like we do for Parser for example.

So i propose to add Syntax getSyntax(); to BlockRenderer interface

Here is my +1

I'm hesitating. Shouldn't we have getSyntax() in the Renderer interface then?

If we add it to BlockRenderer, I'd like to see it in Renderer and to remove it from PrintRendererFactory.

Actually it's more than a renderer thing, it's exactly the same for parsers.

<stupid question> I don't recall why we need this getSyntax() method. Since the user will have to look up the component using a hint, he already knows the syntax for the BlockRenderer. Are there cases where we return a BlockRenderer from an api call and we need to know its syntax? If all we want is to know all syntaxes, then we just need to do a lookupMap and get the key list... </stupid question>

Syntax contains a pretty name you can't get from the component hint. And anyway component descriptor should remain technical component descriptor, the getSyntax() is a renderer/parser information not component information IMO. User should go that deep to get this high level informations from rendering POV.

<thinking aloud> We could potentially have a common interface between BlockRenderer and Renderer since they're both renderers. However that interface would only have the getSyntax() method which is too poor for a functional interface.... Hmmm... Actually could this mean that the Renderer name is a bad name... I think we could remove the current Renderer interface and rename PrintRenderer by PrintListener. Thus the user would use either a PrintListenerFactory or a BlockRenderer. </thinking aloud>

Renderer interface is useless currently actually. I don't like too much rename to *Listener, "Listener" should be used for more generics things. The goal of this API is to be a streaming based renderer where BlockRenderer is a dom based renderer it should be named StreamRenderer/StreamRendererFactory (especially since we voted for StreamParser and i plan to send a vote to rename Parser in BlockParser when i will have less modifications everywhere in the rendering module ;)). It's more clear for the API user that way I think. Another idea is to have only one entry point for streaming and dom based renderers, that would be the easiest to understand for user for sure but I'm not sure it's good. Even if i don't really see technical issues with that: it's easy to produce XDOM from events or events from XDOM if one want to implements just one aspect, he can emulate the other, the more i think of that the more it seems the cleanest to me, *Factory that we introduced for technical reason and that we never really liked would be removed. Here is a proposal: interface Renderer { Syntax getSyntax(); } /* write String */ interface StringRenderer extends Renderer { /* Block based rendering */ void render(Block block, WikiPrinter printer); /* Streaming based rendering */ Listener createListener(WikiPrinter printer); } (I dont't thing we need createStreamRenderer to expose something more specific than a Listener since the user is supposed to keep the WikiPrinter he provided) interface Parser { Syntax getSyntax(); } /* read String */ interface StringParser extends Parser { Syntax getSyntax(); /* Block based parsing */ XDOM parse(Reader source) throws ParseException; /* Streaming based parsing */ void parse(Reader source, Listener listener) throws ParseException; } (StringParser/Renderer is to prepare for the future binary parsers/renderers)

WDYT?

Thanks -Vincent

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Thomas Mortagne