asciidoctor / asciidoctor-pdf

:page_with_curl: Asciidoctor PDF: A native PDF converter for AsciiDoc based on Asciidoctor and Prawn, written entirely in Ruby.
https://docs.asciidoctor.org/pdf-converter/latest/
MIT License
1.13k stars 500 forks source link

min-height-after: auto ignores %unbreakable table option #2520

Closed pstueck closed 1 month ago

pstueck commented 1 month ago

AsciiDoctor-Pdf as of IntelliJ plugin 0.41.14 resp. Asciidoctor PDF 2.3.15 using Asciidoctor 2.0.23

When I set the style entry heading-min-height-after: auto, the table caption no longer respects %unbreakable (in the table’s attribute list). Setting a fixed value (e.g. heading-min-height-after: 20) or deactivating the entry completely, keeps the %unbreakable table and its table caption together again.

Actual known workaround: enclose table and caption in an unbreakable block. Just as I did that … without this wrapping, asciidoctor-pdf seems to think that a heading immediately followed by an unbreakable table do not belong together.

  1. With wrapper and min-height-after-auto (images clipped for privacy reasons) ... image
  2. Without wrapper (the table options say unbreakable) and min-height-after-auto ... image
  3. And without wrapper and without min-height-after ... image

Personally I think, the rendering here should always happen as in 1.

mojavelinux commented 1 month ago

Please provide a reproducible AsciiDoc sample that can be used to demonstrate this issue on a separate machine.

pstueck commented 1 month ago

2520.zip forgot chapter 5 2520.zip

Was afraid, you’ll ask for that :laughing: and here we go ... the samples should work as is with the default PDF theme (which is locally a tad bit modified, to better reflect the use case I’ve screen-shot above; in case you do want to reproduce exactestly, I can provide an extended theme with “Albert Sans” as the main font).

Personally I think “mha-auto”, 1.2.1 & 4.2.1 are the way to go. Them should just work without the additional wrapper, IMHO. On the other hand, neither 2.2.1 nor 3.2.1 should ever happen (a table caption belongs more to the table than the heading, I’d say; even the “mha-default” results—heading-min-height-after not set—look more reasonable for those sections).

Chapter 5 resp. 5.2.1 shouldn’t happen either IMO … just because there’s not enough room for the table header and the first row, caption and table shouldn’t be separated.

mojavelinux commented 1 month ago

Aha, I see what's happening. The described behavior is what the converter is trying to do. However, when the converter converts the table in the scratch document, it is inadvertently removing the breakable or unbreakable option from the table. So when it goes to convert it a subsequent time (perhaps in another scratch document), the option is no longer there. I need to update the code to ensure it doesn't drop the option as a side effect of doing the rendering test.

mojavelinux commented 1 month ago

Note that it will still be necessary to add either %breakable or %unbreakable on the table itself. The converter has no way of "seeing" a title, so it has to be affixed to the table for reasons described in the docs. But the explicit open block wrapper should not be necessary (once this is fixed). (In fact, what the converter is doing is automatically wrapping the table in an open block).

pstueck commented 1 month ago

The converter has no way of "seeing" a title, so it has to be affixed to the table for reasons described in the docs.

I am not really sure, I am following … can you please point to the docs, you’re mentioning (surely been looking in the wrong place or not meticulously enough)?

And also … if the converter doesn’t know that a title belongs to a table, how does %unbreakable for the table itself fix that? IMHO a table caption does belong to the table … and should even be repeated when a table is split across pages. :thinking:

mojavelinux commented 1 month ago

I am not really sure, I am following … can you please point to the docs,

https://docs.asciidoctor.org/pdf-converter/latest/breakable-and-unbreakable/#table

how does %unbreakable for the table itself fix that?

Because internally the converter wraps the table with the caption in an unbreakable block. That's why breakable or unbreakable is always required on a table.

IMHO a table caption does belong to the table … and should even be repeated when a table is split across pages.

I get that, but we are limited by the underlying capabilities of the PDF generator...and this is the solution we came up with. We can't do any better for reasons cited in the docs.

To be clear, %breakable and %unbreakable will work directly on the table as described, even when heading-min-height-after: auto is set, once this bug is fixed. The fix is that the theme setting should not be changing the behavior (it is currently an unintended side effect of the logic for that theme setting).

(I know what fix needs to be made and I am preparing it).

pstueck commented 1 month ago

Thanks for the detailed explanation (and pointing me to the right doc-page … although I must admit: been there, read it … and—of course—forgot some details).

Looking forward to the next release with the fix.