Maven Doxia

Add generic parameters support to Figure and Link events

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.0-alpha-10
  • Fix Version/s: 1.1
  • Component/s: Sink API
  • Labels:
    None
  • Number of attachments :
    0

Description

For example XWiki has the following syntax for image macros and links:

For the image macro there are the "document" and "fromincludingdoc" which are specific to XWiki and thus cannot be put as standard parameters.
Same for links.

Thus I propose to allow parsers to pass a Map of properties (pair/values) to the Sink API so that sinks can be written to understand them (the XWiki sink would understand them for example).

Issue Links

Activity

Hide
Lukas Theussl added a comment -

Just realized that your proposal is the same as what I noted at DOXIA-227, let me copy&paste the relevant part as it really belongs here:

Something like (taking figure as example, this should be applied to all relevant sink methods)

public void figure( SinkEventAttributes attributes );

where SinkEventAttributes is just a Map of attribute names-values which should be parsed and processed by the receiving Sink. I'm not sure about the details yet but this would solve a couple of issues apart from the current one, eg DOXIA-38, DOXIA-63, DOXIA-78, DOXIA-163, DOXIA-164, DOXIA-227.

Comments, ideas?

Show
Lukas Theussl added a comment - Just realized that your proposal is the same as what I noted at DOXIA-227, let me copy&paste the relevant part as it really belongs here: Something like (taking figure as example, this should be applied to all relevant sink methods)
public void figure( SinkEventAttributes attributes );
where SinkEventAttributes is just a Map of attribute names-values which should be parsed and processed by the receiving Sink. I'm not sure about the details yet but this would solve a couple of issues apart from the current one, eg DOXIA-38, DOXIA-63, DOXIA-78, DOXIA-163, DOXIA-164, DOXIA-227. Comments, ideas?
Hide
Vincent Siveton added a comment -

I like your idea of SinkEventAttributes. These attributes add the dynamic that is missing in Doxia and seems, on the paper, to solve several issues.

Show
Vincent Siveton added a comment - I like your idea of SinkEventAttributes. These attributes add the dynamic that is missing in Doxia and seems, on the paper, to solve several issues.
Hide
Vincent Massol added a comment -

The only issue is that we must have some well-known attributes so that sink implementations can be sure that they'll be available and putting them all in a map removes this notion of well-known attributes.

My proposal was a bit different:

  • well-known attributes passed as method parameters as before
  • some extra optional attributes passed as SinkEventAttributes

So sink implementation must support the well-known attributes while they can optionally support the extra optional ones.

I'm not 100% happy with this either but we need a way to ensure that 1) parsers will use the correct attribute names and 2) sink impl. can rely on the existence of some well known attributes.

Show
Vincent Massol added a comment - The only issue is that we must have some well-known attributes so that sink implementations can be sure that they'll be available and putting them all in a map removes this notion of well-known attributes. My proposal was a bit different:
  • well-known attributes passed as method parameters as before
  • some extra optional attributes passed as SinkEventAttributes
So sink implementation must support the well-known attributes while they can optionally support the extra optional ones. I'm not 100% happy with this either but we need a way to ensure that 1) parsers will use the correct attribute names and 2) sink impl. can rely on the existence of some well known attributes.
Hide
Lukas Theussl added a comment -

I don't see any way to ensure that parsers will use the correct attribute names. The only thing we can do is document, in the sink api, what attributes are supposed to be recognized for a given sink event. IMO it should be the standard HTML attributes for the corresponding html output, eg for the figure event above I have the following for now:

private static final String[] BASE_ATTRIBUTES =
    {
        Attribute.ID.toString(), Attribute.CLASS.toString(), Attribute.TITLE.toString(),
        Attribute.STYLE.toString(), Attribute.LANG.toString()
    };

    private static final String[] IMG_ATTRIBUTES =
    {
        Attribute.SRC.toString(), Attribute.ALT.toString(), Attribute.WIDTH.toString(),
        Attribute.HEIGHT.toString(), Attribute.ALIGN.toString(), Attribute.BORDER.toString(),
        Attribute.HSPACE.toString(), Attribute.VSPACE.toString(), Attribute.ISMAP.toString(),
        Attribute.USEMAP.toString()
    };

However, I don't quite see what your problem is. A parser is not a validator, it should just pass on the information it receives. It's the sink that should then decide what to do with each attribute it receives. Eg if the XWiki parser emits an attribute that the XhtmlSink does not understand (ie is not one of the above), then it will simply be ignored by the sink, while the XWiki Sink will know what to do with it. Of course, it has to be documented in the XWiki parser api which attributes are used in addition to the standard ones.

Also note that I want to add the above to the current API, not replace any existing methods.

Show
Lukas Theussl added a comment - I don't see any way to ensure that parsers will use the correct attribute names. The only thing we can do is document, in the sink api, what attributes are supposed to be recognized for a given sink event. IMO it should be the standard HTML attributes for the corresponding html output, eg for the figure event above I have the following for now:
private static final String[] BASE_ATTRIBUTES =
    {
        Attribute.ID.toString(), Attribute.CLASS.toString(), Attribute.TITLE.toString(),
        Attribute.STYLE.toString(), Attribute.LANG.toString()
    };

    private static final String[] IMG_ATTRIBUTES =
    {
        Attribute.SRC.toString(), Attribute.ALT.toString(), Attribute.WIDTH.toString(),
        Attribute.HEIGHT.toString(), Attribute.ALIGN.toString(), Attribute.BORDER.toString(),
        Attribute.HSPACE.toString(), Attribute.VSPACE.toString(), Attribute.ISMAP.toString(),
        Attribute.USEMAP.toString()
    };
