asciidoctor / docbookrx

(An early version of) a DocBook to AsciiDoc converter written in Ruby.
MIT License
22 stars 49 forks source link

Nested <programlisting> element in <para> in <listitem> not handled correctly #41

Closed mrietveld closed 8 years ago

mrietveld commented 8 years ago

This is related to #25:

This is actually a bug.

The format_text method in lib/docbookrx/docbookrx_visitors.rb calls @lines.pop, which causes problems when we have nested elements.

For example:

   <listitem>
     <para>get all process definitions</para>
     <para>
       <programlisting>Collection definitions = runtimeDataService.getProcesses(new QueryContext());</programlisting>
     </para>
   </listitem>
   <listitem>
     <para>get active process instances
       <programlisting>Collection&lt;processinstancedesc> instances = runtimeDataService.getProcessInstances(new QueryContext());</programlisting></para>
   </listitem>

The first listitem is formatted correctly -- while the second is not. The problem is that the code doesn't expect the <para> element to contain the <programlisting>, and pops the formatting for the [source] ("----" in this case), so that you get something that looks like this:

* get all process definitions
+

[source]
----
Collection definitions = runtimeDataService.getProcesses(new QueryContext());
----

[source]
----
 Collection<processinstancedesc> instances = runtimeDataService.getProcessInstances(new QueryContext());
* ----
get active nodes for given process instance

The problem is that we have the following code in format_text:

    if node.is_a? ::Nokogiri::XML::Node
      append_blank_line
      proceed node
      @lines.pop
    else
      nil
    end

and in visit_listitem we have:

 def visit_listitem node
    elements = node.elements.to_a
    item_text = if elements.size > 0
      format_text elements.shift
    else
      format_text node
    end
    # create line with marker and item_text and append it.. 

This means that the text is appended to @lines, then the <programlisting> element is processed (and added to @lines by proceed node in format_text) and only then is the marker added.. which gives us the incorrect asciidoc shown above.

mrietveld commented 8 years ago

Fixed here: https://github.com/mrietveld/docbookrx/commit/71f1dd68c2198186926c18150c12072834138ddd

mojavelinux commented 8 years ago

Thanks for this fix. What worries, me though, is that it seems like this is invalid DocBook and there are a lot of such scenarios that perhaps we shouldn't fix. Is it even valid to have a program listing inside a paragraph?

I'm okay with addressing corner cases that appear frequently, perhaps like this one, but I think we should have a least some restrictions on what DocBook we allow. We can expect some cleanup occur before we touch the DocBook. (Another goal, perhaps, for the planned contributing guide).