4434

[danse]

No description provided.

[danse]

see #4438 before, as this is built on top of that

[jgm]

This brings in walk and a complete AST traversal, which we didn't need before.
Why not just add clauses like

inlineToRST (Strikeout lst)
  | null lst = return mempty
  | otherwise = do
...

That is more direct and efficient.
Alternatively one could write

inlinesToRST (Strikeout []) = return mempty
inlinesToRST (Strikeout lst) = (as before)

[danse]

it's true, this way we traverse every inline list twice once for transforming and once for writing. personally i think that the advantages in terms of readability and maintainability overcome the disadvantages related to efficiency.

a more substantial advantage of the refactoring is to enable a whole set of transformations that wouldn't be possible by writing inlines while traversing.

for instance this enables to render links or strong text making it emerge out of nesting emphasis, which is the more sophisticate solution to #4368. without an additional traverse i wouldn't even know how to drop nested inlines for the simpler solution to #4368.

so while in this specific case hasContents could get embedded in every specific inline writing function, the idea here was mainly to showcase the readability of transformation functions after the refactoring.

also in this simple case though, i struggle imagining a solution without separating transformation logic from rendering logic, even though the issue gets clear only considering some corner cases and the relationship with other issues.

consider how insertBS is applied after filter hasContents within transformInlines. that is needed in order for the test inlines do not cause the introduction of extra spaces when removed to succeed.

let's consider a list like [Emph [], Strong []]. in the solution you propose, inlineToRST is applied to every element in the list after insertBS, so a space would get added between two empty inlines, then the inlines would be removed in inlineToRST so instead of an empty output we would get a single space that did not exist in the input.

the same problem happens when merging this solution with the solution of #4329. investigating how to merge the two led me to this refactoring in the first place. my first naive soultion was to strip terminal spaces using the current recursion logic but doing so leads to syntax errors in the case of Strong [Emph [], Space, Emph[]] for instance, where spaces couldn't get detected as terminal leading to the writing of ** **. the correct solution is once again to strip spaces after filtering out empty inlines like this:

transformInlines :: [Inline] -> [Inline]
transformInlines =  insertBS .
                    stripLeadingTrailingSpace .
                    filter hasContents .
                    removeSpaceAfterDisplayMath

[jgm]

Thanks for the explanation. That helps me understand why you used the generic traversal.

[danse]

sure, sorry if this seemed rather cryptic at first sight. i added a bit more context in #4438 which is specifically about the refactoring