Split inline from block parser class, and other small refactors#1270
Split inline from block parser class, and other small refactors#1270PIG208 merged 6 commits intozulip:mainfrom
Conversation
This makes all the uses of _debugParserContext maximally simple, which will help us refactor it away.
Because this is used for parsing both inline and block nodes, moving it out will help us separate the rest of the parser code between the handling of inline content and block content.
This helps organize the parsing code a bit more cleanly -- not to mention more statically, by making the dynamic _debugParserContext assertions redundant.
This hopefully makes the logic of these loops a bit more readable.
PIG208
left a comment
PIG208
left a comment
There was a problem hiding this comment.
Looks good to me! Merging so that @rajveermalviya can rebase atop main to make use of this for #1156.
| String? parseMathBlock(dom.Element element) { | ||
| return _parseMath(element, block: true); | ||
| BlockContentNode parseMathBlock(dom.Element element) { | ||
| final debugHtmlNode = kDebugMode ? element : null; |
There was a problem hiding this comment.
Looks like we will end up with a different debugHtmlNode here, but that's a debug-mode-only behavior so it should be fine as a part of an [nfc] commit.
There was a problem hiding this comment.
Ah, yeah, would have been good for me to make that explicit.
There's also a related change that if it fails to parse, the UnimplementedNode will get the span.katex-display child instead of the p parent. I think that change is good (and it's definitely needed in the #1156 context, and in fact the current revision of #1156 effectively already makes that change) but it's not quite NFC.
I actually had that thought at one point as I was writing these changes; but I guess I forgot at the moment I was making that commit.
|
Thanks for the review! |
2 participants
This series of commits has a few small refactors of the content-parsing code, prompted by #1156.
@rajveermalviya feel free to rebase #1156 atop this if it seems helpful — the last two commits in particular might simplify that PR a bit. If the conflict resolution seems more annoying than the refactor is helpful, no need, and I can resolve it after either PR is merged.
Selected commit messages
content [nfc]: Split out _ZulipInlineContentParser class
This helps organize the parsing code a bit more cleanly -- not to
mention more statically, by making the dynamic _debugParserContext
assertions redundant.
content [nfc]: Encapsulate parsing math nodes a bit more
content [nfc]: Factor out consumeImageNodes in block-content parse loops
This hopefully makes the logic of these loops a bit more readable.