Skip to content

Move default language setting to Tokenizer - #3368

Merged
thegreatercurve merged 1 commit into
meta:mainfrom
saint1991:feature/tweak-code-highlight
Dec 8, 2022
Merged

thegreatercurve merged 1 commit into
meta:mainfrom
saint1991:feature/tweak-code-highlight

Conversation

@saint1991

@saint1991 saint1991 commented Nov 16, 2022 •

Copy link
Copy Markdown
Contributor

1. Add a preserve feature to CodeNode

In the original implementation, CodeNode is removed when the backward deletion occurs at the beginning of the Node.
In some use cases, implementing a code editor, such behavior is not needed.
The "Preserve" setting is for making it controllable.

2. Move the default language setting to Tokenizer

The former PR #3243 enabled to use arbitrary Tokenizer other than Prism and 'javascript' is not always appropriate to the default so make it a Tokenizer setting.

This PR contains 2 changes but I'm happy to divide them if required.

@vercel

vercel Bot commented Nov 16, 2022 •

Copy link
Copy Markdown

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

Name Status Preview Updated
lexical ✅ Ready (Inspect) Visit Preview Dec 1, 2022 at 10:49AM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview Dec 1, 2022 at 10:49AM (UTC)

@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 Nov 16, 2022
@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from 8cc94e1 to d28288a Compare November 17, 2022 03:37
@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from d28288a to b986aa8 Compare November 17, 2022 10:57
@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from b986aa8 to 95c4e4f Compare November 18, 2022 14:43
@saint1991 saint1991 changed the title Tweak code highlight Add preserve feature to CodeNode Nov 18, 2022
@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from 95c4e4f to 23ffa80 Compare November 19, 2022 07:37
@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from 23ffa80 to f64ce4d Compare November 19, 2022 08:09
@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from f64ce4d to 292abd1 Compare November 19, 2022 22:51
@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from 292abd1 to 1c4dabb Compare November 19, 2022 23:02

@thegreatercurve thegreatercurve left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@saint1991 The default language changes seems fine, but I think we'd want to leave out the preserve flag. Users can just implement this themselves by extending the CodeNode and overwriting the collapseAtStart method.

@saint1991
saint1991 force-pushed the feature/tweak-code-highlight branch from e17bca2 to 8699445 Compare November 28, 2022 14:51
@saint1991 saint1991 changed the title Add preserve feature to CodeNode Move default language setting to Tokenizer Nov 28, 2022
@saint1991

saint1991 commented Nov 28, 2022 •

Copy link
Copy Markdown
Contributor Author

@thegreatercurve

Thanks for your kind review!
Thanks to #3367, the code something like below works as I expected.

Thank you so much for the great work!
I've removed commit 1. which is no longer needed as you mentioned.

export class CodeEditorNode extends CodeNode {
  static getType(): string {
    return 'code-editor';
  }

  collapseAtStart(): true {
    return true;
  }
}

export const $createCodeEditorNode = (language?: string | null | undefined): CodeEditorNode => {
  return new CodeEditorNode(language);
}

const editor = createEditor({
  nodes: [
    CodeEditorNode,
    CodeNode: {
      replace: CodeNode, 
      with: (node: CodeNode) => $createCodeEditorNode(node.getLanguage())
    },
    CodeHighlightNode,
  ]
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should just be enough:

Suggested change
declare export function $createCodeNode(language?: string | null): CodeNode;
declare export function $createCodeNode(language: ?string): CodeNode;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@thegreatercurve
Thanks. Fixed it.

@fantactuka

Copy link
Copy Markdown
Contributor

I'm curious if it's better to have default language as a param to registerCodeHighlighting and CodeHighlightingPlugin. Then you can have single tokenizer but custom default language based on specific surface needs

@saint1991

Copy link
Copy Markdown
Contributor Author

I think it's also OK, but I put the setting here because the supported languages depend on Tokenizer's implementation. The setting 'javascript' is one of the proper choices for PrismTokenizer because Prism supports it.

In terms of the use of the library, the difference is that users should specify the default language even when they use PrismTokenizer by default.

@thegreatercurve
thegreatercurve merged commit 9fbc564 into meta:main Dec 8, 2022
@trueadm trueadm mentioned this pull request Dec 9, 2022
abelsj60 pushed a commit to abelsj60/lexical that referenced this pull request Jan 5, 2023

This branch was successfully deployed

2 active deployments
Preview – lexical-playground — d4176a5d Deployed Dec 1, 2022 by vercel[bot]
Preview – lexical — d4176a5d Deployed Dec 1, 2022 by vercel[bot]
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.

4 participants