redhat-documentation / modular-docs

Modular Documentation Project provides guidelines and examples for writing technical documentation using a modular framework.
Creative Commons Attribution Share Alike 4.0 International
82 stars 68 forks source link

Recommend spacing out includes #165

Closed jherrman closed 2 years ago

jherrman commented 3 years ago

Adding a blank line between includes in assemblies is a good practice, because it prevents mysterious errors when the last line of one include and the first line of the next one are treated as two consecutive lines.

Therefore, it would be useful to include this in the assembly template, both by spacing out the placeholder includes and by adding an explicit recommendation in a comment.

rolfedh commented 3 years ago

How common are these mysterious errors? Do they happen under specific conditions or are they completely sporadic?

jherrman commented 3 years ago

I haven't tested it thoroughly, but I think one of the scenarios is when the included module ends in a comment, preceded by a list, and the following include starts with a comment, too.

Still, since adding a blank line is a pretty reliable way to prevent the problems, I'd say making the recommendation is more important than identifying all the specific problematic scenarios.

VladimirSlavik commented 3 years ago

The necessary conditions are:

  1. The higher level file has includes on adjacent lines, like so:
    include::something.adoc[]
    include::another.adoc[]
    include::final.adoc[]
  2. An included file that is not last in the list of includes is missing an empty line at its end.
  3. The next included file does not have an empty line as first.

The effect is, broadly, that the first line of the following included file is misinterpreted, and with it any other items that depend on correct syntax.

IIRC in practice this manifests mostly as messed up / missing ID and heading of the next file. However, if the incorrectly-terminated file has a table, list item or block right at the end, you get various hilarious and counterintuitive results, such as two headings missing, ToC mysteriously ended, "suddenly margin increases", half of book lumped into a table cell, etc.

The reasons this matters enough to care are:

Obligatory disclaimer: Not a writer for the past two years. If your AsciiDoc tooling suddenly got far better, not all of the above might apply. But I doubt that.

rolfedh commented 3 years ago

Spacing the includes lines in the templates seems harmless, but we should avoid adding to writers' cognitive load by providing too much explanation in the guide.

Preventing asciidoc build issues like this one should be handled by an asciidoc linter in our doc CI tool chain (e.g., Travis CI).

VladimirSlavik commented 3 years ago

we should avoid adding to writers' cognitive load by providing too much explanation in the guide.

Agreed. The details are material for elsewhere. I posted that wall of text mainly to provide answers to your questions.

asciidoc linter

Yes please. Make tools remember the rules. In such a case, it's actually fine to not space includes, as the rule can be "last line is blank".

rolfedh commented 3 years ago

Yes please. Make tools remember the rules. In such a case, it's actually fine to not space includes, as the rule can be "last line is blank".

So maybe a better solution is to add a comment to the penultimate line of the templates saying: // Keep the following blank line at the end of all your files.

jherrman commented 3 years ago

So maybe a better solution is to add a comment to the penultimate line of the templates saying: // Keep the following blank line at the end of all your files.

I don't think so, that would add a potential "visual noise" line to every last module and assembly.

On the other hand, adding something like // Ensure your includes have blank lines between them would only have to be included in assemblies (and potentially in a master.adoc template)

rolfedh commented 3 years ago

Sure, okay. How can we help prevent the root problem: missing blank lines?

msuchane commented 3 years ago

I think the best solution is to surround every include with blank lines, before and after.

The mysterious issues might happen not only when one included file breaks the syntax of the next included file, but the missing blank line can also have unwanted effects within the assembly.

Here's an example that I've seen recently in the wild:

[role="_abstract"]
The abstract.
//A comment
include::con_concept1.adoc[leveloffset=+1]
include::con_concept2.adoc[leveloffset=+1]
include::con_concept3.adoc[leveloffset=+1]

The result is that concept1 becomes a 0-level chapter, as if it had no level offset, and the other two concepts are then nested in it as subsections of concept1.

You can fix this problem by adding a blank line before the first include.

As for the solution in the templates, we can add a comment recommending to use blank lines between includes, as @jherrman said.

But we should also edit the example includes that the assembly template has, because currently, the blank line is missing there. I'd expect that following the example would be easier for writers than interpreting a comment.

mikemckiernan commented 3 years ago

+1 to using blank lines surrounding include:: lines as a convention and I feel like a sentence or two related to improving human readability of the file and the benefit of ensuring that included files are not interpreted as a single block is beneficial.

Regarding the second issue of ensuring files end with a blank line, pre-commit has an end-of-file-fixer hook, but that is likely a heavier lift than anyone who is not already a pre-commit fan wants to make.

Follow up: The end-of-file-fixer trims the end of the file to a \n character, but does not ensure that the file ends with a blank line. I've begun to fiddle with a hook that ensures the file ends with a blank line, but I have no experience in this area.