[
https://issues.apache.org/jira/browse/DOXIA-614?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17152901#comment-17152901
]
ASF GitHub Bot commented on DOXIA-614:
--------------------------------------
abelsromero commented on a change in pull request #35:
URL: https://github.com/apache/maven-doxia/pull/35#discussion_r451014972
##########
File path:
doxia-core/src/main/java/org/apache/maven/doxia/parser/AbstractXmlParser.java
##########
@@ -100,7 +98,7 @@
private boolean validate = false;
/** {@inheritDoc} */
- public void parse( Reader source, Sink sink )
+ public void parse( Reader source, Sink sink, String reference )
Review comment:
Effectively, it is not. As explained in the JIRA issue the PR aims to be
that the default method instead of the other one with 2 arguments. The way it
is implemented right now, even when `Parser` interface has two method, the one
with thre is not usable by how it's overriden in `AbstractParser` and never
invoked in `DefaultDoxia`.
The final goal is to deprecate the one with two arguments so that any plugin
can obtain the metadata of the sourcefile bien processed.
I am available to discuss online if anyone needs to, because I am bit
confused by how this review is proceding. I think the JIRA issue, explanation
here and the additional diagram provide a clear description of the goals and
changes done which are the complex topics. However, I am seeing a focus on
minor things. Furthermore, I raised 2 important points in the JIRA issue which
had had no answer and which imo are worth discussing, one related to problems
with the build and the other with testing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Support obtaining the source reference in a Doxia Parser
> --------------------------------------------------------
>
> Key: DOXIA-614
> URL: https://issues.apache.org/jira/browse/DOXIA-614
> Project: Maven Doxia
> Issue Type: Improvement
> Components: Core, Modules, Site Renderer (moved to DOXIASITETOOLS)
> Affects Versions: 1.9.1
> Reporter: Abel Salgado Romero
> Priority: Major
>
> Currently, the 'org.apache.maven.doxia.parser.Parser' interface provides 2
> methods that parsers can implement, one with and the other without the source
> reference. However, in reality the one with the reference added in DOXIA-224
> is never invoked, thus making it unusable from maven-site-plugin.
> Methods are:
> * void parse( Reader source, Sink sink )
> * void parse( Reader source, Sink sink, String reference )
>
> 1. Base implementation 'org.apache.maven.doxia.parser.AbstractParser'
> ('doxia-core') of the reference method calls the other, removing the
> reference if it is provided.
> {code:java}
> public void parse( Reader source, Sink sink, String reference )
> throws ParseException
> {
> parse( source, sink );
> }{code}
> 2. In 'org.apache.maven.doxia.DefaultDoxia#parse' ('doxia-core') the parser
> method 'parser (reader, sink)' is called at the end, which means
> 'parser(reader, sink, reference)' is never used.
>
> {code:java}
> public void parse( Reader source, String parserId, Sink sink )
> throws ParserNotFoundException, ParseException
> {
> Parser parser = parserManager.getParser( parserId );
> parser.enableLogging( new PlexusLoggerWrapper( getLogger() ) );
> parser.parse( source, sink );
> }
> {code}
>
> 3. 'org.apache.maven.doxia.Doxia#parse(Reader source, String parserId, Sink
> sink )' and it's implementation 'DefaultDoxia' ('doxia-core') does not
> receive the source reference, so it cannot pass it in any way.
>
> For these 3 reasons,
> 'org.apache.maven.doxia.siterenderer.DefaultSiteRenderer#renderDocument'
> ('doxia-sitetools') who has the source reference is not able to pass it to
> the above components.
> To sum up, the chaing of calls is: DefaultSiteRenderer -> Doxia/DefaultDoxia
> -> Parser implementation (e.g. AbstractParser)
>
> After discussion with [~hboutemy] the proposal is to fix it in several steps
> to avoid using SNAPSHOTS and ensure no breaking points:
> # Change doxia-core components to support new method with "reference", still
> having both methods. This means, changing AbstractParser and it's
> implementations to recieve the "reference". This does not break anything
> since no one is using this param now.
> # Change doxia-sitetools's 'DefaultSiteRenderer' component to pass the
> reference to Doxia/DefautDoxia DOXIASITETOOLS-225.
> # Release new doxia-sitetools version.
> # Once released, modifty doxia-core components to pass the reference and
> remove the (Reader, Sink) method completely. Also, in advance that Parsers
> may need more information about the source, instead of passing "String
> reference", we will pass a new class "SourceMetadata" that encapsulates the
> "String reference. That way we can add more information in the future if
> needed.
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
