maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hervé BOUTEMY <herve.bout...@free.fr>
Subject Re: svn commit: r1726407 - in /maven/doxia/doxia/trunk: ./ doxia-core/src/main/java/org/apache/maven/doxia/parser/ doxia-modules/doxia-module-apt/src/main/java/org/apache/maven/doxia/module/apt/ doxia-modules/doxia-module-docbook-simple/src/main/java/org/a...
Date Thu, 28 Jan 2016 04:13:51 GMT
in short: if you prefer the parse method with ParseRequest, I'm ok

in more details:
IMHO, the Parser interface tell what we want it to tell: yes, currently it was 
done as there is no state, and while adding this emitComment property, I 
changed it to be stateful.
I'm not convinced that making the interface stateful is an issue: it's a 
choice (and I confess I didn't check that Plexus did create an instance on 
each independent use)
but if you prefer to change the API to make Parser interface back to 
stateless, and put the state in ParserRequest, I'm ok with that. The only 
drawback I see is that this state has to be passed on each API call. Notice 
that while thinking more at it, there is already a lot of state in Parser 
implementations: you can't use one Parser instance in multiple threads for 
treating multiple contents in parallel
then IMHO this question of "making Parser statefull" seems now more as "making 
clear that Parser implementations are stateful"

Regards,

Hervé

