From 6632bdf15b825dc61c7dd4359c90685278786369 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 15:23:44 -0700 Subject: [PATCH 01/14] content [nfc]: Expand cryptic comment on UserMentionNode I wasn't sure what this comment meant when I came across it recently, even though I wrote it myself. Today I was writing tests for this code and saw what the data looks like, and the meaning became clear. Expand the comment so that it's hopefully clear to other readers without having to see the data. --- lib/model/content.dart | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index cdaf5e090e..a02e288c6e 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -198,9 +198,13 @@ class UserMentionNode extends InlineContainerNode { // required this.isSilent, }); - // We don't actually seem to need this information. - // final UserMentionType mentionType; - // final bool isSilent; + // We don't currently seem to need this information in code. Instead, + // the inner text already shows how to communicate it to the user + // (e.g., silent mentions' text lacks a leading "@"), + // and we show that text in the same style for all types of @-mention. + // If we need this information in the future, go ahead and add it here. + // final UserMentionType mentionType; + // final bool isSilent; } abstract class EmojiNode extends InlineContentNode { From 32b11c581c68edfff95bf5b7685486aae57f3206 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 15:49:18 -0700 Subject: [PATCH 02/14] content [nfc]: Mark ContentNode sealed --- lib/model/content.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index a02e288c6e..1c46069e69 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -7,10 +7,8 @@ import 'package:html/parser.dart'; // TODO: Implement toString for all these classes, for testing/debugging; or // perhaps Diagnosticable instead? -// This should have no subclasses except the ones defined in this file. -// TODO mark as sealed: https://github.com/dart-lang/language/blob/a374667bc/accepted/future-releases/sealed-types/feature-specification.md @immutable -abstract class ContentNode { +sealed class ContentNode { const ContentNode({this.debugHtmlNode}); final dom.Node? debugHtmlNode; From dbf64872f9d0d688ef1e67942bc6c36219abab0b Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 17:36:12 -0700 Subject: [PATCH 03/14] content [nfc]: Document ContentNode and main subclasses --- lib/model/content.dart | 48 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/lib/model/content.dart b/lib/model/content.dart index 1c46069e69..9c39fda343 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -7,6 +7,9 @@ import 'package:html/parser.dart'; // TODO: Implement toString for all these classes, for testing/debugging; or // perhaps Diagnosticable instead? +/// A node in a parse tree for Zulip message-style content. +/// +/// See [ZulipContent]. @immutable sealed class ContentNode { const ContentNode({this.debugHtmlNode}); @@ -22,6 +25,7 @@ sealed class ContentNode { } } +/// A node corresponding to HTML that this client doesn't know how to parse. mixin UnimplementedNode on ContentNode { dom.Node get htmlNode; @@ -29,6 +33,12 @@ mixin UnimplementedNode on ContentNode { dom.Node get debugHtmlNode => htmlNode; } +/// A complete parse tree for a Zulip message's content, +/// or other complete piece of Zulip HTML content. +/// +/// This is a parsed representation for an entire value of [Message.content], +/// [Stream.renderedDescription], or other text from a Zulip server that comes +/// in the same Zulip HTML format. class ZulipContent extends ContentNode { const ZulipContent({super.debugHtmlNode, required this.nodes}); @@ -38,10 +48,21 @@ class ZulipContent extends ContentNode { String toString() => '${objectRuntimeType(this, 'ZulipContent')}($nodes)'; } +/// A content node that expects a block layout context from its parent. +/// +/// When rendered as Flutter widgets, these become children of a [Column] +/// created by the parent node's widget. +/// +/// Generally these correspond to HTML elements which in the Zulip web client +/// are laid out as block-level boxes, in a block formatting context: +/// https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flow_layout/Block_and_inline_layout_in_normal_flow +/// +/// Almost all nodes are either a [BlockContentNode] or an [InlineContentNode]. abstract class BlockContentNode extends ContentNode { const BlockContentNode({super.debugHtmlNode}); } +/// A block node corresponding to HTML that this client doesn't know how to parse. class UnimplementedBlockContentNode extends BlockContentNode with UnimplementedNode { const UnimplementedBlockContentNode({required this.htmlNode}); @@ -128,10 +149,22 @@ class ImageNode extends BlockContentNode { final String srcUrl; } +/// A content node that expects an inline layout context from its parent. +/// +/// When rendered into a Flutter widget tree, an inline content node +/// becomes an [InlineSpan], not a widget. It therefore participates +/// in paragraph layout, as a portion of the paragraph's text. +/// +/// Generally these correspond to HTML elements which in the Zulip web client +/// are laid out as inline boxes, in an inline formatting context: +/// https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flow_layout/Block_and_inline_layout_in_normal_flow#elements_participating_in_an_inline_formatting_context +/// +/// Almost all nodes are either an [InlineContentNode] or a [BlockContentNode]. abstract class InlineContentNode extends ContentNode { const InlineContentNode({super.debugHtmlNode}); } +/// An inline node corresponding to HTML that this client doesn't know how to parse. class UnimplementedInlineContentNode extends InlineContentNode with UnimplementedNode { const UnimplementedInlineContentNode({required this.htmlNode}); @@ -140,6 +173,12 @@ class UnimplementedInlineContentNode extends InlineContentNode final dom.Node htmlNode; } +/// A node consisting of pure text, with no markup of its own. +/// +/// This node type is how plain text is represented. This is also the type +/// of the leaf nodes that ultimately provide the actual text in the +/// parse tree for any piece of content that contains text in a link, italics, +/// bold, a list, a blockquote, or many other constructs. class TextNode extends InlineContentNode { const TextNode(this.text, {super.debugHtmlNode}); @@ -163,6 +202,15 @@ class LineBreakInlineNode extends InlineContentNode { const LineBreakInlineNode({super.debugHtmlNode}); } +/// An inline content node which contains other inline content nodes. +/// +/// A node of this type expects an inline layout context from its parent, +/// and provides an inline layout context for its children. +/// +/// Typically this is realized by building a [TextSpan] whose children are +/// the [InlineSpan]s built from this node's children. In that case, +/// the children participate in the same paragraph layout as this node +/// itself does. abstract class InlineContainerNode extends InlineContentNode { const InlineContainerNode({super.debugHtmlNode, required this.nodes}); From e789a48e18298daff7481e7485fffb1756decc96 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 20 Jun 2023 23:15:20 -0700 Subject: [PATCH 04/14] content: Override ==/hashCode on all appropriate node types --- lib/model/content.dart | 59 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 9c39fda343..565d7698fd 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -2,14 +2,24 @@ import 'package:flutter/foundation.dart'; import 'package:html/dom.dart' as dom; import 'package:html/parser.dart'; -// TODO: Implement ==/hashCode for all these classes where O(1), for testing/debugging -// (Skip them for classes containing lists.) // TODO: Implement toString for all these classes, for testing/debugging; or // perhaps Diagnosticable instead? /// A node in a parse tree for Zulip message-style content. /// /// See [ZulipContent]. +/// +/// When implementing subclasses: +/// * Override [==] and [hashCode] when they are cheap, i.e. when there is +/// an O(1) quantity of data under the node. These are for testing +/// and debugging. +/// * Don't override [==] or [hashCode] when the data includes a list. +/// This avoids accidentally doing a lot of work in an operation that +/// looks like it should be cheap. +/// +/// When modifying subclasses: +/// * Always check the following places to see if they need a matching update: +/// * [==] and [hashCode], if overridden. @immutable sealed class ContentNode { const ContentNode({this.debugHtmlNode}); @@ -69,6 +79,8 @@ class UnimplementedBlockContentNode extends BlockContentNode @override final dom.Node htmlNode; + + // No ==/hashCode, because htmlNode is a whole subtree. } // A `br` element. @@ -101,9 +113,6 @@ class ParagraphNode extends BlockContentNode { final List nodes; - // No == or hashCode overrides; don't want to walk through [nodes] in - // an operation that looks cheap. - @override String toString() => '${objectRuntimeType(this, 'ParagraphNode')}(wasImplicit: $wasImplicit, $nodes)'; } @@ -137,6 +146,14 @@ class CodeBlockNode extends BlockContentNode { const CodeBlockNode({super.debugHtmlNode, required this.text}); final String text; + + @override + bool operator ==(Object other) { + return other is CodeBlockNode && other.text == text; + } + + @override + int get hashCode => Object.hash('CodeBlockNode', text); } class ImageNode extends BlockContentNode { @@ -147,6 +164,14 @@ class ImageNode extends BlockContentNode { /// This may be a relative URL string. It also may not work without adding /// authentication credentials to the request. final String srcUrl; + + @override + bool operator ==(Object other) { + return other is ImageNode && other.srcUrl == srcUrl; + } + + @override + int get hashCode => Object.hash('ImageNode', srcUrl); } /// A content node that expects an inline layout context from its parent. @@ -200,6 +225,12 @@ class TextNode extends InlineContentNode { class LineBreakInlineNode extends InlineContentNode { const LineBreakInlineNode({super.debugHtmlNode}); + + @override + bool operator ==(Object other) => other is LineBreakInlineNode; + + @override + int get hashCode => 'LineBreakInlineNode'.hashCode; } /// An inline content node which contains other inline content nodes. @@ -215,6 +246,8 @@ abstract class InlineContainerNode extends InlineContentNode { const InlineContainerNode({super.debugHtmlNode, required this.nodes}); final List nodes; + + // No ==/hashCode, because contains nodes. } class StrongNode extends InlineContainerNode { @@ -261,6 +294,14 @@ class UnicodeEmojiNode extends EmojiNode { const UnicodeEmojiNode({super.debugHtmlNode, required this.text}); final String text; + + @override + bool operator ==(Object other) { + return other is UnicodeEmojiNode && other.text == text; + } + + @override + int get hashCode => Object.hash('UnicodeEmojiNode', text); } class ImageEmojiNode extends EmojiNode { @@ -268,6 +309,14 @@ class ImageEmojiNode extends EmojiNode { final String src; final String alt; + + @override + bool operator ==(Object other) { + return other is ImageEmojiNode && other.src == src && other.alt == alt; + } + + @override + int get hashCode => Object.hash('ImageEmojiNode', src, alt); } //////////////////////////////////////////////////////////////// From bae5cdcd10032e802a7db2248b821b05d1b60cbb Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 21 Jun 2023 15:31:33 -0700 Subject: [PATCH 05/14] content test: Extend equalsNode to cover all node types --- lib/model/content.dart | 1 + test/model/content_checks.dart | 87 ++++++++++++++++++++++++++++------ 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 565d7698fd..8ebdaa0b2b 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -20,6 +20,7 @@ import 'package:html/parser.dart'; /// When modifying subclasses: /// * Always check the following places to see if they need a matching update: /// * [==] and [hashCode], if overridden. +/// * `equalsNode` in test/model/content_checks.dart . @immutable sealed class ContentNode { const ContentNode({this.debugHtmlNode}); diff --git a/test/model/content_checks.dart b/test/model/content_checks.dart index 402c43ae1d..8addf8ab08 100644 --- a/test/model/content_checks.dart +++ b/test/model/content_checks.dart @@ -5,37 +5,96 @@ extension ContentNodeChecks on Subject { void equalsNode(ContentNode expected) { if (expected is ZulipContent) { isA() - .nodes.deepEquals(expected.nodes.map( - (e) => it()..isA().equalsNode(e))); - // A shame we need the dynamic `isA` there. This - // version hits a runtime type error: - // .nodes.deepEquals(expected.nodes.map( - // (e) => it()..equalsNode(e))); - // and with `it()` with no type argument, it doesn't type-check. - // TODO: report that as API feedback on deepEquals + .nodes.equalsNodes(expected.nodes); + } else if (expected is UnimplementedBlockContentNode) { + isA() + .debugHtmlText.equals(expected.debugHtmlText); } else if (expected is ParagraphNode) { isA() ..wasImplicit.equals(expected.wasImplicit) - ..nodes.deepEquals(expected.nodes.map( - (e) => it()..isA().equalsNode(e))); + ..nodes.equalsNodes(expected.nodes); + } else if (expected is ListNode) { + isA() + ..style.equals(expected.style) + ..items.deepEquals(expected.items.map( + (item) => it()..isA>().equalsNodes(item))); + } else if (expected is HeadingNode) { + isA() + ..level.equals(expected.level) + ..nodes.equalsNodes(expected.nodes); + } else if (expected is QuotationNode) { + isA() + .nodes.equalsNodes(expected.nodes); + } else if (expected is UnimplementedInlineContentNode) { + isA() + .debugHtmlText.equals(expected.debugHtmlText); + } else if (expected is StrongNode) { + isA() + .nodes.equalsNodes(expected.nodes); + } else if (expected is EmphasisNode) { + isA() + .nodes.equalsNodes(expected.nodes); + } else if (expected is InlineCodeNode) { + isA() + .nodes.equalsNodes(expected.nodes); + } else if (expected is LinkNode) { + isA() + .nodes.equalsNodes(expected.nodes); + } else if (expected is UserMentionNode) { + isA() + .nodes.equalsNodes(expected.nodes); } else { - // TODO handle remaining ContentNode subclasses that lack structural == + // The remaining node types have structural `==`. Use that. equals(expected); } } + + Subject get debugHtmlText => has((n) => n.debugHtmlText, 'debugHtmlText'); } extension ZulipContentChecks on Subject { Subject> get nodes => has((n) => n.nodes, 'nodes'); } +extension BlockContentNodeListChecks on Subject> { + void equalsNodes(List expected) { + deepEquals(expected.map( + (e) => it()..isA().equalsNode(e))); + // A shame we need the dynamic `isA` there. This + // version hits a runtime type error: + // .nodes.deepEquals(expected.nodes.map( + // (e) => it()..equalsNode(e))); + // and with `it()` with no type argument, it doesn't type-check. + // TODO(checks): report that as API feedback on deepEquals + } +} + extension ParagraphNodeChecks on Subject { Subject get wasImplicit => has((n) => n.wasImplicit, 'wasImplicit'); Subject> get nodes => has((n) => n.nodes, 'nodes'); } -extension TextNodeChecks on Subject { - Subject get text => has((n) => n.text, 'text'); +extension ListNodeChecks on Subject { + Subject get style => has((n) => n.style, 'style'); + Subject>> get items => has((n) => n.items, 'items'); } -// TODO write similar extensions for the rest of the content node classes +extension HeadingNodeChecks on Subject { + Subject get level => has((n) => n.level, 'level'); + Subject> get nodes => has((n) => n.nodes, 'nodes'); +} + +extension QuotationNodeChecks on Subject { + Subject> get nodes => has((n) => n.nodes, 'nodes'); +} + +extension InlineContentNodeListChecks on Subject> { + void equalsNodes(List expected) { + deepEquals(expected.map( + (e) => it()..isA().equalsNode(e))); + } +} + +extension InlineContainerNodeChecks on Subject { + Subject> get nodes => has((n) => n.nodes, 'nodes'); +} From d6ba144e741eec200b76302494616464e8551313 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 13 Jun 2023 21:39:57 -0700 Subject: [PATCH 06/14] content [nfc]: Extract base class BlockInlineContainerNode This abstraction doesn't pull a lot of weight yet, but it will when we start tracking LinkNode descendants, for #71. --- lib/model/content.dart | 34 ++++++++++++++++++++++++++-------- test/model/content_checks.dart | 6 ++++-- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 8ebdaa0b2b..53e85ca3ab 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -84,6 +84,21 @@ class UnimplementedBlockContentNode extends BlockContentNode // No ==/hashCode, because htmlNode is a whole subtree. } +/// A block content node whose children are inline content nodes. +/// +/// A node of this type expects a block layout context from its parent, +/// but provides an inline layout context for its children. +/// +/// See also [InlineContainerNode]. +class BlockInlineContainerNode extends BlockContentNode { + const BlockInlineContainerNode({ + super.debugHtmlNode, + required this.nodes, + }); + + final List nodes; +} + // A `br` element. class LineBreakNode extends BlockContentNode { const LineBreakNode({super.debugHtmlNode}); @@ -105,15 +120,13 @@ class LineBreakNode extends BlockContentNode { // with [wasImplicit]. // // See also [parseImplicitParagraphBlockContentList]. -class ParagraphNode extends BlockContentNode { +class ParagraphNode extends BlockInlineContainerNode { const ParagraphNode( - {super.debugHtmlNode, this.wasImplicit = false, required this.nodes}); + {super.debugHtmlNode, required super.nodes, this.wasImplicit = false}); /// True when there was no corresponding `p` element in the original HTML. final bool wasImplicit; - final List nodes; - @override String toString() => '${objectRuntimeType(this, 'ParagraphNode')}(wasImplicit: $wasImplicit, $nodes)'; } @@ -129,11 +142,14 @@ class ListNode extends BlockContentNode { enum HeadingLevel { h1, h2, h3, h4, h5, h6 } -class HeadingNode extends BlockContentNode { - const HeadingNode(this.level, this.nodes, {super.debugHtmlNode}); +class HeadingNode extends BlockInlineContainerNode { + const HeadingNode({ + super.debugHtmlNode, + required super.nodes, + required this.level, + }); final HeadingLevel level; - final List nodes; } class QuotationNode extends BlockContentNode { @@ -243,6 +259,8 @@ class LineBreakInlineNode extends InlineContentNode { /// the [InlineSpan]s built from this node's children. In that case, /// the children participate in the same paragraph layout as this node /// itself does. +/// +/// See also [BlockInlineContainerNode]. abstract class InlineContainerNode extends InlineContentNode { const InlineContainerNode({super.debugHtmlNode, required this.nodes}); @@ -538,7 +556,7 @@ BlockContentNode parseBlockContent(dom.Node node) { if (headingLevel == HeadingLevel.h6 && classes.isEmpty) { // TODO(#192) handle h1, h2, h3, h4, h5 return HeadingNode( - headingLevel!, inlineNodes(), debugHtmlNode: debugHtmlNode); + level: headingLevel!, nodes: inlineNodes(), debugHtmlNode: debugHtmlNode); } if (localName == 'blockquote' && classes.isEmpty) { diff --git a/test/model/content_checks.dart b/test/model/content_checks.dart index 8addf8ab08..2858972939 100644 --- a/test/model/content_checks.dart +++ b/test/model/content_checks.dart @@ -69,9 +69,12 @@ extension BlockContentNodeListChecks on Subject> { } } +extension BlockInlineContainerNodeChecks on Subject { + Subject> get nodes => has((n) => n.nodes, 'nodes'); +} + extension ParagraphNodeChecks on Subject { Subject get wasImplicit => has((n) => n.wasImplicit, 'wasImplicit'); - Subject> get nodes => has((n) => n.nodes, 'nodes'); } extension ListNodeChecks on Subject { @@ -81,7 +84,6 @@ extension ListNodeChecks on Subject { extension HeadingNodeChecks on Subject { Subject get level => has((n) => n.level, 'level'); - Subject> get nodes => has((n) => n.nodes, 'nodes'); } extension QuotationNodeChecks on Subject { From 7a40d0b8326f91a187ddc92445d2c8c4ce26bb68 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 14:43:19 -0700 Subject: [PATCH 07/14] content [nfc]: Order HeadingNode next to ParagraphNode As their new common superclass BlockInlineContainerNode shows, these two have a lot in common. --- lib/model/content.dart | 26 +++++++++++++------------- lib/widgets/content.dart | 4 ++-- test/model/content_checks.dart | 16 ++++++++-------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 53e85ca3ab..e0e267ac1e 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -131,15 +131,6 @@ class ParagraphNode extends BlockInlineContainerNode { String toString() => '${objectRuntimeType(this, 'ParagraphNode')}(wasImplicit: $wasImplicit, $nodes)'; } -enum ListStyle { ordered, unordered } - -class ListNode extends BlockContentNode { - const ListNode(this.style, this.items, {super.debugHtmlNode}); - - final ListStyle style; - final List> items; -} - enum HeadingLevel { h1, h2, h3, h4, h5, h6 } class HeadingNode extends BlockInlineContainerNode { @@ -152,6 +143,15 @@ class HeadingNode extends BlockInlineContainerNode { final HeadingLevel level; } +enum ListStyle { ordered, unordered } + +class ListNode extends BlockContentNode { + const ListNode(this.style, this.items, {super.debugHtmlNode}); + + final ListStyle style; + final List> items; +} + class QuotationNode extends BlockContentNode { const QuotationNode(this.nodes, {super.debugHtmlNode}); @@ -540,10 +540,6 @@ BlockContentNode parseBlockContent(dom.Node node) { return ParagraphNode(nodes: inlineNodes(), debugHtmlNode: debugHtmlNode); } - if ((localName == 'ol' || localName == 'ul') && classes.isEmpty) { - return parseListNode(element); - } - HeadingLevel? headingLevel; switch (localName) { case 'h1': headingLevel = HeadingLevel.h1; break; @@ -559,6 +555,10 @@ BlockContentNode parseBlockContent(dom.Node node) { level: headingLevel!, nodes: inlineNodes(), debugHtmlNode: debugHtmlNode); } + if ((localName == 'ol' || localName == 'ul') && classes.isEmpty) { + return parseListNode(element); + } + if (localName == 'blockquote' && classes.isEmpty) { return QuotationNode(blockNodes(), debugHtmlNode: debugHtmlNode); } diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 1a56d2ded3..24c9bff7ba 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -79,8 +79,6 @@ class BlockContentNodeWidget extends StatelessWidget { return const Text(''); } else if (node is ParagraphNode) { return Paragraph(node: node); - } else if (node is ListNode) { - return ListNodeWidget(node: node); } else if (node is HeadingNode) { // TODO(#192) h1, h2, h3, h4, h5 -- same as h6 except font size assert(node.level == HeadingLevel.h6); @@ -89,6 +87,8 @@ class BlockContentNodeWidget extends StatelessWidget { child: Text.rich(TextSpan( style: const TextStyle(fontWeight: FontWeight.w600, height: 1.4), children: _buildInlineList(node.nodes)))); + } else if (node is ListNode) { + return ListNodeWidget(node: node); } else if (node is QuotationNode) { return Padding( padding: const EdgeInsets.only(left: 10), diff --git a/test/model/content_checks.dart b/test/model/content_checks.dart index 2858972939..f066b64342 100644 --- a/test/model/content_checks.dart +++ b/test/model/content_checks.dart @@ -13,15 +13,15 @@ extension ContentNodeChecks on Subject { isA() ..wasImplicit.equals(expected.wasImplicit) ..nodes.equalsNodes(expected.nodes); + } else if (expected is HeadingNode) { + isA() + ..level.equals(expected.level) + ..nodes.equalsNodes(expected.nodes); } else if (expected is ListNode) { isA() ..style.equals(expected.style) ..items.deepEquals(expected.items.map( (item) => it()..isA>().equalsNodes(item))); - } else if (expected is HeadingNode) { - isA() - ..level.equals(expected.level) - ..nodes.equalsNodes(expected.nodes); } else if (expected is QuotationNode) { isA() .nodes.equalsNodes(expected.nodes); @@ -77,15 +77,15 @@ extension ParagraphNodeChecks on Subject { Subject get wasImplicit => has((n) => n.wasImplicit, 'wasImplicit'); } +extension HeadingNodeChecks on Subject { + Subject get level => has((n) => n.level, 'level'); +} + extension ListNodeChecks on Subject { Subject get style => has((n) => n.style, 'style'); Subject>> get items => has((n) => n.items, 'items'); } -extension HeadingNodeChecks on Subject { - Subject get level => has((n) => n.level, 'level'); -} - extension QuotationNodeChecks on Subject { Subject> get nodes => has((n) => n.nodes, 'nodes'); } From 83b737e53364b224d8958bf4b606a8228e0c26f3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 15:25:44 -0700 Subject: [PATCH 08/14] content: Use Diagnosticable for debugging output See upstream docs: https://api.flutter.dev/flutter/foundation/Diagnosticable-mixin.html https://api.flutter.dev/flutter/foundation/Diagnosticable/debugFillProperties.html --- lib/model/content.dart | 121 ++++++++++++++++++++++++++++++--- test/model/content_checks.dart | 6 ++ 2 files changed, 119 insertions(+), 8 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index e0e267ac1e..1dd237b3f7 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -2,9 +2,6 @@ import 'package:flutter/foundation.dart'; import 'package:html/dom.dart' as dom; import 'package:html/parser.dart'; -// TODO: Implement toString for all these classes, for testing/debugging; or -// perhaps Diagnosticable instead? - /// A node in a parse tree for Zulip message-style content. /// /// See [ZulipContent]. @@ -16,13 +13,18 @@ import 'package:html/parser.dart'; /// * Don't override [==] or [hashCode] when the data includes a list. /// This avoids accidentally doing a lot of work in an operation that /// looks like it should be cheap. +/// * Don't override [toString]. +/// * Override [debugDescribeChildren] and/or [debugFillProperties] +/// to report all the data attached to the node, for debugging. +/// See docs: https://api.flutter.dev/flutter/foundation/Diagnosticable/debugFillProperties.html /// /// When modifying subclasses: /// * Always check the following places to see if they need a matching update: /// * [==] and [hashCode], if overridden. +/// * [debugFillProperties] and/or [debugDescribeChildren], if present. /// * `equalsNode` in test/model/content_checks.dart . @immutable -sealed class ContentNode { +sealed class ContentNode extends DiagnosticableTree { const ContentNode({this.debugHtmlNode}); final dom.Node? debugHtmlNode; @@ -34,6 +36,22 @@ sealed class ContentNode { if (node is dom.Text) return "(text «${node.text}»)"; return "(node of type ${node.nodeType})"; } + + @override + String toStringShort() => objectRuntimeType(this, 'ContentNode'); + + @override + String toString({DiagnosticLevel minLevel = DiagnosticLevel.info}) { + // TODO(checks): Better integrate package:checks with Diagnosticable, for + // better interaction in output indentation. + // (See also comment at equalsNode, with related improvements.) + String? result; + assert(() { + result = toStringDeep(minLevel: minLevel); + return true; + }()); + return result ?? toStringShort(); + } } /// A node corresponding to HTML that this client doesn't know how to parse. @@ -42,6 +60,12 @@ mixin UnimplementedNode on ContentNode { @override dom.Node get debugHtmlNode => htmlNode; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('html', debugHtmlText)); + } } /// A complete parse tree for a Zulip message's content, @@ -56,7 +80,9 @@ class ZulipContent extends ContentNode { final List nodes; @override - String toString() => '${objectRuntimeType(this, 'ZulipContent')}($nodes)'; + List debugDescribeChildren() { + return nodes.map((node) => node.toDiagnosticsNode()).toList(); + } } /// A content node that expects a block layout context from its parent. @@ -97,6 +123,11 @@ class BlockInlineContainerNode extends BlockContentNode { }); final List nodes; + + @override + List debugDescribeChildren() { + return nodes.map((node) => node.toDiagnosticsNode()).toList(); + } } // A `br` element. @@ -128,7 +159,10 @@ class ParagraphNode extends BlockInlineContainerNode { final bool wasImplicit; @override - String toString() => '${objectRuntimeType(this, 'ParagraphNode')}(wasImplicit: $wasImplicit, $nodes)'; + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(FlagProperty('wasImplicit', value: wasImplicit, ifTrue: 'was implicit')); + } } enum HeadingLevel { h1, h2, h3, h4, h5, h6 } @@ -141,6 +175,12 @@ class HeadingNode extends BlockInlineContainerNode { }); final HeadingLevel level; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(EnumProperty('level', level)); + } } enum ListStyle { ordered, unordered } @@ -150,12 +190,45 @@ class ListNode extends BlockContentNode { final ListStyle style; final List> items; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(FlagProperty('ordered', value: style == ListStyle.ordered, + ifTrue: 'ordered', ifFalse: 'unordered')); + } + + @override + List debugDescribeChildren() { + return items + .map((nodes) => _ListItemDiagnosticableNode(nodes).toDiagnosticsNode()) + .toList(); + } +} + +class _ListItemDiagnosticableNode extends DiagnosticableTree { + _ListItemDiagnosticableNode(this.nodes); + + final List nodes; + + @override + String toStringShort() => 'list item'; + + @override + List debugDescribeChildren() { + return nodes.map((node) => node.toDiagnosticsNode()).toList(); + } } class QuotationNode extends BlockContentNode { const QuotationNode(this.nodes, {super.debugHtmlNode}); final List nodes; + + @override + List debugDescribeChildren() { + return nodes.map((node) => node.toDiagnosticsNode()).toList(); + } } class CodeBlockNode extends BlockContentNode { @@ -171,6 +244,12 @@ class CodeBlockNode extends BlockContentNode { @override int get hashCode => Object.hash('CodeBlockNode', text); + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('text', text)); + } } class ImageNode extends BlockContentNode { @@ -189,6 +268,12 @@ class ImageNode extends BlockContentNode { @override int get hashCode => Object.hash('ImageNode', srcUrl); + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('srcUrl', srcUrl)); + } } /// A content node that expects an inline layout context from its parent. @@ -235,9 +320,11 @@ class TextNode extends InlineContentNode { @override int get hashCode => Object.hash('TextNode', text); - // TODO encode unambiguously regardless of text contents @override - String toString() => '${objectRuntimeType(this, 'TextNode')}(text: $text)'; + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('text', text, showName: false)); + } } class LineBreakInlineNode extends InlineContentNode { @@ -266,6 +353,11 @@ abstract class InlineContainerNode extends InlineContentNode { final List nodes; + @override + List debugDescribeChildren() { + return nodes.map((node) => node.toDiagnosticsNode()).toList(); + } + // No ==/hashCode, because contains nodes. } @@ -321,6 +413,12 @@ class UnicodeEmojiNode extends EmojiNode { @override int get hashCode => Object.hash('UnicodeEmojiNode', text); + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('text', text)); + } } class ImageEmojiNode extends EmojiNode { @@ -336,6 +434,13 @@ class ImageEmojiNode extends EmojiNode { @override int get hashCode => Object.hash('ImageEmojiNode', src, alt); + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('alt', alt)); + properties.add(StringProperty('src', src)); + } } //////////////////////////////////////////////////////////////// diff --git a/test/model/content_checks.dart b/test/model/content_checks.dart index f066b64342..d0412d515e 100644 --- a/test/model/content_checks.dart +++ b/test/model/content_checks.dart @@ -3,6 +3,12 @@ import 'package:zulip/model/content.dart'; extension ContentNodeChecks on Subject { void equalsNode(ContentNode expected) { + // TODO: Make equalsNode output clearer on failure, applying Diagnosticable. + // In particular (a) show the top-level expected node in one piece + // (as well as the actual); (a') ideally, suppress on the "expected" side + // the various predicates below, which should be redundant with just + // the expected node; (b) show expected for the specific `equals` leaf. + // See also comment on [ContentNode.toString]. if (expected is ZulipContent) { isA() .nodes.equalsNodes(expected.nodes); From 6e95db210735f8f77032285e98bd8ed8ccc8ba5a Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 12:30:23 -0700 Subject: [PATCH 09/14] content test [nfc]: Extract helper testParse --- test/model/content_test.dart | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 75f33e8f4a..ff58c62c9b 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -4,21 +4,24 @@ import 'package:zulip/model/content.dart'; import 'content_checks.dart'; -void main() { - test('parse a plain-text paragraph', () { - check(parseContent('

hello world

')) - .equalsNode(const ZulipContent(nodes: [ - ParagraphNode(nodes: [TextNode('hello world')]), - ])); +void testParse(String name, String html, List nodes) { + test(name, () { + check(parseContent(html)) + .equalsNode(ZulipContent(nodes: nodes)); }); +} - test('parse two plain-text paragraphs', () { - check(parseContent('

hello

world

')) - .equalsNode(const ZulipContent(nodes: [ - ParagraphNode(nodes: [TextNode('hello')]), - ParagraphNode(nodes: [TextNode('world')]), - ])); - }); +void main() { + testParse('parse a plain-text paragraph', + '

hello world

', const [ + ParagraphNode(nodes: [TextNode('hello world')]), + ]); + + testParse('parse two plain-text paragraphs', + '

hello

world

', const [ + ParagraphNode(nodes: [TextNode('hello')]), + ParagraphNode(nodes: [TextNode('world')]), + ]); // TODO write more tests for this code } From 93fb63637d1b9af220789005fae8d502f8d15cf0 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 12:47:22 -0700 Subject: [PATCH 10/14] content test: Use real HTML examples for parsing --- test/model/content_test.dart | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/test/model/content_test.dart b/test/model/content_test.dart index ff58c62c9b..25767d0a46 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -12,13 +12,39 @@ void testParse(String name, String html, List nodes) { } void main() { + // When writing test cases in this file: + // + // * Try to use actual HTML emitted by a Zulip server. + // Record the corresponding Markdown source in a comment. + // + // * Here's a handy `curl` command for getting the server's HTML. + // First, as one-time setup, create a file with a test account's + // Zulip credentials in "netrc" format, meaning one line that looks like: + // machine HOSTNAME login EMAIL password API_KEY + // + // * Then send some test messages, and fetch with a command like this. + // (Change "sender" operand to your user ID, and "topic" etc. as desired.) + /* $ curl -sS --netrc-file ../.netrc -G https://chat.zulip.org/api/v1/messages \ + --data-urlencode 'narrow=[{"operator":"sender", "operand":2187}, + {"operator":"stream", "operand":"test here"}, + {"operator":"topic", "operand":"content"}]' \ + --data-urlencode anchor=newest --data-urlencode num_before=10 --data-urlencode num_after=0 \ + --data-urlencode apply_markdown=true \ + | jq '.messages[] | .content' + */ + // + // * To get the corresponding Markdown source, use the same command + // with `apply_markdown` changed to `false`. + testParse('parse a plain-text paragraph', + // "hello world" '

hello world

', const [ ParagraphNode(nodes: [TextNode('hello world')]), ]); testParse('parse two plain-text paragraphs', - '

hello

world

', const [ + // "hello\n\nworld" + '

hello

\n

world

', const [ ParagraphNode(nodes: [TextNode('hello')]), ParagraphNode(nodes: [TextNode('world')]), ]); From cc969072200bf78cc78763856b007304057a1a20 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 13:16:08 -0700 Subject: [PATCH 11/14] content test: Test parsing inline content --- test/model/content_test.dart | 104 ++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 3 deletions(-) diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 25767d0a46..768d1ae091 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -36,11 +36,109 @@ void main() { // * To get the corresponding Markdown source, use the same command // with `apply_markdown` changed to `false`. + // + // Inline content. + // + + void testParseInline(String name, String html, InlineContentNode node) { + testParse(name, html, [ParagraphNode(nodes: [node])]); + } + testParse('parse a plain-text paragraph', // "hello world" - '

hello world

', const [ - ParagraphNode(nodes: [TextNode('hello world')]), - ]); + '

hello world

', const [ParagraphNode(nodes: [ + TextNode('hello world'), + ])]); + + testParse('parse
inside a paragraph', + // "a\nb" + '

a
\nb

', const [ParagraphNode(nodes: [ + TextNode('a'), + LineBreakInlineNode(), + TextNode('\nb'), + ])]); + + testParseInline('parse strong/bold', + // "**bold**" + '

bold

', + const StrongNode(nodes: [TextNode('bold')])); + + testParseInline('parse emphasis/italic', + // "*italic*" + '

italic

', + const EmphasisNode(nodes: [TextNode('italic')])); + + testParseInline('parse inline code', + // "`inline code`" + '

inline code

', + const InlineCodeNode(nodes: [TextNode('inline code')])); + + testParseInline('parse nested strong, em, code', + // "***`word`***" + '

word

', + const StrongNode(nodes: [EmphasisNode(nodes: [InlineCodeNode(nodes: [ + TextNode('word')])])])); + + testParseInline('parse link', + // "[text](https://example/)" + '

text

', + const LinkNode(nodes: [TextNode('text')])); + + testParseInline('parse #-mention of stream', + // "#**general**" + '

' + '#general

', + const LinkNode(nodes: [TextNode('#general')])); + + testParseInline('parse #-mention of topic', + // "#**mobile-team>zulip-flutter**" + '

' + '#mobile-team > zulip-flutter

', + const LinkNode(nodes: [TextNode('#mobile-team > zulip-flutter')])); + + testParseInline('parse nested link, strong, em, code', + // "[***`word`***](https://example/)" + '

word' + '

', + const LinkNode(nodes: [StrongNode(nodes: [ + EmphasisNode(nodes: [InlineCodeNode(nodes: [ + TextNode('word')])])])])); + + group('parse @-mentions', () { + testParseInline('plain user @-mention', + // "@**Greg Price**" + '

@Greg Price

', + const UserMentionNode(nodes: [TextNode('@Greg Price')])); + + testParseInline('silent user @-mention', + // "@_**Greg Price**" + '

Greg Price

', + const UserMentionNode(nodes: [TextNode('Greg Price')])); + + // TODO test group mentions and wildcard mentions + }); + + testParseInline('parse Unicode emoji', + // ":thumbs_up:" + '

:thumbs_up:

', + const UnicodeEmojiNode(text: ':thumbs_up:')); + + testParseInline('parse custom emoji', + // ":flutter:" + '

:flutter:

', + const ImageEmojiNode( + src: '/user_avatars/2/emoji/images/204.png', alt: ':flutter:')); + + testParseInline('parse Zulip extra emoji', + // ":zulip:" + '

:zulip:

', + const ImageEmojiNode( + src: '/static/generated/emoji/images/emoji/unicode/zulip.png', alt: ':zulip:')); + + // + // Block content. + // testParse('parse two plain-text paragraphs', // "hello\n\nworld" From 4f9b54dd2b54ab4ba4562c0866a45031eeb2a414 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 15:41:15 -0700 Subject: [PATCH 12/14] content test: Test parsing headings --- test/model/content_test.dart | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 768d1ae091..b46e1ff858 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -1,4 +1,5 @@ import 'package:checks/checks.dart'; +import 'package:html/parser.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/model/content.dart'; @@ -11,6 +12,16 @@ void testParse(String name, String html, List nodes) { }); } +UnimplementedBlockContentNode blockUnimplemented(String html) { + var fragment = HtmlParser(html, parseMeta: false).parseFragment(); + return UnimplementedBlockContentNode(htmlNode: fragment.nodes.single); +} + +UnimplementedInlineContentNode inlineUnimplemented(String html) { + var fragment = HtmlParser(html, parseMeta: false).parseFragment(); + return UnimplementedInlineContentNode(htmlNode: fragment.nodes.single); +} + void main() { // When writing test cases in this file: // @@ -147,5 +158,41 @@ void main() { ParagraphNode(nodes: [TextNode('world')]), ]); + group('parse headings', () { + testParse('plain h6', + // "###### six" + '
six
', const [ + HeadingNode(level: HeadingLevel.h6, nodes: [TextNode('six')])]); + + testParse('containing inline markup', + // "###### one [***`two`***](https://example/)" + '
one two' + '
', const [ + HeadingNode(level: HeadingLevel.h6, nodes: [ + TextNode('one '), + LinkNode(nodes: [StrongNode(nodes: [ + EmphasisNode(nodes: [InlineCodeNode(nodes: [ + TextNode('two')])])])]), + ])]); + + testParse('amidst paragraphs', + // "intro\n###### section\ntext" + "

intro

\n
section
\n

text

", const [ + ParagraphNode(nodes: [TextNode('intro')]), + HeadingNode(level: HeadingLevel.h6, nodes: [TextNode('section')]), + ParagraphNode(nodes: [TextNode('text')]), + ]); + + testParse('h1, h2, h3, h4, h5 unimplemented', + // "# one\n## two\n### three\n#### four\n##### five" + '

one

\n

two

\n

three

\n

four

\n
five
', [ + blockUnimplemented('

one

'), + blockUnimplemented('

two

'), + blockUnimplemented('

three

'), + blockUnimplemented('

four

'), + blockUnimplemented('
five
'), + ]); + }); + // TODO write more tests for this code } From fe1428df3357a9326ca7c1aea81bdcbce900d467 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 12:59:49 -0700 Subject: [PATCH 13/14] content test: Test parsing lists --- test/model/content_test.dart | 43 ++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/model/content_test.dart b/test/model/content_test.dart index b46e1ff858..1be455e67c 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -194,5 +194,48 @@ void main() { ]); }); + group('parse lists', () { + testParse('
    ', + // "1. first\n2. then" + '
      \n
    1. first
    2. \n
    3. then
    4. \n
    ', const [ + ListNode(ListStyle.ordered, [ + [ParagraphNode(wasImplicit: true, nodes: [TextNode('first')])], + [ParagraphNode(wasImplicit: true, nodes: [TextNode('then')])], + ]), + ]); + + testParse('
      ', + // "* something\n* another" + '
        \n
      • something
      • \n
      • another
      • \n
      ', const [ + ListNode(ListStyle.unordered, [ + [ParagraphNode(wasImplicit: true, nodes: [TextNode('something')])], + [ParagraphNode(wasImplicit: true, nodes: [TextNode('another')])], + ]), + ]); + + testParse('implicit paragraph with internal
      ', + // "* a\n b" + '
        \n
      • a
        \n b
      • \n
      ', const [ + ListNode(ListStyle.unordered, [ + [ParagraphNode(wasImplicit: true, nodes: [ + TextNode('a'), + LineBreakInlineNode(), + TextNode('\n b'), // TODO: this renders misaligned + ])], + ]) + ]); + + testParse('explicit paragraphs', + // "* a\n\n b" + '
        \n
      • \n

        a

        \n

        b

        \n
      • \n
      ', const [ + ListNode(ListStyle.unordered, [ + [ + ParagraphNode(nodes: [TextNode('a')]), + ParagraphNode(nodes: [TextNode('b')]), + ], + ]), + ]); + }); + // TODO write more tests for this code } From 99d565910cb22d96986052bbcf7d75cc149bcc72 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 26 Jun 2023 14:03:59 -0700 Subject: [PATCH 14/14] content test: Test parsing remaining block content --- test/model/content_test.dart | 54 +++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 1be455e67c..208c5d6ffc 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -151,6 +151,13 @@ void main() { // Block content. // + testParse('parse
      in block context', + '

      a


      ', const [ // TODO not sure how to reproduce this example + LineBreakNode(), + ParagraphNode(nodes: [TextNode('a')]), + LineBreakNode(), + ]); + testParse('parse two plain-text paragraphs', // "hello\n\nworld" '

      hello

      \n

      world

      ', const [ @@ -237,5 +244,50 @@ void main() { ]); }); - // TODO write more tests for this code + testParse('parse quotations', + // "```quote\nwords\n```" + '
      \n

      words

      \n
      ', const [ + QuotationNode([ParagraphNode(nodes: [TextNode('words')])]), + ]); + + testParse('parse code blocks, no language', + // "```\nverb\natim\n```" + '
      verb\natim\n
      ', const [ + CodeBlockNode(text: 'verb\natim'), + ]); + + testParse('parse code blocks, with highlighted language', + // "```dart\nclass A {}\n```" + '
      '
      +        'class '
      +        'A {}'
      +        '\n
      ', const [ + CodeBlockNode(text: 'class A {}'), + ]); + + testParse('parse image', + // "https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3" + '', const [ + ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3'), + ]); + + testParse('parse nested lists, quotes, headings, code blocks', + // "1. > ###### two\n > * three\n\n four" + '
        \n
      1. \n
        \n
        two
        \n
          \n
        • three
        • \n' + '
        \n
        \n
        '
        +        'four\n
        \n\n
      2. \n
      ', const [ + ListNode(ListStyle.ordered, [[ + QuotationNode([ + HeadingNode(level: HeadingLevel.h6, nodes: [TextNode('two')]), + ListNode(ListStyle.unordered, [[ + ParagraphNode(wasImplicit: true, nodes: [TextNode('three')]), + ]]), + ]), + CodeBlockNode(text: 'four'), + ParagraphNode(wasImplicit: true, nodes: [TextNode('\n\n')]), // TODO avoid this; it renders wrong + ]]), + ]); }