resolves #32 align options with Asciidoctor ecosystem

[mojavelinux]

• use double-hyphen prefix for long options (where possible)
• rename "include-basedir" to just "basedir"
• rename "stylesheetfile" to just "stylesheet"

[benevans]

Actually the "-stylesheetfile" option is a standard doclet option, we shouldn't be changing that.

Edit: we just test for its presence to see if the user wanted to use their own stylesheet. If it's not present then we copy the asciidoctor stylesheets to the output dir.

[mojavelinux]

Dang it. Okay, we can revert that one back.

So what I'm going for here is that anything that's a standard doclet option will have a single hyphen, and anything that's specific to Asciidoctor (i.e., custom) should have two.

...unless you just want to can the idea and stick with the single attribute.

To be consistent, maybe we should switch "attributes-file" to "attributesfile" then. wdyt?

[benevans]

The use of double-hyphen for asciidoclet-specific options and also "--attributesfile" makes sense to me.

What about "-r"? Should it be "--require" like asciidoctor?

[mojavelinux]

Technically, Asciidoctor supports both. Is it possible to support both in Asciidoclet?

Would you be willing to send a pull request to my branch with the changes. (Atm, I'm busy getting the Maven plugin in order).

I've thought about this a bit more and it makes sense to have the double-hypen for Asciidoclet-specific options. That way, we don't step on built-in options and people are fully aware which tool is receiving the option.

[johncarl81]

I think we can support both... we just need a strategy of handling one or the other if both are defined.

[mojavelinux]

In that case, I usually just go with last wins and assume that the user just needs to use a little sense using the same name of an option twice ;)

[benevans]

Pretty simple to support both, will PR your branch soon - only problem will be if some future version of javadoc decides to use "-r" for something else. But there will be plenty of time to migrate if that ever happens!

Also, WDYT about the current "--attributes" option? Currently this is a semicolon-delimited list of attributes (can't use comma, see here). Instead we could support multiple "-a" or "--attribute" options like Asciidoctor does?

[mojavelinux]

Great!

I like the idea of supporting multiple attributes instead of a single one that is delimited. We may come back to a delimited list as an option once we have something more standardized across the ecosystem. Right now, it relies on something which instead easily reproducible in this environment (space separated, any spaces in content have to be escaped with a backslash). It's a whole lot easier to read with multiple options IMO.

[benevans]

Right now, it relies on something which instead easily reproducible in this environment (space separated [...]

Not sure what you're referring to here... where are space separated attributes used?

[mojavelinux]

Asciidoctor supports many ways of specifying attributes (via the core API). One of those ways is a space separated string. See https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor.rb#L1284.

The problem is, if there are spaces in the value, they have to be escaped using a backslash. Since many of the values that we would set in the Asciidoclet configuration are resolved from Maven properties, there's no easy way to perform this escaping.

We need to decide in the ecosystem on a delimiter that we can support across the board. Perhaps it will be a semi-colon. Once we decide, then we can align on that. But we want to avoid making conventions like this in subprojects before it goes upstream.

That's why I'm in favor of having attributes specified one at a time.

[benevans]

OK, so if we allow multiple occurrences of attribute args then we can avoid delimiters altogether:

-a foo=bar -a x=y -a baz

If using a property that may contain spaces then you can just put the attribute in quotes:

-a "foo=${project.name}"

Does this sound OK?

[mojavelinux]

Sounds perfect!

[benevans]

Hmm got a bit of an issue here. Gradle's javadoc task assumes that all javadoc options start with a single hyphen, so this is omitted when you add the option. For example to add the option -foo bar:

javadoc {
    ...
    options.addStringOption "foo", "bar"
}

So if you wanted to use --base-dir or --attribute you'd need to do this:

options.addStringOption "-base-dir", "${projectDir}"
options.addStringOption "-attribute", "foo=bar"

... which will be a bit confusing for Gradle users when the docs say to use --base-dir etc.

So we might just have to stick to single-hyphen names to avoid confusion.

[mojavelinux]

I became aware of this issue when editing the README.

I will say this. What Gradle is doing here is actually confusing even with single hyphen options. I don't think it should be messing with the hyphens at all. If we make it clear that Gradle automatically adds one hyphen, then it's not too much of a stretch to understand why a single hyphen is required for an option that takes two hyphens. It's always one more than you type.

I don't see a problem with explain the "+1 hyphen" thing. However, if you feel strongly about making all options take only a single hyphen, I won't oppose it.

[aalmiray]

Should we raise the "implicit hyphen" issue to the Gradle forum?

[mojavelinux]

I think we should. This is the kind of thing that I think turns people off of Gradle. It's good when a tool helps. It's bad when it overreaches and starts breaking conventions.

To provide a bit more context, it was really bad of Sun to use the single form for long options...as it goes against the Unix conventions. It was even worse for Gradle to confuse the issue by adding a layer of abstraction and reinforcing this bad practice. I think Gradle should just get out from the middle of this issue and let flags be set as they are defined.

[mojavelinux]

To make my position clear, my primary criticism is with Sun for being oblivious to the conventions of platform on which they built their business (Unix). Seriously? WAT?

[benevans]

Just came across a more serious issue with Gradle's javadoc task: it doesn't support multiple occurrences of the same option. It puts options into a map by name, so only the last occurrence is used.

A workaround is to use options.optionFiles to read options from a file, but then you lose the ability to easily pass in build properties.

This would only affect the new -a/--attribute and -r/--require options. Perhaps these could be overloaded to still take a list of values as an argument so Gradle users can do something like:

options.addStringOption "-attribute", "foo=bar; x=${project.x}"
options.addStringOption "-require", "asciidoctor-diagram; foo; bar"

...or make these variants separate options altogether.

[mojavelinux]

Wow, this issue has some fight in it, doesn't it?

Let's overload so that Gradle can pass the options. To reinforce they should only be used in this environment, can we go with comma as the delimiter? If you feel strongly about using the semi-colon, I'm fine with that, but otherwise let's go with comma.

[benevans]

Agreed, comma is fine for this.

[johncarl81]

Hey guys, how close is this to completion? This is the only issue holding up the 1.5.0 release.

Also, before I merge you rebase against master?

[mojavelinux]

I think we've got the design worked out. Ben, need any help with the code changes?

[benevans]

All good, ready to go. Since this PR originally came from Dan's repo, and master has diverged since then, do I need to raise a new PR rebased from master?

[johncarl81]

Oh yeah, this did come from Dan's repo. No need to issue another PR, just push your branch to github and tell me what to merge and I'll take care of the rest.

[benevans]

OK, cool. These changes are now in opencloud/asciidoclet:issue-32.

[johncarl81]

Looks good. @mojavelinux, do you want to weigh in before I merge?

[mojavelinux]

Go for it!

[johncarl81]

Done! Thanks again guys.