Skip to content

Draft: Implement escape characters for text format transformers#4449

Closed
themagickoala wants to merge 3 commits intofacebook:mainfrom
themagickoala:escape-chars
Closed

Draft: Implement escape characters for text format transformers#4449
themagickoala wants to merge 3 commits intofacebook:mainfrom
themagickoala:escape-chars

Conversation

@themagickoala
Copy link
Copy Markdown
Contributor

An attempt at allowing TextMatchTransformers to specify escape characters.

On export, will add in escape characters (if no formatting has been applied already).

On import, will strip out escape characters and not apply the transformer formatting.

I'm almost certain I've missed some behaviours here. I've added a very basic test case, but could probably do with adding more!

@vercel
Copy link
Copy Markdown

vercel bot commented May 3, 2023

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 3, 2023 1:52pm
lexical-playground ❌ Failed (Inspect) May 3, 2023 1:52pm

@themagickoala themagickoala changed the title Implement escape characters for text format transformers Draft: Implement escape characters for text format transformers May 3, 2023
@facebook-github-bot 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, 2023
@themagickoala
Copy link
Copy Markdown
Contributor Author

I've added a few more test cases which highlight some failures with double characters, going to take a look into that.

@fantactuka
Copy link
Copy Markdown
Collaborator

fantactuka commented May 5, 2023

I think we should go ahead and split markdown into two separate packages:

  • @lexical/markdown-shortcuts that will only handle as-you-type conversion
  • @lexical/commonmark or @lexical/remark for import/export with external library dependency. Attempting to do import/export with regexp will never be 100% precise, and proper tokenization is the only way (although at cost of code size)

@acywatson
Copy link
Copy Markdown
Contributor

I think we should go ahead and split markdown into two separate packages:

  • @lexical/markdown-shortcuts that will only handle as-you-type conversion
  • @lexical/commonmark or @lexical/remark for import/export with external library dependency. Attempting to do import/export with regexp will never be 100% precise, and proper tokenization is the only way (although at cost of code size)

I wonder if we can keep a single package, just expose different utilities from it. I understand the idea of not having the large 3P dependencies, but structured properly those should be tree-shaken out for anyone not using exports that depend on them.

@kevinansfield
Copy link
Copy Markdown
Contributor

I think we should go ahead and split markdown into two separate packages:

@lexical/markdown-shortcuts that will only handle as-you-type conversion
@lexical/commonmark or @lexical/remark for import/export with external library dependency. Attempting to do import/export with regexp will never be 100% precise, and proper tokenization is the only way (although at cost of code size)

FWIW that's pretty much what we ended up doing in our editor because regex based matching wasn't reliable or complete enough.

On plain text paste events we first run the text through markdown-it with the same setup/plug-ins we use elsewhere then effectively "paste" the resulting HTML so that it's run through all of our node's importDOM methods. Felt like that way we can keep the importDOM methods as a more central place for handling conversion too.

@themagickoala
Copy link
Copy Markdown
Contributor Author

FYI @fantactuka I'm no longer working this PR as I've fully switched over to lexical-remark. Feel free to close it.

@acywatson acywatson closed this Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants