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.

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

upgrade markdown package reference #2364

Merged
merged 8 commits into from
Nov 6, 2024
Merged

upgrade markdown package reference #2364

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6050484
upgrade markdown package reference
timu-jesse-ezell Oct 4, 2024
9084683
document _tags in UnderlineSyntax, cleanup _tryCreateInlineLink refer…
timu-jesse-ezell Oct 5, 2024
fddd6dc
fix parser writing node instead of node content
timu-jesse-ezell Oct 5, 2024
95ea5b2
improve markdown tests to align with common behaviors
timu-jesse-ezell Oct 5, 2024
7578b41
add backwards compat logic, add test to ensure backwards compat on ha…
timu-jesse-ezell Oct 7, 2024
a5ae6c2
improve documentation on _tags
timu-jesse-ezell Oct 7, 2024
a1d18d8
improve newline tests
timu-jesse-ezell Oct 7, 2024
5f428bd
change >> to ~/ 2
timu-jesse-ezell Oct 29, 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
20 changes: 13 additions & 7 deletions super_editor_markdown/lib/src/image_syntax.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class SuperEditorImageSyntax extends md.LinkSyntax {
);

@override
md.Node? close(
Iterable<md.Node>? close(
md.InlineParser parser,
covariant md.SimpleDelimiter opener,
md.Delimiter? closer, {
Expand All @@ -38,7 +38,8 @@ class SuperEditorImageSyntax extends md.LinkSyntax {
if (parser.pos + 1 >= parser.source.length) {
// The `]` is at the end of the document, but this may still be a valid
// shortcut reference link.
return _tryCreateReferenceLink(parser, text, getChildren: getChildren);
final link = _tryCreateReferenceLink(parser, text, getChildren: getChildren);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like all uses of _tryCreateReferenceLink() gets converted to a list. Should we just change that method to return Iterable<md.Node>? and then continue to return it directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. That would be better.

return link == null ? null : [ link ];
}

// Peek at the next character; don't advance, so as to avoid later stepping
Expand All @@ -51,7 +52,8 @@ class SuperEditorImageSyntax extends md.LinkSyntax {
var leftParenIndex = parser.pos;
var inlineLink = _parseInlineLink(parser);
if (inlineLink != null) {
return _tryCreateInlineLink(parser, inlineLink, getChildren: getChildren);
final link = _tryCreateInlineLink(parser, inlineLink, getChildren: getChildren);
return link == null ? null : [ link ];
}
// At this point, we've matched `[...](`, but that `(` did not pan out to
// be an inline link. We must now check if `[...]` is simply a shortcut
Expand All @@ -60,7 +62,8 @@ class SuperEditorImageSyntax extends md.LinkSyntax {
// Reset the parser position.
parser.pos = leftParenIndex;
parser.advanceBy(-1);
return _tryCreateReferenceLink(parser, text, getChildren: getChildren);
final link = _tryCreateReferenceLink(parser, text, getChildren: getChildren);
return link == null ? null : [ link ];
}

if (char == AsciiTable.leftBracket) {
Expand All @@ -71,18 +74,21 @@ class SuperEditorImageSyntax extends md.LinkSyntax {
// That opening `[` is not actually part of the link. Maybe a
// *shortcut* reference link (followed by a `[`).
parser.advanceBy(1);
return _tryCreateReferenceLink(parser, text, getChildren: getChildren);
final link = _tryCreateReferenceLink(parser, text, getChildren: getChildren);
return link == null ? null : [ link ];
}
var label = _parseReferenceLinkLabel(parser);
if (label != null) {
return _tryCreateReferenceLink(parser, label, getChildren: getChildren);
final link = _tryCreateReferenceLink(parser, label, getChildren: getChildren);
return link == null ? null : [ link ];
}
return null;
}

// The link text (inside `[...]`) was not followed with a opening `(` nor
// an opening `[`. Perhaps just a simple shortcut reference link (`[...]`).
return _tryCreateReferenceLink(parser, text, getChildren: getChildren);
final link = _tryCreateReferenceLink(parser, text, getChildren: getChildren);
return link == null ? null : [ link ];
}

/// Parses a size using the notation `=widthxheight`.
Expand Down
55 changes: 31 additions & 24 deletions super_editor_markdown/lib/src/markdown_to_document_parsing.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ MutableDocument deserializeMarkdownToDocument(
List<ElementToNodeConverter> customElementToNodeConverters = const [],
bool encodeHtml = false,
}) {
final markdownLines = const LineSplitter().convert(markdown);
final markdownLines = const LineSplitter().convert(markdown).map((l) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happened to require this difference? I'm not sure what the existing code did, but it's not immediately obvious what the difference is in impact between the original code and this new code.

Copy link
Contributor Author

@jezell jezell Oct 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Markdown lib changed from taking List<String> to taking List<Line>.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add an explicit type parameter to your map function so that the difference is unambiguous? Part of the confusion here is that reading the code doesn't tell us what type "l" is.

return md.Line(l);
}).toList();

final markdownDoc = md.Document(
encodeHtml: encodeHtml,
blockSyntaxes: [
...customBlockSyntax,
if (syntax == MarkdownSyntax.superEditor) ...[
Expand Down Expand Up @@ -519,22 +522,26 @@ abstract class ElementToNodeConverter {
DocumentNode? handleElement(md.Element element);
}

/// A Markdown [TagSyntax] that matches underline spans of text, which are represented in
/// A Markdown [DelimiterSyntax] that matches underline spans of text, which are represented in
/// Markdown with surrounding `¬` tags, e.g., "this is ¬underline¬ text".
///
/// This [TagSyntax] produces `Element`s with a `u` tag.
class UnderlineSyntax extends md.TagSyntax {
UnderlineSyntax() : super('¬', requiresDelimiterRun: true, allowIntraWord: true);
/// This [DelimiterSyntax] produces `Element`s with a `u` tag.
class UnderlineSyntax extends md.DelimiterSyntax {

UnderlineSyntax() : super('¬', requiresDelimiterRun: true, allowIntraWord: true, tags: _tags);

@override
md.Node? close(
Iterable<md.Node>? close(
md.InlineParser parser,
md.Delimiter opener,
md.Delimiter closer, {
required List<md.Node> Function() getChildren,
required String tag,
}) {
return md.Element('u', getChildren());
final element = md.Element('u', getChildren());
return [ element ];
}
static final _tags = [ md.DelimiterTag("u", 1) ];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please declare all static members above all instance members (we probably need to add a lint for that at some point).

And can you also add a comment above _tags that describe what it is, and how it's used?

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 is required by md.DelimiterSyntax, in theory the constructor takes an optional tags member, but a bug in
the Markdown lib replaces that with a const [], and then tries to sort it. Will add a comment to that effect.

}

/// Parses a paragraph preceded by an alignment token.
Expand All @@ -548,7 +555,7 @@ class _ParagraphWithAlignmentSyntax extends _EmptyLinePreservingParagraphSyntax

@override
bool canParse(md.BlockParser parser) {
if (!_alignmentNotationPattern.hasMatch(parser.current)) {
if (!_alignmentNotationPattern.hasMatch(parser.current.content)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this a pre-existing error on our part? Should we have been accessing parser.current.content this whole time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Markdown lib changed from String to Line which has a content member, so this just redirects to where the string is now.

return false;
}

Expand All @@ -564,7 +571,7 @@ class _ParagraphWithAlignmentSyntax extends _EmptyLinePreservingParagraphSyntax
/// We found a paragraph alignment token, but the block after the alignment token isn't a paragraph.
/// Therefore, the paragraph alignment token is actually regular content. This parser doesn't need to
/// take any action.
if (_standardNonParagraphBlockSyntaxes.any((syntax) => syntax.pattern.hasMatch(nextLine))) {
if (_standardNonParagraphBlockSyntaxes.any((syntax) => syntax.pattern.hasMatch(nextLine.content))) {
return false;
}

Expand All @@ -575,7 +582,7 @@ class _ParagraphWithAlignmentSyntax extends _EmptyLinePreservingParagraphSyntax

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

// We've parsed the alignment token on the current line. We know a paragraph starts on the
// next line. Move the parser to the next line so that we can parse the paragraph.
Expand Down Expand Up @@ -630,13 +637,13 @@ class _EmptyLinePreservingParagraphSyntax extends md.BlockSyntax {
return false;
}

if (parser.current.isEmpty) {
if (parser.current.content.isEmpty) {
// We consider this input to be a separator between blocks because
// it started with an empty line. We want to parse this input.
return true;
}

if (_isAtParagraphEnd(parser, ignoreEmptyBlocks: _endsWithHardLineBreak(parser.current))) {
if (_isAtParagraphEnd(parser, ignoreEmptyBlocks: _endsWithHardLineBreak(parser.current.content))) {
// Another parser wants to parse this input. Let the other parser run.
return false;
}
Expand All @@ -648,12 +655,12 @@ class _EmptyLinePreservingParagraphSyntax extends md.BlockSyntax {
@override
md.Node? parse(md.BlockParser parser) {
final childLines = <String>[];
final startsWithEmptyLine = parser.current.isEmpty;
final startsWithEmptyLine = parser.current.content.isEmpty;

// A hard line break causes the next line to be treated
// as part of the same paragraph, except if the next line is
// the beginning of another block element.
bool hasHardLineBreak = _endsWithHardLineBreak(parser.current);
bool hasHardLineBreak = _endsWithHardLineBreak(parser.current.content);

if (startsWithEmptyLine) {
// The parser started at an empty line.
Expand All @@ -669,7 +676,7 @@ class _EmptyLinePreservingParagraphSyntax extends md.BlockSyntax {
return null;
}

if (!_blankLinePattern.hasMatch(parser.current)) {
if (!_blankLinePattern.hasMatch(parser.current.content)) {
// We found an empty line, but the following line isn't blank.
// As there is no hard line break, the first line is consumed
// as a separator between blocks.
Expand All @@ -682,7 +689,7 @@ class _EmptyLinePreservingParagraphSyntax extends md.BlockSyntax {
childLines.add('');

// Check for a hard line break, so we consume the next line if we found one.
hasHardLineBreak = _endsWithHardLineBreak(parser.current);
hasHardLineBreak = _endsWithHardLineBreak(parser.current.content);
parser.advance();
}

Expand All @@ -691,9 +698,9 @@ class _EmptyLinePreservingParagraphSyntax extends md.BlockSyntax {
// ends with a hard line break.
while (!_isAtParagraphEnd(parser, ignoreEmptyBlocks: hasHardLineBreak)) {
final currentLine = parser.current;
childLines.add(currentLine);
childLines.add(currentLine.content);

hasHardLineBreak = _endsWithHardLineBreak(currentLine);
hasHardLineBreak = _endsWithHardLineBreak(currentLine.content);

parser.advance();
}
Expand Down Expand Up @@ -777,7 +784,7 @@ class _TaskSyntax extends md.BlockSyntax {

@override
md.Node? parse(md.BlockParser parser) {
final match = pattern.firstMatch(parser.current);
final match = pattern.firstMatch(parser.current.content);
if (match == null) {
return null;
}
Expand All @@ -795,8 +802,8 @@ class _TaskSyntax extends md.BlockSyntax {
// - find a blank line OR
// - find the start of another block element (including another task)
while (!parser.isDone &&
!_blankLinePattern.hasMatch(parser.current) &&
!_standardNonParagraphBlockSyntaxes.any((syntax) => syntax.pattern.hasMatch(parser.current))) {
!_blankLinePattern.hasMatch(parser.current.content) &&
!_standardNonParagraphBlockSyntaxes.any((syntax) => syntax.pattern.hasMatch(parser.current.content))) {
buffer.write('\n');
buffer.write(parser.current);

Expand Down Expand Up @@ -832,7 +839,7 @@ class _HeaderWithAlignmentSyntax extends md.BlockSyntax {

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

Expand All @@ -846,7 +853,7 @@ class _HeaderWithAlignmentSyntax extends md.BlockSyntax {
}

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

Expand All @@ -855,7 +862,7 @@ class _HeaderWithAlignmentSyntax extends md.BlockSyntax {

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

// 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.
Expand Down
2 changes: 1 addition & 1 deletion super_editor_markdown/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:

super_editor: ^0.3.0-dev.3
logging: ^1.0.1
markdown: ^5.0.0
markdown: ^7.2.1

#dependency_overrides:
# Override to local mono-repo path so devs can test this repo
Expand Down
12 changes: 6 additions & 6 deletions super_editor_markdown/test/custom_parsers/callout_block.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,38 +22,38 @@ class CalloutBlockSyntax extends md.BlockSyntax {
// This method was adapted from the standard Blockquote parser, and
// the standard code fence block parser.
@override
List<String> parseChildLines(md.BlockParser parser) {
List<md.Line?> parseChildLines(md.BlockParser parser) {
// Grab all of the lines that form the custom block, stripping off the
// first line, e.g., "@@@ customBlock", and the last line, e.g., "@@@".
var childLines = <String>[];

while (!parser.isDone) {
final openingLine = pattern.firstMatch(parser.current);
final openingLine = pattern.firstMatch(parser.current.content);
if (openingLine != null) {
// This is the first line. Ignore it.
parser.advance();
continue;
}
final closingLine = _endLinePattern.firstMatch(parser.current);
final closingLine = _endLinePattern.firstMatch(parser.current.content);
if (closingLine != null) {
// This is the closing line. Ignore it.
parser.advance();

// If we're followed by a blank line, skip it, so that we don't end
// up with an extra paragraph for that blank line.
if (parser.current.trim().isEmpty) {
if (parser.current.content.isEmpty) {
parser.advance();
}

// We're done.
break;
}

childLines.add(parser.current);
childLines.add(parser.current.content);
parser.advance();
}

return childLines;
return childLines.map((l) => md.Line(l)).toList();
}

// This method was adapted from the standard Blockquote parser, and
Expand Down
Loading