Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate duplicate block and paragraph templates #703

Open
gdamore opened this issue Jul 7, 2020 · 4 comments
Open

Consolidate duplicate block and paragraph templates #703

gdamore opened this issue Jul 7, 2020 · 4 comments

Comments

@gdamore
Copy link
Collaborator

gdamore commented Jul 7, 2020

We have some templates which are basically the same, but only differ because of the parser context -- but otherwise carry identical (or should be identical) data and result in identical output.

We should consolidate these:

  • quote
  • verse
  • admonition
  • source

There are probably some others but these are obviously the same (or sufficiently nearly so that they could be made so trivially).

@gdamore
Copy link
Collaborator Author

gdamore commented Jul 7, 2020

So I started looking at this, and its kind of a bigger mess than I'd really like -- the problem is that there are subtle differences in whitespace -- for example in the admonition block when an icon is used, we do we put the closing </td> on the same line as adjacent text or on the next line.

The answer to this question won't affect the output one bit, but paragraphs and delimited blocks have different answers -- I presume this relates to an attempt to achieve the highest fidelity to asciidoctor possible.

I'd like to relax that fidelity where non-significant whitespace is concerned -- it would be better to generate "correct" output, and reduce the overall complexity of the code base, than to try so hard for a 1:1 match with asciidoctor.

(Having said that -- I know that for some comparing the results will be important -- it's an easier to choice to make to switch toolchains when diff says there is no change.)

@xcoulon
Copy link
Member

xcoulon commented Jul 7, 2020

I have mixed feelings about this change: on the one hand, I agree with you that this looks like code duplication, but on the other hand and as you also pointed out, we obtain the exact same output as Asciidoctor, which should give users more confidence if they want to switch.
But I agree with you that the rendering in the browser may not be affected by the extra whitespaces (or the lack of thereof)

@xcoulon xcoulon added this to the backlog milestone Jul 7, 2020
@gdamore
Copy link
Collaborator Author

gdamore commented Jul 7, 2020

I'm starting to rethink this for another reason.

The issue is nesting of <p> elements.

We would like to have outermost paragraphs identified as semantically <p>. But content that might appear nested (e.g. in a table cell or as part of a list item) the use of <p> is actually wrong, and can create problems for some UAs. For those it's better to use <div>.

@xcoulon
Copy link
Member

xcoulon commented Jul 7, 2020

sure, list items and table cells should actually use <div>, indeed. But that's another issue, isn't it?
I mean it's not directly related to shared templates for blocks and paragraphs. Or am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants