Hi Trygve,
I like your points :) My comments inside.
[snip]
>
> + /**
> + * Default constructor
> + *
> + * @param sectionEntry
> + */
> public BookIndexingSink( IndexEntry sectionEntry )
> {
> stack.push( sectionEntry );
> }
These comments are pretty useless, I'd rather not sprinkle the code with
obvious comments.
Agree but if no comment, nobody thinks to had comment, specially for
new and important updates. my2cents ;)
IMHO I think that Doxia needs to be review at large to add comment.
[snip]
> -// public void definedTerm()
> -// {
> -// type = TYPE_DEFINED_TERM;
> -// }
> -//
> -// public void figureCaption()
> -// {
> -// type = TYPE_FIGURE;
> -// }
> -//
> -// public void tableCaption()
> -// {
> -// type = TYPE_TABLE;
> -// }
> + // public void definedTerm()
> + // {
> + // type = TYPE_DEFINED_TERM;
> + // }
> + //
> + // public void figureCaption()
> + // {
> + // type = TYPE_FIGURE;
> + // }
> + //
> + // public void tableCaption()
> + // {
> + // type = TYPE_TABLE;
> + // }
Any special reason for this other than you identing it by accident?
Well, it is not an accident: it is the Eclipse formatter with the
Maven Code Style
(http://maven.apache.org/guides/development/guide-m2-development.html)
Promise, I will try idea ASAP ;)
>
> public void text( String text )
> {
> IndexEntry entry;
>
> - switch( type )
> + switch ( type )
> {
> case TITLE:
> this.title = text;
> @@ -137,15 +168,7 @@
> // Sanitize the id. The most important step is to remove any
blanks
> //
-----------------------------------------------------------------------
>
> - String id = text;
> - id = id.toLowerCase();
> - id = id.replace( '\'', '_' );
> - id = id.replace( '\"', '_' );
> - id = id.replace( ' ', '_' );
> -
> - //
-----------------------------------------------------------------------
> - //
> - //
-----------------------------------------------------------------------
> + String id = HtmlTools.encodeId( text );
Ah, I knew it was somewhere.
It is a new method ;)
[snip]
> {
> if ( !context.getOutputDirectory().mkdirs() )
> {
> - throw new BookDoxiaException(
> - "Could not make directory: " +
context.getOutputDirectory().getAbsolutePath() + "." );
> + throw new BookDoxiaException( "Could not make directory: "
> + + context.getOutputDirectory().getAbsolutePath() + "." );
> }
> }
>
> - //
-----------------------------------------------------------------------
> - //
> - //
-----------------------------------------------------------------------
> -
I like these, they are separators between logical parts of the method.
It is not a Maven standard style thus I removed it. Is it for Doxia?
If yes, we need a developping guide.
[snip]
> +
> + //
-----------------------------------------------------------------------
> // Private
> //
-----------------------------------------------------------------------
>
> + /**
> + * Render the book, ie the book index and all chapter index
> + *
> + * @param book
> + * @param context
> + * @throws BookDoxiaException if any
> + */
> private void renderBook( BookModel book, BookContext context )
> throws BookDoxiaException
> {
> - //
-----------------------------------------------------------------------
> - // Render the book index.xml page
> - //
-----------------------------------------------------------------------
> -
> File index = new File( context.getOutputDirectory(), "index.xml" );
>
> try
> @@ -86,12 +137,6 @@
> }
>
> //
-----------------------------------------------------------------------
> - // Render the index.html files for each chapter
> - //
-----------------------------------------------------------------------
> -
> - // TODO: Implement
> -
> - //
-----------------------------------------------------------------------
> // Render all the chapters
> //
-----------------------------------------------------------------------
Ditto here about the commends. They explain the flow in the code.
IMHO javadoc should provide it ;) developing guide...?
[snip]
> + protected String getString( String key )
> + {
> + if ( StringUtils.isEmpty( key ) )
> + {
> + throw new IllegalArgumentException( "The key cannot be empty" );
> + }
> +
> + return i18n.getString( "book-renderer", Locale.getDefault(), key
).trim();
> + }
This should probably be moved to the i18n component or at least to a
I18nUtil.
+1
Thanks!
Vincent
