[lexical-markdown][lexical-playground] Feature: Option to include blanklines in markdown render #6020

potatowagon · 2024-05-03T10:25:32Z

Description

Describe the changes in this pull request

option to make the number of blank lines consistent between markdown and the rendered markdown

Closes: #

To enable:

$convertFromMarkdownString(
  ...,
  true // shouldPreserveNewLines -> set to true,
)

$convertToMarkdownString(
   ...,
   true // shouldPreserveNewLines -> set to true,
);

These flags are optional. Defaults to old behavior of removing newlines on import and delimiting with newlines on export.

Test plan

Before

Screen.Recording.2024-05-03.at.10.16.38.PM.mov

blank lines are ignored and removed

After

Screen.Recording.2024-05-03.at.10.18.02.PM.mov

number of blank lines consistent between markdown and rendered markdown

Added settings in playground to toggle newline preservation

Screen.Recording.2024-05-17.at.1.44.36.AM.mov

Unit and e2e tests pass locally

vercel · 2024-05-03T10:25:38Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 17, 2024 0:56am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 17, 2024 0:56am

github-actions · 2024-05-03T10:27:16Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
packages/lexical/dist/Lexical.js	23.93 KB (0%)	479 ms (0%)	175 ms (+1113.49% 🔺)	654 ms
packages/lexical-rich-text/dist/LexicalRichText.js	34.59 KB (0%)	692 ms (0%)	207 ms (+521% 🔺)	899 ms
packages/lexical-plain-text/dist/LexicalPlainText.js	34.51 KB (0%)	691 ms (0%)	195 ms (+490.96% 🔺)	885 ms

fantactuka · 2024-05-03T17:56:17Z

LGTM, I believe there were some PRs in past with similar idea. More broader discussion (covered it a bit offline with Gerard & folks): it seems like best route for MD plugin is to eliminate out of the box import and export parsing and instead start relying on external library (e.g., remark), while plugin will only responsible to connect Lexical and 3rd party lib. That would allow delegating this kind of configuration to those libs.

Shortcuts is a bit more lightweight functionality (and handles simple formats only) so it could still remain a part of the plugin (that way we're not bloating editor with full MD parsing library for simple shortcuts).

zurfyx · 2024-05-07T21:32:52Z

packages/lexical-markdown/src/MarkdownExport.ts


 export function createMarkdownExport(
 transformers: Array<Transformer>,
+ isNewlineDelimited: boolean = true,


I believe the two are the same feature, do we need a different flag name for these?

I would probably call it preserveNewLines

that makes sense to standardise the flag name for both import and export, will change!

addressed in 89a7b30

zurfyx · 2024-05-07T21:34:55Z

packages/lexical-markdown/src/MarkdownImport.ts

- firstChild == null ||
- (node.getChildrenSize() === 1 &&
- $isTextNode(firstChild) &&
- MARKDOWN_EMPTY_LINE_REG_EXP.test(firstChild.getTextContent()))


Is this intentional? I believe this regex is particularly important on top of the default Node isEmpty

i moved this to packages/lexical-markdown/src/utils.ts for reuse

potatowagon · 2024-05-08T21:08:17Z

packages/lexical-markdown/src/utils.ts

+ $isTextNode(firstChild) &&
+ MARKDOWN_EMPTY_LINE_REG_EXP.test(firstChild.getTextContent()))
+ );
+}


@zurfyx here it is

GermanJablo · 2024-05-13T04:35:10Z

it seems like best route for MD plugin is to eliminate out of the box import and export parsing and instead start relying on external library (e.g., remark) @fantactuka

I'm not very familiar with Remark, but I have a hard time seeing how a generic library could correctly parse Lexical's editorState.

Separate topic, I do like this PR. In my app I use markdown for the tests, and to make the conversion idempotent I do something quite dirty: I add a specific string to the empty paragraphs, and after the conversion is done I delete them.

…-> para matching

potatowagon · 2024-05-16T18:10:49Z

note: there is a change to blockquote behavior.

with shouldPreserveNewLines = false (old behavior)

hello

text

becomes

>hello

text

because of newline delimeter

with shouldPreserveNewLines = true (new optional behavior)

hello

text

^imagine no newlines seperating them

becomes

>hello
>text

tho this is consistent with other markdown surfaces such as github & dillinger where texts right below blockquotes get included in the blockquote

zurfyx · 2024-05-16T22:53:17Z

packages/lexical-markdown/src/MarkdownExport.ts

