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

[SuperEditorMarkdown] Add header alignments #1350

Merged
merged 10 commits into from
Sep 29, 2023
76 changes: 76 additions & 0 deletions super_editor_markdown/lib/src/document_to_markdown_serializer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ String serializeDocumentToMarkdown(
const HorizontalRuleNodeSerializer(),
const ListItemNodeSerializer(),
const TaskNodeSerializer(),
HeaderNodeSerializer(syntax),
ParagraphNodeSerializer(syntax),
];

Expand Down Expand Up @@ -362,3 +363,78 @@ class AttributedTextMarkdownSerializer extends AttributionVisitor {
return "";
}
}

/// [DocumentNodeMarkdownSerializer], which serializes Markdown headers to
/// [ParagraphNode]s with an appropriate header block type, and (optionally) a
/// block alignment.
///
/// Headers are represented by `ParagraphNode`s and therefore this serializer must
/// run before a [ParagraphNodeSerializer], so that this serializer can process
/// header-specific details, such as header alignment.
class HeaderNodeSerializer extends NodeTypedDocumentNodeMarkdownSerializer<ParagraphNode> {
const HeaderNodeSerializer(this.markdownSyntax);

final MarkdownSyntax markdownSyntax;

@override
String? serialize(Document document, DocumentNode node) {
if (node is! ParagraphNode) {
return null;
}

// Only serialize this node when this is a header node.
final Attribution? blockType = node.getMetadataValue('blockType');
final isHeaderNode = blockType == header1Attribution ||
blockType == header2Attribution ||
blockType == header3Attribution ||
blockType == header4Attribution ||
blockType == header5Attribution ||
blockType == header6Attribution;

if (!isHeaderNode) {
return null;
}

return doSerialization(document, node);
}

@override
String doSerialization(Document document, ParagraphNode node) {
final buffer = StringBuffer();

final Attribution? blockType = node.getMetadataValue('blockType');
final String? textAlign = node.getMetadataValue('textAlign');

// Left alignment is the default, so there is no need to add the alignment token.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is in a strange location. It's unclear how it relates to the code. After reading this comment, I go down a few lines, and we're adding the alignment token, so it's confusing to see a comment that says "there is no need to add the alignment token". Please ensure that comments appear in places that clarify code behavior, rather than confuse it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the next line, we have textAlign != 'left' condition, which means we don't need to add the left alignment token to the markdown when we serialize a header node (the same as ParagraphNodeSerializer). Do you have any better comment for it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary issue is the location of the comment. The code directly below that comment doesn't implement "no need to add the alignment token". This comment actually refers to a non-existent else statement.

The code that actually exists is code that adds an alignment token. Not code that avoids adding an alignment token.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Left alignment is the default, so there is no need to add the alignment token.
if (markdownSyntax == MarkdownSyntax.superEditor && textAlign != null && textAlign != 'left') {
   final alignmentToken = _convertAlignmentToMarkdown(textAlign);
   if (alignmentToken != null) {
      buffer.writeln(alignmentToken);
  }
}

I think it would be clear if the comment is "Left alignment is the default, so there is no need to add the left alignment token.".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're still phrasing the comment in terms of what the code ISN'T doing, instead of what the code IS doing. Please write code comments in terms of what the code IS doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment has been updated as you requested.

if (markdownSyntax == MarkdownSyntax.superEditor && textAlign != null && textAlign != 'left') {
final alignmentToken = _convertAlignmentToMarkdown(textAlign);
if (alignmentToken != null) {
buffer.writeln(alignmentToken);
}
}

if (blockType == header1Attribution) {
buffer.write('# ${node.text.toMarkdown()}');
} else if (blockType == header2Attribution) {
buffer.write('## ${node.text.toMarkdown()}');
} else if (blockType == header3Attribution) {
buffer.write('### ${node.text.toMarkdown()}');
} else if (blockType == header4Attribution) {
buffer.write('#### ${node.text.toMarkdown()}');
} else if (blockType == header5Attribution) {
buffer.write('##### ${node.text.toMarkdown()}');
} else if (blockType == header6Attribution) {
buffer.write('###### ${node.text.toMarkdown()}');
}
lamnhan066 marked this conversation as resolved.
Show resolved Hide resolved

// We're not at the end of the document yet. Add a blank line after the
// paragraph so that we can tell the difference between separate
// paragraphs vs. newlines within a single paragraph.
final nodeIndex = document.getNodeIndexById(node.id);
if (nodeIndex != document.nodes.length - 1) {
buffer.writeln();
}

return buffer.toString();
}
}
86 changes: 85 additions & 1 deletion super_editor_markdown/lib/src/markdown_to_document_parsing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ MutableDocument deserializeMarkdownToDocument(
final markdownDoc = md.Document(
blockSyntaxes: [
...customBlockSyntax,
if (syntax == MarkdownSyntax.superEditor) //
if (syntax == MarkdownSyntax.superEditor) ...[
_HeaderWithAlignmentSyntax(),
_ParagraphWithAlignmentSyntax(),
],
_EmptyLinePreservingParagraphSyntax(),
_TaskSyntax(),
],
Expand Down Expand Up @@ -197,12 +199,14 @@ class _MarkdownToDocument implements md.NodeVisitor {
break;
}

final textAlign = element.attributes['textAlign'];
_content.add(
ParagraphNode(
id: Editor.createNodeId(),
text: _parseInlineText(element),
metadata: {
'blockType': headerAttribution,
'textAlign': textAlign,
},
),
);
Expand Down Expand Up @@ -694,6 +698,86 @@ class _TaskSyntax extends md.BlockSyntax {
}
}

/// Parses a header preceded by an alignment token.
matthew-carroll marked this conversation as resolved.
Show resolved Hide resolved
///
/// Headers are represented by `_ParagraphWithAlignmentSyntax`s and therefore
/// this parser must run before a [_ParagraphWithAlignmentSyntax], so that this parser
/// can process header-specific details, such as header alignment.
class _HeaderWithAlignmentSyntax extends md.BlockSyntax {
/// This pattern matches the text alignment notation.
///
/// Possible values are `:---`, `:---:`, `---:` and `-::-`.
static final _alignmentNotationPattern = RegExp(r'^:-{3}|:-{3}:|-{3}:|-::-$');

/// Use internal HeaderSyntax.
final _headerSyntax = const md.HeaderSyntax();

@override
RegExp get pattern => RegExp('');

@override
bool canEndBlock(md.BlockParser parser) => false;

@override
bool canParse(md.BlockParser parser) {
if (!_alignmentNotationPattern.hasMatch(parser.current)) {
return false;
}

final nextLine = parser.peek(1);

// We found a match for a paragraph alignment token. However, the alignment token is the last
// line of content in the document. Therefore, it's not really a paragraph alignment token, and we
// should treat it as regular content.
if (nextLine == null) {
return false;
}

// Only parse if the next line is header.
if (!_headerSyntax.pattern.hasMatch(nextLine)) {
return false;
}

return true;
}

@override
md.Node? parse(md.BlockParser parser) {
final match = _alignmentNotationPattern.firstMatch(parser.current);

// We've parsed the alignment token on the current line. We know a header starts on the
// next line. Move the parser to the next line so that we can parse the header.
parser.advance();

final headerNode = _headerSyntax.parse(parser);

if (headerNode is md.Element) {
headerNode.attributes.addAll({'textAlign': _convertMarkdownAlignmentTokenToSuperEditorAlignment(match!.input)});
}

return headerNode;
}

/// Converts a markdown alignment token to the textAlign metadata used to configure
/// the [ParagraphNode] alignment.
String _convertMarkdownAlignmentTokenToSuperEditorAlignment(String alignmentToken) {
switch (alignmentToken) {
case ':---':
return 'left';
case ':---:':
return 'center';
case '---:':
return 'right';
case '-::-':
return 'justify';
// As we already check that the input matches the notation,
// we shouldn't reach this point.
default:
return 'left';
}
}
}

/// Matches empty lines or lines containing only whitespace.
final _blankLinePattern = RegExp(r'^(?:[ \t]*)$');

Expand Down
148 changes: 145 additions & 3 deletions super_editor_markdown/test/super_editor_markdown_test.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import 'package:super_editor/super_editor.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:super_editor/super_editor.dart';
import 'package:super_editor_markdown/super_editor_markdown.dart';

void main() {
Expand Down Expand Up @@ -32,6 +32,67 @@ void main() {
expect(serializeDocumentToMarkdown(doc), '###### My Header');
});

test('header with left alignment', () {
final doc = MutableDocument(nodes: [
ParagraphNode(
id: '1',
text: AttributedText('Header1'),
metadata: {
'textAlign': 'left',
'blockType': header1Attribution,
},
),
]);
// Even when using superEditor markdown syntax, which has support
// for text alignment, we don't add an alignment token when
// the paragraph is left-aligned.
// Paragraphs are left-aligned by default, so it isn't necessary
// to serialize the alignment token.
expect(serializeDocumentToMarkdown(doc), '# Header1');
});

test('header with center alignment', () {
final doc = MutableDocument(nodes: [
ParagraphNode(
id: '1',
text: AttributedText('Header1'),
metadata: {
'textAlign': 'center',
'blockType': header1Attribution,
},
),
]);
expect(serializeDocumentToMarkdown(doc), ':---:\n# Header1');
});

test('header with right alignment', () {
final doc = MutableDocument(nodes: [
ParagraphNode(
id: '1',
text: AttributedText('Header1'),
metadata: {
'textAlign': 'right',
'blockType': header1Attribution,
},
),
]);
expect(serializeDocumentToMarkdown(doc), '---:\n# Header1');
});

test('header with justify alignment', () {
final doc = MutableDocument(nodes: [
ParagraphNode(
id: '1',
text: AttributedText('Header1'),
metadata: {
'textAlign': 'justify',
'blockType': header1Attribution,
},
),
]);
expect(serializeDocumentToMarkdown(doc), '-::-\n# Header1');
});

test('header with styles', () {
final doc = MutableDocument(nodes: [
ParagraphNode(
Expand Down Expand Up @@ -558,6 +619,26 @@ with multiple lines
text: AttributedText('Example Doc'),
metadata: {'blockType': header1Attribution},
),
ParagraphNode(
id: Editor.createNodeId(),
text: AttributedText('Example Doc With Left Alignment'),
metadata: {'blockType': header1Attribution, 'textAlign': 'left'},
),
ParagraphNode(
id: Editor.createNodeId(),
text: AttributedText('Example Doc With Center Alignment'),
metadata: {'blockType': header1Attribution, 'textAlign': 'center'},
),
ParagraphNode(
id: Editor.createNodeId(),
text: AttributedText('Example Doc With Right Alignment'),
metadata: {'blockType': header1Attribution, 'textAlign': 'right'},
),
ParagraphNode(
id: Editor.createNodeId(),
text: AttributedText('Example Doc With Justify Alignment'),
metadata: {'blockType': header1Attribution, 'textAlign': 'justify'},
),
HorizontalRuleNode(id: Editor.createNodeId()),
ParagraphNode(
id: Editor.createNodeId(),
Expand Down Expand Up @@ -671,6 +752,38 @@ with multiple lines
expect((header6Doc.nodes.first as ParagraphNode).getMetadataValue('blockType'), header6Attribution);
});

test('header with left alignment', () {
final headerLeftAlignment1 = deserializeMarkdownToDocument(':---\n# Header 1');
final header = headerLeftAlignment1.nodes.first as ParagraphNode;
expect(header.getMetadataValue('blockType'), header1Attribution);
expect(header.getMetadataValue('textAlign'), 'left');
expect(header.text.text, 'Header 1');
});

test('header with center alignment', () {
final headerLeftAlignment1 = deserializeMarkdownToDocument(':---:\n# Header 1');
final header = headerLeftAlignment1.nodes.first as ParagraphNode;
expect(header.getMetadataValue('blockType'), header1Attribution);
expect(header.getMetadataValue('textAlign'), 'center');
expect(header.text.text, 'Header 1');
});

test('header with right alignment', () {
final headerLeftAlignment1 = deserializeMarkdownToDocument('---:\n# Header 1');
final header = headerLeftAlignment1.nodes.first as ParagraphNode;
expect(header.getMetadataValue('blockType'), header1Attribution);
expect(header.getMetadataValue('textAlign'), 'right');
expect(header.text.text, 'Header 1');
});

test('header with justify alignment', () {
final headerLeftAlignment1 = deserializeMarkdownToDocument('-::-\n# Header 1');
final header = headerLeftAlignment1.nodes.first as ParagraphNode;
expect(header.getMetadataValue('blockType'), header1Attribution);
expect(header.getMetadataValue('textAlign'), 'justify');
expect(header.text.text, 'Header 1');
});

test('blockquote', () {
final blockquoteDoc = deserializeMarkdownToDocument('> This is a blockquote');

Expand Down Expand Up @@ -885,7 +998,7 @@ with multiple lines
test('example doc 1', () {
final document = deserializeMarkdownToDocument(exampleMarkdownDoc1);

expect(document.nodes.length, 21);
expect(document.nodes.length, 26);

expect(document.nodes[0], isA<ParagraphNode>());
expect((document.nodes[0] as ParagraphNode).getMetadataValue('blockType'), header1Attribution);
Expand Down Expand Up @@ -916,7 +1029,25 @@ with multiple lines

expect(document.nodes[19], isA<TaskNode>());

expect(document.nodes[20], isA<ParagraphNode>());
expect(document.nodes[20], isA<HorizontalRuleNode>());

expect(document.nodes[21], isA<ParagraphNode>());
expect((document.nodes[21] as ParagraphNode).getMetadataValue('blockType'), header1Attribution);
expect((document.nodes[21] as ParagraphNode).getMetadataValue('textAlign'), 'left');

expect(document.nodes[22], isA<ParagraphNode>());
expect((document.nodes[22] as ParagraphNode).getMetadataValue('blockType'), header1Attribution);
expect((document.nodes[22] as ParagraphNode).getMetadataValue('textAlign'), 'center');

expect(document.nodes[23], isA<ParagraphNode>());
expect((document.nodes[23] as ParagraphNode).getMetadataValue('blockType'), header1Attribution);
expect((document.nodes[23] as ParagraphNode).getMetadataValue('textAlign'), 'right');

expect(document.nodes[24], isA<ParagraphNode>());
expect((document.nodes[24] as ParagraphNode).getMetadataValue('blockType'), header1Attribution);
expect((document.nodes[24] as ParagraphNode).getMetadataValue('textAlign'), 'justify');

expect(document.nodes[25], isA<ParagraphNode>());
});

test('paragraph with strikethrough', () {
Expand Down Expand Up @@ -1152,5 +1283,16 @@ Another paragraph

- [x] Completed task

---

:---
# Example 1 With Left Alignment
:---:
# Example 1 With Center Alignment
---:
# Example 1 With Right Alignment
-::-
# Example 1 With Justify Alignment

The end!
''';