Skip to content

Cut inefficient CssClassSet out of content parsing #505

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Feb 6, 2024
Merged

Cut inefficient CssClassSet out of content parsing #505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
95ca687
content [nfc]: Inline a small helper function
chrisbobbe Jan 30, 2024
96ee7a9
content: Save some work by avoiding `classes.contains` in many cases
chrisbobbe Jan 31, 2024
432b057
content: Save work by using className over `classes` for zero classes…
gnprice Feb 2, 2024
d3da7f5
content [nfc]: Cut dead empty-class case for code-highlighting spans
gnprice Feb 2, 2024
36feae2
content: Use className rather than class set, for CodeBlockSpanNode
gnprice Feb 2, 2024
d94df90
content: Use className regexp for user-mention, instead of class set
gnprice Feb 2, 2024
89ea1cd
content [nfc]: Use non-capturing group in _emojiClassRegexp
gnprice Feb 2, 2024
0acde85
content: Use className regexp for Unicode emoji, instead of class set
gnprice Feb 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion lib/model/code_block.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ enum CodeBlockSpanType {

CodeBlockSpanType codeBlockSpanTypeFromClassName(String className) {
return switch (className) {
'' => CodeBlockSpanType.text,
'hll' => CodeBlockSpanType.highlightedLines,
'w' => CodeBlockSpanType.whitespace,
'esc' => CodeBlockSpanType.escape,
Expand Down
111 changes: 48 additions & 63 deletions lib/model/content.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,22 +577,17 @@ class _ZulipContentParser {

final dom.Element katexElement;
if (!block) {
assert(element.localName == 'span'
&& element.classes.length == 1
&& element.classes.contains('katex'));
assert(element.localName == 'span' && element.className == 'katex');

katexElement = element;
} else {
assert(element.localName == 'span'
&& element.classes.length == 1
&& element.classes.contains('katex-display'));
assert(element.localName == 'span' && element.className == 'katex-display');

if (element.nodes.length != 1) return null;
final child = element.nodes.single;
if (child is! dom.Element) return null;
if (child.localName != 'span') return null;
if (child.classes.length != 1) return null;
if (!child.classes.contains('katex')) return null;
if (child.className != 'katex') return null;
katexElement = child;
}

Expand All @@ -602,8 +597,7 @@ class _ZulipContentParser {
final child = katexElement.nodes.first;
if (child is! dom.Element) return null;
if (child.localName != 'span') return null;
if (child.classes.length != 1) return null;
if (!child.classes.contains('katex-mathml')) return null;
if (child.className != 'katex-mathml') return null;

if (child.nodes.length != 1) return null;
final grandchild = child.nodes.single;
Expand Down Expand Up @@ -638,7 +632,18 @@ class _ZulipContentParser {
return result;
}

static final _emojiClassRegexp = RegExp(r"^emoji(-[0-9a-f]+)*$");
static final _userMentionClassNameRegexp = () {
// This matches a class `user-mention` or `user-group-mention`,
// plus an optional class `silent`, appearing in either order.
const mentionClass = r"user(?:-group)?-mention";
return RegExp("^(?:$mentionClass(?: silent)?|silent $mentionClass)\$");
}();

static final _emojiClassNameRegexp = () {
const specificEmoji = r"emoji(?:-[0-9a-f]+)+";
return RegExp("^(?:emoji $specificEmoji|$specificEmoji emoji)\$");
}();
static final _emojiCodeFromClassNameRegexp = RegExp(r"emoji-([^ ]+)");

InlineContentNode parseInlineContent(dom.Node node) {
assert(_debugParserContext == _ParserContext.inline);
Expand All @@ -654,27 +659,25 @@ class _ZulipContentParser {

final element = node;
final localName = element.localName;
final classes = element.classes;
final className = element.className;
List<InlineContentNode> nodes() => parseInlineContentList(element.nodes);

if (localName == 'br' && classes.isEmpty) {
if (localName == 'br' && className.isEmpty) {
return LineBreakInlineNode(debugHtmlNode: debugHtmlNode);
}
if (localName == 'strong' && classes.isEmpty) {
if (localName == 'strong' && className.isEmpty) {
return StrongNode(nodes: nodes(), debugHtmlNode: debugHtmlNode);
}
if (localName == 'em' && classes.isEmpty) {
if (localName == 'em' && className.isEmpty) {
return EmphasisNode(nodes: nodes(), debugHtmlNode: debugHtmlNode);
}
if (localName == 'code' && classes.isEmpty) {
if (localName == 'code' && className.isEmpty) {
return InlineCodeNode(nodes: nodes(), debugHtmlNode: debugHtmlNode);
}

if (localName == 'a'
&& (classes.isEmpty
|| (classes.length == 1
&& (classes.contains('stream-topic')
|| classes.contains('stream'))))) {
&& (className.isEmpty
|| (className == 'stream-topic' || className == 'stream'))) {
final href = element.attributes['href'];
if (href == null) return unimplemented();
final link = LinkNode(nodes: nodes(), url: href, debugHtmlNode: debugHtmlNode);
Expand All @@ -683,43 +686,31 @@ class _ZulipContentParser {
}

if (localName == 'span'
&& (classes.contains('user-mention')
|| classes.contains('user-group-mention'))
&& (classes.length == 1
|| (classes.length == 2 && classes.contains('silent')))) {
&& _userMentionClassNameRegexp.hasMatch(className)) {
// TODO assert UserMentionNode can't contain LinkNode;
// either a debug-mode check, or perhaps we can make expectations much
// tighter on a UserMentionNode's contents overall.
return UserMentionNode(nodes: nodes(), debugHtmlNode: debugHtmlNode);
}

if (localName == 'span'
&& classes.length == 2
&& classes.contains('emoji')
&& classes.every(_emojiClassRegexp.hasMatch)) {
final emojiCode = classes
.firstWhere((className) => className.startsWith('emoji-'))
.replaceFirst('emoji-', '');
assert(emojiCode.isNotEmpty);

&& _emojiClassNameRegexp.hasMatch(className)) {
final emojiCode = _emojiCodeFromClassNameRegexp.firstMatch(className)!
.group(1)!;
final unicode = tryParseEmojiCodeToUnicode(emojiCode);
if (unicode == null) return unimplemented();
return UnicodeEmojiNode(emojiUnicode: unicode, debugHtmlNode: debugHtmlNode);
}

if (localName == 'img'
&& classes.contains('emoji')
&& classes.length == 1) {
if (localName == 'img' && className == 'emoji') {
final alt = element.attributes['alt'];
if (alt == null) return unimplemented();
final src = element.attributes['src'];
if (src == null) return unimplemented();
return ImageEmojiNode(src: src, alt: alt, debugHtmlNode: debugHtmlNode);
}

if (localName == 'span'
&& classes.length == 1
&& classes.contains('katex')) {
if (localName == 'span' && className == 'katex') {
final texSource = parseMath(element, block: false);
if (texSource == null) return unimplemented();
return MathInlineNode(texSource: texSource, debugHtmlNode: debugHtmlNode);
Expand Down Expand Up @@ -756,13 +747,13 @@ class _ZulipContentParser {
case 'ul': listStyle = ListStyle.unordered; break;
}
assert(listStyle != null);
assert(element.classes.isEmpty);
assert(element.className.isEmpty);

final debugHtmlNode = kDebugMode ? element : null;
final List<List<BlockContentNode>> items = [];
for (final item in element.nodes) {
if (item is dom.Text && item.text == '\n') continue;
if (item is! dom.Element || item.localName != 'li' || item.classes.isNotEmpty) {
if (item is! dom.Element || item.localName != 'li' || item.className.isNotEmpty) {
items.add([UnimplementedBlockContentNode(htmlNode: item)]);
}
items.add(parseImplicitParagraphBlockContentList(item.nodes));
Expand All @@ -775,8 +766,7 @@ class _ZulipContentParser {
assert(_debugParserContext == _ParserContext.block);
final mainElement = () {
assert(divElement.localName == 'div'
&& divElement.classes.length == 1
&& divElement.classes.contains("codehilite"));
&& divElement.className == "codehilite");

if (divElement.nodes.length != 1) return null;
final child = divElement.nodes[0];
Expand Down Expand Up @@ -819,9 +809,8 @@ class _ZulipContentParser {
}
span = CodeBlockSpanNode(text: text, type: CodeBlockSpanType.text);

case dom.Element(localName: 'span', :final text, :final classes)
when classes.length == 1:
final CodeBlockSpanType type = codeBlockSpanTypeFromClassName(classes.first);
case dom.Element(localName: 'span', :final text, :final className):
final CodeBlockSpanType type = codeBlockSpanTypeFromClassName(className);
switch (type) {
case CodeBlockSpanType.unknown:
// TODO(#194): Show these as un-syntax-highlighted code, in production.
Expand All @@ -848,20 +837,19 @@ class _ZulipContentParser {
assert(_debugParserContext == _ParserContext.block);
final imgElement = () {
assert(divElement.localName == 'div'
&& divElement.classes.length == 1
&& divElement.classes.contains('message_inline_image'));
&& divElement.className == 'message_inline_image');

if (divElement.nodes.length != 1) return null;
final child = divElement.nodes[0];
if (child is! dom.Element) return null;
if (child.localName != 'a') return null;
if (child.classes.isNotEmpty) return null;
if (child.className.isNotEmpty) return null;

if (child.nodes.length != 1) return null;
final grandchild = child.nodes[0];
if (grandchild is! dom.Element) return null;
if (grandchild.localName != 'img') return null;
if (grandchild.classes.isNotEmpty) return null;
if (grandchild.className.isNotEmpty) return null;
return grandchild;
}();

Expand All @@ -886,18 +874,16 @@ class _ZulipContentParser {
}
final element = node;
final localName = element.localName;
final classes = element.classes;
List<BlockContentNode> blockNodes() => parseBlockContentList(element.nodes);
final className = element.className;

if (localName == 'br' && classes.isEmpty) {
if (localName == 'br' && className.isEmpty) {
return LineBreakNode(debugHtmlNode: debugHtmlNode);
}

if (localName == 'p' && classes.isEmpty) {
if (localName == 'p' && className.isEmpty) {
// Oddly, the way a math block gets encoded in Zulip HTML is inside a <p>.
if (element.nodes case [dom.Element(localName: 'span') && var child, ...]) {
if (child.classes.length == 1
&& child.classes.contains('katex-display')) {
if (child.className == 'katex-display') {
if (element.nodes case [_]
|| [_, dom.Element(localName: 'br'),
dom.Text(text: "\n")]) {
Expand Down Expand Up @@ -927,29 +913,28 @@ class _ZulipContentParser {
case 'h5': headingLevel = HeadingLevel.h5; break;
case 'h6': headingLevel = HeadingLevel.h6; break;
}
if (headingLevel != null && classes.isEmpty) {
if (headingLevel != null && className.isEmpty) {
final parsed = parseBlockInline(element.nodes);
return HeadingNode(debugHtmlNode: debugHtmlNode,
level: headingLevel,
links: parsed.links,
nodes: parsed.nodes);
}

if ((localName == 'ol' || localName == 'ul') && classes.isEmpty) {
if ((localName == 'ol' || localName == 'ul') && className.isEmpty) {
return parseListNode(element);
}

if (localName == 'blockquote' && classes.isEmpty) {
return QuotationNode(blockNodes(), debugHtmlNode: debugHtmlNode);
if (localName == 'blockquote' && className.isEmpty) {
return QuotationNode(debugHtmlNode: debugHtmlNode,
parseBlockContentList(element.nodes));
}

if (localName == 'div'
&& classes.length == 1 && classes.contains('codehilite')) {
if (localName == 'div' && className == 'codehilite') {
return parseCodeBlock(element);
}

if (localName == 'div'
&& classes.length == 1 && classes.contains('message_inline_image')) {
if (localName == 'div' && className == 'message_inline_image') {
return parseImageNode(element);
}

Expand Down
27 changes: 26 additions & 1 deletion test/model/content_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,39 @@ void main() {
'<p><span class="user-mention silent" data-user-id="2187">Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('Greg Price')]));

// TODO test group mentions and wildcard mentions
testParseInline('silent user @-mention, class order reversed',
// "@_**Greg Price**" (hypothetical server variation)
'<p><span class="silent user-mention" data-user-id="2187">Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('Greg Price')]));

testParseInline('plain group @-mention',
// "@*test-empty*"
'<p><span class="user-group-mention" data-user-group-id="186">@test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('@test-empty')]));

testParseInline('silent group @-mention',
// "@_*test-empty*"
'<p><span class="user-group-mention silent" data-user-group-id="186">test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('test-empty')]));

testParseInline('silent group @-mention, class order reversed',
// "@_*test-empty*" (hypothetical server variation)
'<p><span class="silent user-group-mention" data-user-group-id="186">test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('test-empty')]));

// TODO test wildcard mentions
});

testParseInline('parse Unicode emoji, encoded in span element',
// ":thumbs_up:"
'<p><span aria-label="thumbs up" class="emoji emoji-1f44d" role="img" title="thumbs up">:thumbs_up:</span></p>',
const UnicodeEmojiNode(emojiUnicode: '\u{1f44d}')); // "👍"

testParseInline('parse Unicode emoji, encoded in span element, class order reversed',
// ":thumbs_up:" (hypothetical server variation)
'<p><span aria-label="thumbs up" class="emoji-1f44d emoji" role="img" title="thumbs up">:thumbs_up:</span></p>',
const UnicodeEmojiNode(emojiUnicode: '\u{1f44d}')); // "👍"

testParseInline('parse Unicode emoji, encoded in span element, multiple codepoints',
// ":transgender_flag:"
'<p><span aria-label="transgender flag" class="emoji emoji-1f3f3-fe0f-200d-26a7-fe0f" role="img" title="transgender flag">:transgender_flag:</span></p>',
Expand Down