asciidoctor / asciidoclet

:clipboard: A Javadoc Doclet based on Asciidoctor that lets you write Javadoc in the AsciiDoc syntax.
https://github.com/asciidoctor/asciidoclet
Apache License 2.0
133 stars 40 forks source link

Resolves #5 #6

Closed LightGuard closed 11 years ago

LightGuard commented 11 years ago

The main problem with this not working was setting the base directory for inclusions, especially in SAFE mode, which is what we're using.

This fixes that problem by requiring a new parameter -include-basedir to be set for the doclet to work. I'm also demonstrating this by using it in the usage section.

johncarl81 commented 11 years ago

This is interesting. I expected this PR to be incompatible with #4, but it looks like the special characters (@, /, etc) are handled properly. Not sure exactly why....

Here's what I mean:

This tag:

 * [source,java]
 * --
 * include::src/main/java/org/asciidoctor/AsciidocletExample.java[lines=1..24]
 * --

Referencing this class:

package org.asciidoctor;

import com.sun.javadoc.Doclet;
import com.sun.javadoc.RootDoc;
import com.sun.tools.doclets.standard.Standard;

/**
 * = Asciidoclet
 *
 * A Javadoc Doclet that uses http://asciidoctor.org[Asciidoctor]
 * to render http://asciidoc.org[AsciiDoc] markup in Javadoc comments.
 *
 * @author https://github.com/johncarl81[John Ericksen]
 */
public class AsciidocletExample extends Doclet {
    private static final Asciidoctor asciidoctor = Asciidoctor.Factory.create(); // <1>

    @SuppressWarnings("UnusedDeclaration")
    public static boolean start(RootDoc rootDoc) {
        asciidoctor.render("Test", null); // <2>
        return Standard.start(rootDoc);
    }
}

Renders both @ and */ correctly in the resulting html. I expected javadoc to have problems with these as we don't post-process the rendered result from asciidoctor.

johncarl81 commented 11 years ago

Awesome work @LightGuard, thanks again!

mojavelinux commented 11 years ago

After some analysis, I think I understand when special characters need to be escaped, and when they don't.

The special characters are a problem in the Javadoc reader. When it scans a source file, it strips the leading asterisk (*) in every line that has one (it does not perform a column-wise removal, as I originally thought). It then looks for any line that has a leading at sign (@) and marks that line as a Javadoc tag.

However, it seems to not apply this logic on the source string after the initial parsing. That's why including additional content in the Javadoc comment string doesn't require special escaping.

(In other words, the Javadoc parser is extremely naive :))

Btw, the only reason we have to escape the asterisk forward slash (*/) is because the Java parser would see this as ending the comment string. Javadoc doesn't actually look for this character sequence once the comment string makes it way into the Javadoc processor.

johncarl81 commented 11 years ago

@LightGuard what do you think about making -include-basedir not required? This surprised me today as I didn't realize it was required.

LightGuard commented 11 years ago

How could we make it not required? If it isn't set then the base directory with regards to how the paths are created is based on where whatever is calling the doclet happens to call it, which in maven seems to be target/apidocs. As this is a general doclet and could be used by maven, ant, gradle, standalone, or other tools I see no way to do this correctly without explicitly requiring it.  — Sent from Mailbox for iPhone

On Tue, May 21, 2013 at 7:20 PM, John Ericksen notifications@github.com wrote:

@LightGuard what do you think about making -include-basedir not required? This surprised me today as I didn't realize it was required.

Reply to this email directly or view it on GitHub: https://github.com/asciidoctor/asciidoclet/pull/6#issuecomment-18252156

johncarl81 commented 11 years ago

What I am proposing is just not raising an error if -include-basedir is not set, i.e. removing these lines (currently 260 - 262):

if (!hasBaseDir) {
    errorReporter.printError("-include-basedir must be present");
}

and then only set .option("base_dir", this.baseDir) if baseDir != null.

I expect the result of this would be that we couldn't use the include:: feature. Is there something more that would happen / not happen?

LightGuard commented 11 years ago

Okay, that works. Yes, include files and images would not work, you get warning.  — Sent from Mailbox for iPhone

On Tue, May 21, 2013 at 10:26 PM, John Ericksen notifications@github.com wrote:

What I am proposing is just not raising an error if -include-basedir is not set, i.e. removing these lines (currently 260 - 262):

if (!hasBaseDir) {
    errorReporter.printError("-include-basedir must be present");
}

and then only set .option("base_dir", this.baseDir) if baseDir != null.

I expect the result of this would be that we couldn't use the include:: feature. Is there something more that would happen / not happen?

Reply to this email directly or view it on GitHub: https://github.com/asciidoctor/asciidoclet/pull/6#issuecomment-18256844