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.

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

Fixes microsoft/monaco-editor#1818: check matchOnlyAtLineStart when e… #90266

Merged
merged 1 commit into from Feb 10, 2020

Conversation

@bolinfest
Copy link
Contributor

bolinfest commented Feb 7, 2020

This PR fixes microsoft/monaco-editor#1818.

I tested this by copying one of the .html files in playground.generated and changing the JavaScript to include the following:

const DEMO_LANG_ID = "demo";

const languageConfiguration = {
  // The main tokenizer for our languages
  tokenizer: {
    root: [
      [/^\!/, { token: 'delimiter.curly', next: 'jsonInBang', nextEmbedded: 'json' }],
    ],
    jsonInBang: [
      [/^\!/, { token: 'delimiter.curly', next: '@pop', nextEmbedded: '@pop' }],
    ],
  },
};
monaco.languages.register({
  id: DEMO_LANG_ID,
  extensions: ['.example'],
});
monaco.languages.setMonarchTokensProvider(DEMO_LANG_ID, languageConfiguration);

      const value = `\
!
  {
    "foo": "b!ar"
  }
!
`;

var editor = monaco.editor.create(document.getElementById("container"), {
  value,
  language: DEMO_LANG_ID,

  lineNumbers: "off",
  roundedSelection: false,
  scrollBeyondLastLine: false,
  readOnly: false,
  theme: "vs-dark",
});

When I loaded the page, now the entire "b!ar" string literal was highlighted correctly. Previously, the syntax highlighting stopped at the !.

…xiting embedded mode
@bolinfest

This comment has been minimized.

Copy link
Contributor Author

bolinfest commented Feb 7, 2020

https://github.com/microsoft/monaco-editor/blob/master/CONTRIBUTING.md mentions a bunch of test pages, but not a lot about running automated tests. I did try running the following from my ~/src/vscode folder as it recommends:

npm run monaco-editor-test

but that failed with:

npm ERR! missing script: monaco-editor-test

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/mbolin/.npm/_logs/2020-02-07T23_54_12_536Z-debug.log
@rebornix rebornix requested a review from alexdima Feb 10, 2020
@rebornix rebornix assigned alexdima and unassigned rebornix Feb 10, 2020
@alexdima

This comment has been minimized.

Copy link
Member

alexdima commented Feb 10, 2020

Thank you! The change looks good.

I ran the tests at https://github.com/microsoft/monaco-languages using the patch and they all pass.

@alexdima alexdima added this to the February 2020 milestone Feb 10, 2020
@alexdima alexdima merged commit ac11d00 into microsoft:master Feb 10, 2020
5 checks passed
5 checks passed
linux
Details
windows
Details
darwin
Details
VS Code #20200207.146 succeeded
Details
license/cla All CLA requirements met.
@bolinfest

This comment has been minimized.

Copy link
Contributor Author

bolinfest commented Feb 11, 2020

@alexdima Thanks so much for the fast accept and the release!

If I want to add a test, is monaco-languages the right place to do it?

@alexdima

This comment has been minimized.

Copy link
Member

alexdima commented Feb 11, 2020

The best would be to have tests inside the vscode repository... The monaco-languages repository is more concerned with testing that the various tokenizers are correct in tokenizing their respective programming languages...

But currently there are no specific Monarch tests in the vscode repository so you would need to scaffold a bit to get some tests running...

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.