Handle unsupported attributes in Markdown conversion + horizontal lines #418
Conversation
…al rules for dark themes
# Conflicts: # packages/fleather/lib/src/widgets/editor.dart # packages/fleather/lib/src/widgets/theme.dart
…headings localizations
# Conflicts: # packages/fleather/lib/src/widgets/editor_toolbar.dart
|
Thank you @maelchiotti, will take a look as soon as I find some time |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #418 +/- ##
==========================================
- Coverage 87.94% 87.93% -0.02%
==========================================
Files 62 62
Lines 10364 10433 +69
==========================================
+ Hits 9115 9174 +59
- Misses 1249 1259 +10 ☔ View full report in Codecov by Sentry. |
| import '../document/block.dart'; | ||
| import '../document/leaf.dart'; | ||
| import '../document/line.dart'; | ||
| import 'package:parchment/parchment.dart'; |
There was a problem hiding this comment.
When refering to files within the package, we'd rather use relative file imports as per
https://dart.dev/effective-dart/usage#prefer-relative-import-paths
| @@ -486,8 +498,11 @@ class _ParchmentMarkdownEncoder extends Converter<ParchmentDocument, String> { | |||
| return buffer.toString(); | |||
| } | |||
|
|
|||
| void _writeAttribute(StringBuffer buffer, ParchmentAttribute? attribute, | |||
| {bool close = false}) { | |||
| void _writeAttribute( | |||
There was a problem hiding this comment.
We'd rather avoid trailing commas in function declarations when not a Flutter widget
| @@ -497,18 +512,26 @@ class _ParchmentMarkdownEncoder extends Converter<ParchmentDocument, String> { | |||
| } else if (attribute == ParchmentAttribute.strikethrough) { | |||
| _writeStrikeThoughTag(buffer); | |||
| } else if (attribute?.key == ParchmentAttribute.link.key) { | |||
| _writeLinkTag(buffer, attribute as ParchmentAttribute<String>, | |||
| close: close); | |||
| _writeLinkTag( | |||
| } else if (attribute?.key == ParchmentAttribute.heading.key) { | ||
| _writeHeadingTag(buffer, attribute as ParchmentAttribute<int>); | ||
| } else if (attribute?.key == ParchmentAttribute.block.key) { | ||
| _writeBlockTag(buffer, attribute as ParchmentAttribute<String>, | ||
| close: close); | ||
| _writeBlockTag( |
|
|
||
| class ParchmentMarkdownCodec extends Codec<ParchmentDocument, String> { | ||
| const ParchmentMarkdownCodec(); | ||
| const ParchmentMarkdownCodec({this.strictEncoding = true}); | ||
|
|
There was a problem hiding this comment.
Might be worth mentioning we only support <u> at the moment
| @@ -529,8 +560,11 @@ class _ParchmentMarkdownEncoder extends Converter<ParchmentDocument, String> { | |||
| buffer.write('~~'); | |||
| } | |||
|
|
|||
| void _writeLinkTag(StringBuffer buffer, ParchmentAttribute<String> link, | |||
| {bool close = false}) { | |||
| void _writeLinkTag( | |||
| @@ -26,6 +26,12 @@ final class LineNode extends ContainerNode<LeafNode> with StyledNode { | |||
| return false; | |||
| } | |||
|
|
|||
| EmbedNode get embedNode { | |||
There was a problem hiding this comment.
Having a getter that may throw an assertion error seems dangerous here
There was a problem hiding this comment.
Thank you @maelchiotti for this PR and very sorry for the late review
See my comments inline
More generally, you seem to tackle only the decoding part, did you intentionally ignore the encoding?
Also, we had mentioned the replacement of unsupported embeds with [object]
Lastly, can you prove this works with some tests please and remove the localization file from the PR?
Thanks!
There was a problem hiding this comment.
@maelchiotti let me know if you want me to contribute to your PR
|
Thanks, I'll take a look at your review in a week or two, work on the encoding part that is missing, and I'll let you know where I could use some help! |
fixes #405