Le mercredi 27 janvier 2016 20:49:25 Robert Scholte a écrit :
> If it is a per-file setting, then I think this approach is wrong (even if
> it is a singleton or not).
> 
> The Parser interface only tells us what kind of parser it is ( text, xml
> or unknown ) and it has the parse-method (ignoring the LogEnabled
> interface :) ).
> 
> So only if you can say that for instance every AptParser must always
> emitComments, then it is fine here, but then there's no need for a setter.
> 
> I would say that it must be an argument of the parse-method. In fact, I
> already added the sourceName too.
> Looks like we need a ParseRequest to be able to support advanced parsing.
> 
> thanks,
> Robert
> 
> [1]
> http://maven.apache.org/doxia/doxia/doxia-core/apidocs/org/apache/maven/doxi
> a/parser/Parser.html
> 
> 
> Op Wed, 27 Jan 2016 04:05:01 +0100 schreef Hervé BOUTEMY
> 
> <herve.boutemy@free.fr>:
> > Doxia being a generic lib, able to be used in any context and not just
> > maven-
> > site-plugin, we need to make our own choice on how flexible we want to be
> > 
> > regarding how this API is used in Doxia Sitetools for a site rendering,
> > the
> > value is set with static value (false) on each source file parsing
> > 
> > when used by doxia-converter, it is not expected to have non-default
> > value, ie
> > it is expected to render comments
> > 
> > and I don't know current case where we have a mixed run maven-site-
> > plugin+doxia-converter, but in theory, it is possible
> > 
> > 
> > then Parser implementation IMHO should not be a singleton, or we're at
> > risk
> > 
> > Regards,
> > 
> > Hervé
> > 
> > Le mardi 26 janvier 2016 21:26:18 Robert Scholte a écrit :
> >> Hi Hervé,
> >> 
> >> based on only the interface it is possible that the Parser
> >> implementation
> >> is a singleton.
> >> Is the emitComments meant to be a per-Parser setting or a per-file
> >> setting?
> >> 
> >> thanks,
> >> Robert
> >> 
> >> Op Sat, 23 Jan 2016 16:48:08 +0100 schreef <hboutemy@apache.org>:
> >> > Author: hboutemy
> >> > Date: Sat Jan 23 15:48:07 2016
> >> > New Revision: 1726407
> >> > 
> >> > URL: http://svn.apache.org/viewvc?rev=1726407&view=rev
> >> > Log:
> >> > [DOXIA-482] added an API to define if comments from source markup are
> >> > emitted as Doxia comment events
> >> 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxi
> >> 
> >> >     a/parser/AbstractParser.java
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/dox
> >> 
> >> >     ia/parser/AbstractXmlParser.java
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/dox
> >> 
> >> >     ia/parser/Parser.java
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/dox
> >> 
> >> >     ia/parser/XhtmlBaseParser.java
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-apt/src/main/java/
> >> 
> >> >     org/apache/maven/doxia/module/apt/AptParser.java
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src
> >> 
> >> /main/java/org/apache/maven/doxia/module/docbook/DocBookParser.java
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-fml/src/main/java/
> >> 
> >> >     org/apache/maven/doxia/module/fml/FmlParser.java
> >> >     maven/doxia/doxia/trunk/pom.xml
> >> > 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/AbstractParser.java URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/
> >> j
> >> 
> >> ava/org/apache/maven/doxia/parser/AbstractParser.java?rev=1726407&r1=1726
> >> 4
> >> 
> >> > 06&r2=1726407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== ---
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/AbstractParser.java (original)
> >> > +++
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/AbstractParser.java Sat Jan 23 15:48:07 2016
> >> > @@ -56,6 +56,11 @@ public abstract class AbstractParser
> >> > 
> >> >      /** Log instance. */
> >> >      private Log logger;
> >> > 
> >> > +    /**
> >> > +     * Emit Doxia comment events when parsing comments?
> >> > +     */
> >> > +    private boolean emitComments = true;
> >> > +
> >> > 
> >> >      private static final String DOXIA_VERSION;
> >> >     
> >> >     static
> >> > 
> >> > @@ -99,6 +104,16 @@ public abstract class AbstractParser
> >> > 
> >> >          return UNKNOWN_TYPE;
> >> >      
> >> >      }
> >> > 
> >> > +    public void setEmitComments( boolean emitComments )
> >> > +    {
> >> > +        this.emitComments = emitComments;
> >> > +    }
> >> > +
> >> > +    public boolean isEmitComments()
> >> > +    {
> >> > +        return emitComments;
> >> > +    }
> >> > +
> >> > 
> >> >      /**
> >> >      
> >> >       * Execute a macro on the given sink.
> >> >       *
> >> > 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/AbstractXmlParser.java URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/
> >> j
> >> 
> >> ava/org/apache/maven/doxia/parser/AbstractXmlParser.java?rev=1726407&r1=1
> >> 7
> >> 
> >> > 26406&r2=1726407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== ---
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/AbstractXmlParser.java (original)
> >> > +++
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/AbstractXmlParser.java Sat Jan 23 15:48:07 2016
> >> > @@ -371,7 +371,10 @@ public abstract class AbstractXmlParser
> >> > 
> >> >      protected void handleComment( XmlPullParser parser, Sink sink )
> >> >      
> >> >          throws XmlPullParserException
> >> >      
> >> >      {
> >> > 
> >> > -        sink.comment( getText( parser ) );
> >> > +        if ( isEmitComments() )
> >> > +        {
> >> > +            sink.comment( getText( parser ) );
> >> > +        }
> >> > 
> >> >      }
> >> >     
> >> >     /**
> >> > 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/Parser.java URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/
> >> j
> >> 
> >> ava/org/apache/maven/doxia/parser/Parser.java?rev=1726407&r1=1726406&r2=1
> >> 7
> >> 
> >> > 26407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== ---
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/Parser.java (original)
> >> > +++
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/Parser.java Sat Jan 23 15:48:07 2016
> >> > @@ -66,4 +66,18 @@ public interface Parser
> >> > 
> >> >       * @return the type of Parser
> >> >       */
> >> >      
> >> >      int getType();
> >> > 
> >> > +
> >> > +    /**
> >> > +     * When comments are found in source markup, emit comment Doxia
> >> > events or just ignore?
> >> > +     *
> >> > +     * @param emitComments <code>true</code> (default value)
to emit
> >> > comment Doxia events
> >> > +     */
> >> > +    void setEmitComments( boolean emitComments );
> >> > +
> >> > +    /**
> >> > +     * Does the parser emit Doxia comments event when comments found
> >> 
> >> in
> >> 
> >> > source?
> >> > +     *
> >> > +     * @return <code>true</code> (default value) if comment
Doxia
> >> > events are emitted
> >> > +     */
> >> > +    boolean isEmitComments();
> >> > 
> >> >  }
> >> > 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/XhtmlBaseParser.java URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/
> >> j
> >> 
> >> ava/org/apache/maven/doxia/parser/XhtmlBaseParser.java?rev=1726407&r1=172
> >> 6
> >> 
> >> > 406&r2=1726407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== ---
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/XhtmlBaseParser.java (original)
> >> > +++
> >> 
> >> maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/p
> >> a
> >> 
> >> > rser/XhtmlBaseParser.java Sat Jan 23 15:48:07 2016
> >> > @@ -797,7 +797,10 @@ public class XhtmlBaseParser
> >> > 
> >> >          }
> >> >          else
> >> >          {
> >> > 
> >> > -            sink.comment( text );
> >> > +            if ( isEmitComments() )
> >> > +            {
> >> > +                sink.comment( text );
> >> > +            }
> >> > 
> >> >          }
> >> >      
> >> >      }
> >> > 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-apt/src/main/java/org/
> >> a
> >> 
> >> > pache/maven/doxia/module/apt/AptParser.java URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia->
>> m
> >> 
> >> odule-apt/src/main/java/org/apache/maven/doxia/module/apt/AptParser.java?
> >> r
> >> 
> >> > ev=1726407&r1=1726406&r2=1726407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== ---
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-apt/src/main/java/org/
> >> a
> >> 
> >> > pache/maven/doxia/module/apt/AptParser.java (original)
> >> > +++
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-apt/src/main/java/org/
> >> a
> >> 
> >> > pache/maven/doxia/module/apt/AptParser.java Sat Jan 23 15:48:07 2016
> >> > @@ -2231,7 +2231,10 @@ public class AptParser
> >> > 
> >> >          public void traverse()
> >> >          
> >> >              throws AptParseException
> >> >          
> >> >          {
> >> > 
> >> > -            AptParser.this.sink.comment( text );
> >> > +            if ( isEmitComments() )
> >> > +            {
> >> > +                AptParser.this.sink.comment( text );
> >> > +            }
> >> > 
> >> >          }
> >> >      
> >> >      }
> >> > 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/mai
> >> n
> >> 
> >> > /java/org/apache/maven/doxia/module/docbook/DocBookParser.java URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia->
>> m
> >> 
> >> odule-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/
> >> D
> >> 
> >> > ocBookParser.java?rev=1726407&r1=1726406&r2=1726407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== ---
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/mai
> >> n
> >> 
> >> > /java/org/apache/maven/doxia/module/docbook/DocBookParser.java
> >> 
> >> (original)
> >> 
> >> > +++
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/mai
> >> n
> >> 
> >> > /java/org/apache/maven/doxia/module/docbook/DocBookParser.java Sat
> >> 
> >> Jan 23
> >> 
> >> > 15:48:07 2016
> >> > @@ -514,7 +514,10 @@ public class DocBookParser
> >> > 
> >> >          }
> >> >          else
> >> >          {
> >> > 
> >> > -            sink.comment( text );
> >> > +            if ( isEmitComments() )
> >> > +            {
> >> > +                sink.comment( text );
> >> > +            }
> >> > 
> >> >          }
> >> >      
> >> >      }
> >> > 
> >> > Modified:
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-fml/src/main/java/org/
> >> a
> >> 
> >> > pache/maven/doxia/module/fml/FmlParser.java URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia->
>> m
> >> 
> >> odule-fml/src/main/java/org/apache/maven/doxia/module/fml/FmlParser.java?
> >> r
> >> 
> >> > ev=1726407&r1=1726406&r2=1726407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== ---
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-fml/src/main/java/org/
> >> a
> >> 
> >> > pache/maven/doxia/module/fml/FmlParser.java (original)
> >> > +++
> >> 
> >> maven/doxia/doxia/trunk/doxia-modules/doxia-module-fml/src/main/java/org/
> >> a
> >> 
> >> > pache/maven/doxia/module/fml/FmlParser.java Sat Jan 23 15:48:07 2016
> >> > @@ -404,7 +404,10 @@ public class FmlParser
> >> > 
> >> >          }
> >> >          else
> >> >          {
> >> > 
> >> > -            sink.comment( comment );
> >> > +            if ( isEmitComments() )
> >> > +            {
> >> > +                sink.comment( comment );
> >> > +            }
> >> > 
> >> >          }
> >> >      
> >> >      }
> >> > 
> >> > Modified: maven/doxia/doxia/trunk/pom.xml
> >> 
> >> > URL:
> >> http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/pom.xml?rev=1726407&
> >> r
> >> 
> >> > 1=1726406&r2=1726407&view=diff
> >> 
> >> =========================================================================
> >> 
> >> > ===== --- maven/doxia/doxia/trunk/pom.xml (original)
> >> > +++ maven/doxia/doxia/trunk/pom.xml Sat Jan 23 15:48:07 2016
> >> > @@ -272,6 +272,8 @@ under the License.
> >> > 
> >> >                  <exclude>org/apache/maven/doxia/module/site</exclude>
> >> 
> >> <exclude>org/apache/maven/doxia/module/site/**</exclude>
> >> 
> >> <exclude>org/apache/maven/doxia/module/*/*SiteModule</exc
> >> 
> >> >                  lude>
> >> > 
> >> > +                <!-- DOXIA-482 -->
> >> > +
> >> 
> >> <exclude>org/apache/maven/doxia/parser/Parser</exclude>
> >> 
> >> >                </excludes>
> >> >              
> >> >              </configuration>
> >> >            
> >> >            </execution>
> >> 
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Mime
View raw message