Add feature to convert from condition to ifdef for #43

junaruga commented 8 years ago

I added new feature about condition. https://github.com/asciidoctor/docbookrx/issues/43

The "condition" of DocBook XSL is converted to "ifdef".

There are several restriction for usage.

The condition is not supported for several XML nodes, and cases. For example.
- The node itself is not supported for DocBookRX itself that warns "No visitor defined for <#{node.name}>! Skipping.", it does not work.
- subtitle node in section, productname,
- xi:include (include tag looks supported, but xi:include looks not supported.)

Could you check my code? Thanks.

mojavelinux commented 8 years ago

Overall, this looks good. I think we should use the single-line conditional in the case of inline elements like phrase.

a
ifdef::condition[b]
c

Does that seem reasonable?

junaruga commented 8 years ago

Hi, @mojavelinux Thanks for your checking.

Well, when I compared single line: ifdef , and multi line ifdef,

Comparing single line ifdef and multi line ifdef

single line ifdef

:foo:

a
ifdef::foo[b]
c

multi line ifdef

:foo:

a
ifdef::foo[]
b
endif::foo[]
c

both cases will get below same result. I checked it by asciidoctor firefox plugin.

a b c

However, multi line ifdef looks better than single line ifdef. Because

Multi ifdef can support to display multi lines in the context of ifdef.
behavior of "]" in the ifdef context, is easier to understand.

Merit 1

For example of merit 1, in below case, user can expect to display the context (2 lines) with condition "foo".

<para condition="foo">
Today is Friday.
Tomorrow is Saturday.
</para>

But below actual generated result does not work.

:foo:

ifdef::foo[Today is Friday.
Tomorrow is Saturday.]

This actual result for html with asciidoctor firefox plugin, is ifdef::foo[Today is Friday. Tomorrow is Saturday.]. because single-line ifdef is not recognized. It seems that single-line ifdef exists for single line context.

I do not want to use below way that works in this case, because this looks more complicated.

ifdef::foo[Today is Friday.]
ifdef::foo[Tomorrow is Saturday.]

However below multi line ifdef works for this case, easily.

:foo:

ifdef::foo[]
Today is Friday.
Tomorrow is Saturday.
endif::foo[]

Merit 2

For example of merit 2 you can see in below case.

:foo:

a
ifdef::foo[b]c]
d

This actual result works. "a b]c d" is displayed. But When open the adoc file with vim, vim syntax highlight for adoc mode, fails to show correct highlight. So, this is the evidence that behavior of "]" in the single line ifdef context, is not clearly for users including vim itself.

So, I prefered "multi line ifdef" than single line ifdef in this time.

How do you think?

junaruga commented 8 years ago

Hi, @mojavelinux How do you think? If you agree my opinion, and you like to use multi line ifdef, as I found conflicts between my pull-request and current master, I can solve the conflict, and can do rebase my branch for the pull-request, by myself.

junaruga commented 8 years ago

Hi, @mojavelinux ( Cc: @mrietveld ) After my previous comment, 7 days has been passed. I would like you to merge this modification to master branch. Is there any problem for you?

Thanks.

mrietveld commented 8 years ago

Merged @ https://github.com/asciidoctor/docbookrx/commit/01412b9bc68a803d9fba955c33cbc1713d5e16d6

junaruga commented 8 years ago

@mrietveld Thanks!!

asciidoctor / docbookrx