However, I don't quite see what your problem is. A parser is not a validator, it should just pass on the information it receives. It's the sink that should then decide what to do with each attribute it receives. Eg if the XWiki parser emits an attribute that the XhtmlSink does not understand (ie is not one of the above), then it will simply be ignored by the sink, while the XWiki Sink will know what to do with it. Of course, it has to be documented in the XWiki parser api which attributes are used in addition to the standard ones. Also note that I want to add the above to the current API, not replace any existing methods.
Hide
Vincent Massol added a comment -

If ParserA sends a ("height", "100") attribue and ParserB sends a ("element-height", "100") and ParserC sends ("myheight", "100") then writing a sink that works in most cases is going to be a real pain...

Show
Vincent Massol added a comment - If ParserA sends a ("height", "100") attribue and ParserB sends a ("element-height", "100") and ParserC sends ("myheight", "100") then writing a sink that works in most cases is going to be a real pain...
Hide
Vincent Massol added a comment -

I think we agree and all I was saying is that it was maybe better to have more strongly typed attributes for well known ones. It's a detail probably.

If we offer enumerated types as shown above that'll certainly help.

Show
Vincent Massol added a comment - I think we agree and all I was saying is that it was maybe better to have more strongly typed attributes for well known ones. It's a detail probably. If we offer enumerated types as shown above that'll certainly help.
Hide
Lukas Theussl added a comment -

But how would you enforce a particular attribute name in any arbitrary parser? I don't think a sink should try to adjust to all such possibilities, parsers should simply be made to emit the (documented) standard attribute, Attribute.HEIGHT.toString() in this case. If ParserC still wants to emit a "myheight" attribute, it can still do so by adding it to the SinkEventAttributes, it will just be ignored by most sinks.

Show
Lukas Theussl added a comment - But how would you enforce a particular attribute name in any arbitrary parser? I don't think a sink should try to adjust to all such possibilities, parsers should simply be made to emit the (documented) standard attribute, Attribute.HEIGHT.toString() in this case. If ParserC still wants to emit a "myheight" attribute, it can still do so by adding it to the SinkEventAttributes, it will just be ignored by most sinks.
Hide
Vincent Massol added a comment -

How do you ensure that a parser will emit Attribute.HEIGHT instead of "myheight"? Of course it should emit Attribute.HEIGHT.

OTOH if the event is named onImage(int height, ...) then the parser is forced to emit the correct value.

As I said it's only a question of "typed" vs "untyped".

Show
Vincent Massol added a comment - How do you ensure that a parser will emit Attribute.HEIGHT instead of "myheight"? Of course it should emit Attribute.HEIGHT. OTOH if the event is named onImage(int height, ...) then the parser is forced to emit the correct value. As I said it's only a question of "typed" vs "untyped".
Hide
Lukas Theussl added a comment -

I think it's a matter of practicability (if that word exists in english... ), and keeping the api as concise as possible. It is somewhat subjective what you consider 'well known' attributes. For instance for the img attributes I listed above, I would consider all but the last two as 'well known'. So together with the base attributes that would make 13 method parameters...

Also note the example of tableCell(); where someone already added the method tableCell( String width ); but I wouldn't consider 'align, colspan, height, rowspan, valign' any less known than 'width'. And we are not going to add a method for each of them...

IMO we should only keep parameters if they are required or logically necessary for a sink event, eg:

void anchor( String name, SinkEventAttributes attributes );
void figureGraphics( String src, SinkEventAttributes attributes );
void link( String name, SinkEventAttributes attributes );
void numberedList( int numbering, SinkEventAttributes attributes );
void section( int level, SinkEventAttributes attributes );
void sectionTitle( int level, SinkEventAttributes attributes );

so SinkEventAttributes would only specify optional parameters. WDYT?

Show
Lukas Theussl added a comment - I think it's a matter of practicability (if that word exists in english... ), and keeping the api as concise as possible. It is somewhat subjective what you consider 'well known' attributes. For instance for the img attributes I listed above, I would consider all but the last two as 'well known'. So together with the base attributes that would make 13 method parameters... Also note the example of tableCell(); where someone already added the method tableCell( String width ); but I wouldn't consider 'align, colspan, height, rowspan, valign' any less known than 'width'. And we are not going to add a method for each of them... IMO we should only keep parameters if they are required or logically necessary for a sink event, eg:
void anchor( String name, SinkEventAttributes attributes );
void figureGraphics( String src, SinkEventAttributes attributes );
void link( String name, SinkEventAttributes attributes );
void numberedList( int numbering, SinkEventAttributes attributes );
void section( int level, SinkEventAttributes attributes );
void sectionTitle( int level, SinkEventAttributes attributes );
so SinkEventAttributes would only specify optional parameters. WDYT?
Hide
Vincent Massol added a comment -

ok you've convinced me. I wish we still had a way to make that "typed" as much as possible, maybe by having specialized versions of SinkEventAttributes per event types. At the very least we should enumeration types to get well-known attributes, such as: attributes.get(LinkAttributes.LABEL).

BTW for me a well known attribute is simply an attribute that is defined in Doxia (i.e. for which we have a an enumeration for example).

Show
Vincent Massol added a comment - ok you've convinced me. I wish we still had a way to make that "typed" as much as possible, maybe by having specialized versions of SinkEventAttributes per event types. At the very least we should enumeration types to get well-known attributes, such as: attributes.get(LinkAttributes.LABEL). BTW for me a well known attribute is simply an attribute that is defined in Doxia (i.e. for which we have a an enumeration for example).
Hide
Lukas Theussl added a comment -

I committed a first version in r634662, please review and feel free to adjust and improve.

Show
Lukas Theussl added a comment - I committed a first version in r634662, please review and feel free to adjust and improve.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: