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
132 stars 40 forks source link

Mixed traditional and Asciidoclet javadoc markup #90

Closed sebersole closed 2 years ago

sebersole commented 6 years ago

In Hibernate we are trying to migrate to using Asciidoclet to author Javadoc. However, as Hibernate is a huge project it is simply not feasible to convert all of the docs at once so we have been working on a phased approach change pieces at a time such that we end up with a mix of traditional and Asciidoclet javadoc markup within the same project.

But in practice this has led to problems rendering the javadocs. If we enable Asciidoclet it messes up some of the traditional Javadoc markup; e.g. it escapes and "passes through" all HTML tags (you literally end up with <p/>, <li>, etc in the rendered HTML output). And if we disable Asciidoclet, of course the standard doclet butchers the Asciidoctor markup.

Are we missing something here? Does a project have to use one or the other approach? If so, that is severely limiting for larger projects to move to Asciidoclet

sebersole commented 6 years ago

Guillaume had ended up starting a nabble discussion as well - http://discuss.asciidoctor.org/Migrating-to-Asciidoclet-progressively-td6425.html

johncarl81 commented 6 years ago

Excellent point @sebersole. I imagine a solution here would be for Asciidoclet to gracefully get out of the way in certain conditions. We could add a package or class filter parameter to include or exclude certain sources from processing... would that help?

msgilligan commented 6 years ago

One alternative would be to require that Asciidoc-documented classes have a = header (or something similar) and if that is not present fall back to standard javadoc processing if the = is missing in the topmost documentation for the class. It would be easier to have the content itself have the marker, rather than requiring a filter pattern / list of classes that must be maintained.

johncarl81 commented 6 years ago

Put together a quick example here: https://github.com/johncarl81/asciidoclet/tree/include_exclude

This specifies the following regex include and exclude options:

--include "org.asciidoctor.*"
--exclude "org.asciidoctor.asciidoclet.Stylesheets"

This should allow you to convert to Asciidoclet class-by-class or package-by-package as you wish.

gsmet commented 6 years ago

I think some sort of marker would probably be easier for our use case.

But I could see how includes/excludes could be useful too.

johncarl81 commented 6 years ago

A marker to indicate class-by-class within the javadoc documentation seems noisy especially considering this should be a bridge case for converting to use Asciidoclet.

sebersole commented 6 years ago

@johncarl81 exclude/include would definitely help.

An automated solution like @msgilligan suggest would be nice as well, but honestly the Asciidoclet Javadocs we have already don't even start with the header marker (=). So that particular option really would not help us.

Whatever y'all think is the best long term solution for Asciidoc(tor/let) we can use.

gsmet commented 6 years ago

What I'm wondering is how we would maintain the list of classes to transform with Asciidoctor.

And refactorings come to mind as problematic.

I don't know if it's feasible or not but I think annotating the class Javadoc with @asciidoc or a similar marker would maybe be easier. We would have to think about it when writing the doc (and annotate the existing classes) but that would survive a refactoring.

That way, you add the marker when you write the doc and you can forget about it.

Just thinking out loud :).

johncarl81 commented 6 years ago

Good input @gsmet. How long do you think the conversion would take?

My hope was that using wildcards would help with refactors since you could convert a package and contained packages at a time to avoid an exhaustive list of every class converted, i.e.: --include org.hibernate.action.*, as long as a refactor stays within the same package. If --include is specified then everything outside of the --include regex would not be processed by Asciidoclet.

johncarl81 commented 6 years ago

Hmm, I kind of like the javadoc annotation idea... something like this?

 * ...
 * *Important* documentation
 * @author https://github.com/johncarl81[John Ericksen]
 * @version {project_version}
 * @see org.asciidoctor.Asciidoclet
 * @since 0.1.0
 * @serial (or @serialField or @serialData)
 * @asciidoclet
 */

I think this would somewhat match @msgilligan's marker idea and allow for major refactorings.

I still think this is a special case for the library, so a configuration option would be necessary. Maybe --exclude ".*"?

gsmet commented 6 years ago

Well, Steve would know more as he's the one leading the project but I'm not sure we will even convert all the codebase as it's huge.

AFAIK, they write the new classes with Asciidoc but the old ones will stay like this for a while if not ever. And a new class could be in a package with preexisting classes.

I think your include/exclude idea has value when you're actively converting an entire code base so it would be a nice feature. But I think the marker approach would better fit our current requirement.

The difficult part will be to identify the existing classes already using Asciidoctor.

gsmet commented 6 years ago

I would have gone with the shorter @asciidoc but that was the spirit :).

Can a tag be excluded so that it does not appear in the Javadoc output?

johncarl81 commented 6 years ago

Yeah, absolutely... that was my thought as well - the @asciidoclet tag wouldn't show up in the rendered javadoc and I'm open to naming it @asciidoc. Updated my branch with this idea and deployed it to 1.5.7-SNAPSHOT if you want to try it out.

gsmet commented 6 years ago

@johncarl81 thanks! I added a couple of comments in your commits.

johncarl81 commented 6 years ago

https://github.com/asciidoctor/asciidoclet/pull/91

gsmet commented 6 years ago

@sebersole could you point me to a class of the 6 code base using Asciidoctor? I'll check how this all works.

thanks!

mojavelinux commented 5 years ago

How did the testing go?

I like the @asciidoc marker. Is it true that it's only required when a certain flag is set in the configuration? In other words, Asciidoclet defaults to being applied everywhere, but that default can be flipped with an option (hence requiring the @asciidoc marker). Is that correct?

johncarl81 commented 5 years ago

Yes, that's right @mojavelinux - it's only useful when a class is excluded from being processed. For instance, we can either explicitly include classes for processing or exclude classes from processing, the resulting classes not processed can finally be added back into the set being processed by adding the @asciidoclet javadoc tag.

mojavelinux commented 5 years ago

Thanks for the clarification! That's exactly what I was hoping you'd say.

johncarl81 commented 5 years ago

@mojavelinux, @msgilligan: it might be early as we haven't gotten feedback, but would you review https://github.com/asciidoctor/asciidoclet/pull/91 and merge?

@sebersole, @gsmet: have you been able to use this feature, and, if so, does it meet your needs?

gsmet commented 5 years ago

@johncarl81 sorry about the delay... totally forgot about it.

We plan to release a new version next Wednesday so I'll check what you did later today or tomorrow. Do you think you could plan a release once you have our feedback?

Thanks!

sebersole commented 5 years ago

@mojavelinux, I actually missed the notification from this discussion. I am working on a benchmark for our current work that is super critical, but I will take a look at this again soon. But based on the comment by @johncarl81 that sounds perfect

johncarl81 commented 5 years ago

Sounds good @gsmet. Yeah, we'll make release at that point.

mojavelinux commented 5 years ago

Thanks for chiming in @gsmet and @sebersole. Sounds like we're all set to move forward. :tada:

sebersole commented 5 years ago

Was there a release of this yet to test out?

On Mon, Nov 5, 2018 at 2:18 PM Dan Allen notifications@github.com wrote:

Thanks for chiming in @gsmet https://github.com/gsmet and @sebersole https://github.com/sebersole. Sounds like we're all set to move forward. 🎉

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/asciidoctor/asciidoclet/issues/90#issuecomment-436020290, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOUE1x3oeaV9VgmyGUyQ9OxFbhXPqfMks5usJz-gaJpZM4VQP-G .

gsmet commented 5 years ago

@sebersole no you need to build a snapshot. I'll share my ORM branch with you so that you can see what I did

gsmet commented 5 years ago

@sebersole see https://github.com/sebersole/hibernate-core/pull/158 . I added instructions in the description.

sebersole commented 5 years ago

Is this available to try in any published (SNAPSHOT even) artifact yet?

johncarl81 commented 5 years ago

Not on maven central. You can, of course, build the branch and work with the SNAPSHOT locally. I'm waiting on is confirmation it fits the use case and review here: https://github.com/asciidoctor/asciidoclet/pull/91

sebersole commented 5 years ago

@johncarl81 I hate using mavenLocal. I'll grab this branch, modify the groupId and publish a SNAPSHOT to the JBoss Snapshot repo and test from there

johncarl81 commented 5 years ago

If I remember a bit later, I could publish the snapshot to maven central if that helps.

sebersole commented 4 years ago

Sorry for the late reply.. I've finally been able to test this out and it does not work for me. Probably I'm missing something simple. I use the code from #91 and build the javadoc - any docs using @asciidoc do not render anything. I saw something above about still needing to specify some configuration property to trigger @asciidoc handling - if that's still true, what setting do I need?

sebersole commented 4 years ago

What I have atm:

configurations {
    asciidoclet {
        description = 'Dependencies for Asciidoctor Javadoc taglet'
    }
}

dependencies {
    // snapshot built from #91
    asciidoclet( libraries.asciidoclet )
}

javadoc {
    configure( options ) {
        docletpath = configurations.asciidoclet.files.asType(List)
        doclet = 'org.asciidoctor.Asciidoclet'
        ...
    }

    options.addStringOption( '-exclude-asciidoclet-process', '**' )
}

with


/**
 * @asciidoclet
 *
 * Represents a selection at the SQL/JDBC level.  Essentially made up of:
 *
 *      {@link #getJdbcValueExtractor}:: How to read a value from JDBC (conceptually similar to a method reference)
 *      {@link #getValuesArrayPosition}:: The position for this selection in relation to the "JDBC values array" (see {@link RowProcessingState#getJdbcValue})
 *      {@link #getJdbcResultSetIndex()}:: The position for this selection in relation to the JDBC object (ResultSet, etc)
 *
 * Additional support for allowing a selection to "prepare" itself prior to first use is defined through
 * {@link #prepare}.  This is generally only used for NativeQuery execution.
 */
public interface SqlSelection {
}

Nothing gets rendered for the SqlSelection class-level docs

sebersole commented 4 years ago

Could the problem be the use of org.asciidoctor:asciidoctor-gradle-plugin:1.5.7 in combination with org.asciidoctor:asciidoclet:1.5.7-SNAPSHOT? The later bring a build from #91

sebersole commented 4 years ago

Could the problem be the use of org.asciidoctor:asciidoctor-gradle-plugin:1.5.7 in combination with org.asciidoctor:asciidoclet:1.5.7-SNAPSHOT? The later bring a build from #91

Nope that made no difference. So at this point, as far as I can tell, #91 does not fix our problem - https://github.com/hibernate/hibernate-orm/commit/e43c5a3166c6bd7a1f4dc4b4a617679118846718

sebersole commented 2 years ago

Is this implemented? Can someone please tell me how to use the doclet with mixed Javadoc?

sebersole commented 2 years ago

Have to assume at this point that this simply won't be implemented :(

mojavelinux commented 2 years ago

I don't think it's a matter of lack of interest. I just don't think anyone is actively working on features for this project. It's probably in need of more contributors.

mojavelinux commented 2 years ago

There's an open PR for this issue, so I'm reopening.

sebersole commented 2 years ago

Will the PR ever get integrated?

mojavelinux commented 2 years ago

If there are no objections in the next 24-48 hours, I will merge the outstanding PR (#91).

johncarl81 commented 2 years ago

Sorry I've been absent... Lets go ahead and merge.

mojavelinux commented 2 years ago

@johncarl81 Did you have any thoughts about my comment here? https://github.com/asciidoctor/asciidoclet/pull/91#issuecomment-948305206

mojavelinux commented 2 years ago

The PR that addresses this issue has now been merged. Thanks @johncarl81!! If there are further enhancements needed, let's discuss them in a separate issue.

sebersole commented 2 years ago

Any idea what version will include this?

sebersole commented 2 years ago

Is this in any released version yet? I could not find how to link up issues/prs to releases here.

mojavelinux commented 2 years ago

@johncarl81 would you be willing to publish a release?