@@ -116,6 +128,7 @@ function exportChildren(
 exportTextFormat(child, child.getTextContent(), textTransformersIndex),
 );
 } else if ($isElementNode(child)) {
+ // empty para returns ""


If you think this comment adds some value, let's write "paragraph" instead

addressed in 6a42853

zurfyx · 2024-05-16T22:57:11Z

packages/lexical-markdown/src/MarkdownExport.ts


 export function createMarkdownExport(
 transformers: Array<Transformer>,
+ shouldPreserveNewLines: boolean = false,


Do you think the preserveNewLines comment we discussed applies here?

i hope i understood u correctly, its been applied in 89a7b30 ,

isNewlineDelimited replaced with shouldPreserveNewLines,

preserveNewLines is named as shouldPreserveNewLines to be consistent with boolean naming conventions

Sahejkm

LGTM

potatowagon requested review from zurfyx, fantactuka, acywatson, Fetz and ivailop7 as code owners May 3, 2024 10:25

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 3, 2024

vercel bot deployed to Preview – lexical May 3, 2024 10:26 View deployment

vercel bot deployed to Preview – lexical-playground May 3, 2024 10:27 View deployment

potatowagon marked this pull request as draft May 3, 2024 10:28

vercel bot deployed to Preview – lexical May 3, 2024 10:34 View deployment

vercel bot deployed to Preview – lexical-playground May 3, 2024 10:35 View deployment

vercel bot deployed to Preview – lexical May 3, 2024 10:41 View deployment

vercel bot deployed to Preview – lexical-playground May 3, 2024 10:43 View deployment

vercel bot deployed to Preview – lexical May 3, 2024 10:52 View deployment

vercel bot deployed to Preview – lexical-playground May 3, 2024 10:54 View deployment

vercel bot deployed to Preview – lexical May 3, 2024 13:51 View deployment

vercel bot deployed to Preview – lexical-playground May 3, 2024 13:52 View deployment

vercel bot deployed to Preview – lexical May 3, 2024 14:09 View deployment

vercel bot deployed to Preview – lexical-playground May 3, 2024 14:10 View deployment

potatowagon changed the title ~~Optional-markdown-blanklines~~ [lexical-markdown][lexical-playground] Feature: Option to include blanklines in markdown render May 3, 2024

potatowagon mentioned this pull request May 4, 2024

Feature: Option to include blank lines in markdown #6023

Closed

zurfyx reviewed May 7, 2024

View reviewed changes

potatowagon commented May 8, 2024

View reviewed changes

zurfyx mentioned this pull request May 8, 2024

[RFC] Plugins, normalizers, and nodes #6066

Open

potatowagon added 2 commits May 16, 2024 23:52

init

767ad05

make sure import and export html have consistent number of newlines <…

884a1ae

…-> para matching

potatowagon added 6 commits May 16, 2024 23:56

edit comment

3d5a481

change condition ternery for better performance

921ec14

Split Screen

b815fd9

optoin to not insert newlines as delimeter for markdown export

b39eb63

test

ff8c5c4

test

cf42a14

potatowagon force-pushed the optional-markdown-blanklines branch from 7096aae to cf42a14 Compare May 16, 2024 16:19

vercel bot deployed to Preview – lexical-playground May 16, 2024 16:20 View deployment

vercel bot deployed to Preview – lexical May 16, 2024 16:20 View deployment

standardise shouldPreserveNewLines flag

89a7b30

vercel bot deployed to Preview – lexical May 16, 2024 16:34 View deployment

vercel bot deployed to Preview – lexical-playground May 16, 2024 16:34 View deployment

sort imports+ comment

36fa818

vercel bot deployed to Preview – lexical May 16, 2024 16:59 View deployment

vercel bot deployed to Preview – lexical-playground May 16, 2024 17:00 View deployment

add settings for preserve new lines in markdown

d35bc85

vercel bot deployed to Preview – lexical May 16, 2024 17:17 View deployment

vercel bot deployed to Preview – lexical-playground May 16, 2024 17:18 View deployment

potatowagon marked this pull request as ready for review May 16, 2024 17:54

zurfyx previously approved these changes May 16, 2024

View reviewed changes

github-actions bot added the extended-tests Run extended e2e tests on a {R label May 16, 2024

adjust comment

6a42853

potatowagon dismissed zurfyx’s stale review via 6a42853 May 17, 2024 00:55

vercel bot deployed to Preview – lexical May 17, 2024 00:55 View deployment

vercel bot deployed to Preview – lexical-playground May 17, 2024 00:56 View deployment

Sahejkm approved these changes May 17, 2024

View reviewed changes

potatowagon added this pull request to the merge queue May 17, 2024

Merged via the queue into main with commit f02e163 May 17, 2024
52 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lexical-markdown][lexical-playground] Feature: Option to include blanklines in markdown render #6020

[lexical-markdown][lexical-playground] Feature: Option to include blanklines in markdown render #6020

potatowagon commented May 3, 2024 •

edited

vercel bot commented May 3, 2024 •

edited

github-actions bot commented May 3, 2024 •

edited

fantactuka commented May 3, 2024

zurfyx May 7, 2024

potatowagon May 8, 2024

potatowagon May 16, 2024

zurfyx May 7, 2024

potatowagon May 8, 2024

potatowagon May 8, 2024

GermanJablo commented May 13, 2024

potatowagon commented May 16, 2024 •

edited

zurfyx May 16, 2024

potatowagon May 17, 2024

zurfyx May 16, 2024

potatowagon May 17, 2024 •

edited

Sahejkm left a comment

[lexical-markdown][lexical-playground] Feature: Option to include blanklines in markdown render #6020

[lexical-markdown][lexical-playground] Feature: Option to include blanklines in markdown render #6020

Conversation

potatowagon commented May 3, 2024 • edited

Description

Test plan

Before

After

Added settings in playground to toggle newline preservation

vercel bot commented May 3, 2024 • edited

github-actions bot commented May 3, 2024 • edited

size-limit report 📦

fantactuka commented May 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GermanJablo commented May 13, 2024

potatowagon commented May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potatowagon May 17, 2024 • edited

Choose a reason for hiding this comment

Sahejkm left a comment

Choose a reason for hiding this comment

potatowagon commented May 3, 2024 •

edited

vercel bot commented May 3, 2024 •

edited

github-actions bot commented May 3, 2024 •

edited

potatowagon commented May 16, 2024 •

edited

potatowagon May 17, 2024 •

edited