microsoft / monaco-editor

A browser based code editor
https://microsoft.github.io/monaco-editor/
MIT License
40.43k stars 3.6k forks source link

Monarch definition of shell language breaks bash syntax highlighting with some command substitutions #1984

Open pemj opened 4 years ago

pemj commented 4 years ago

monaco-editor version: 0.20.0 (assuming that's the version on the playground) Browser: Chrome OS: Windows Playground code that reproduces the issue:

monaco.editor.create(document.getElementById("container"), {
    value: "function sample-highlight() {\n  echo \"the echo command looks different from this quoted text,\"\n  loc_formatted=$(echo \"WORD\" | awk \'{print tolower($0)}\')\n  echo \"this echo command is the same color as this text\"\n}\n\nfunction still() {\n  echo \"the issue extends out of the scope of that function\"\n}\n",
    language: "shell"
});

I ran into an issue that can be reproduced by displaying the following bash in the Monaco editor, but not VS code:

function sample-highlight() {
  echo "the echo command looks different from this quoted text,"
  loc_formatted=$(echo "WORD" | awk '{print tolower($0)}')
  echo "this echo command SHOULD look different from this text"
}

function still() {
  echo "the issue extends out of the scope of that function"
}

If typed into Visual Studio Code, version info below,

Version: 1.42.1 (system setup)
Commit: c47d83b293181d9be64f27ff093689e8e7aed054
Date: 2020-02-11T14:45:59.656Z
Electron: 6.1.6
Chrome: 76.0.3809.146
Node.js: 12.4.0
V8: 7.6.303.31-electron.0
OS: Windows_NT x64 10.0.18363

that text renders correctly, like so: image

If you create a file called sample.sh in the visual studio code playground (version info below),

Version: 1.46.0-insider
Commit: b26e0bcf39c1b8faaf770265391b5a387d1d9172
Date: 2020-05-26T15:43:08.115Z (product build time)
Browser: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4103.61 Safari/537.36

and add bash in question, it will again render correctly: image

However, if navigate to the monaco homepage at https://microsoft.github.io/monaco-editor/index.html and select shell, then paste in the sample functions, the syntax highlighting breaks image

If you visit the Monaco playground and add this command:

monaco.editor.create(document.getElementById("container"), {
    value: "function sample-highlight() {\n  echo \"the echo command looks different from this quoted text,\"\n  loc_formatted=$(echo \"WORD\" | awk \'{print tolower($0)}\')\n  echo \"this echo command is the same color as this text\"\n}\n\nfunction still() {\n  echo \"the issue extends out of the scope of that function\"\n}\n",
    language: "shell"
});

after clicking "run" you get an instance of incorrect highlighting on the right-hand side: image

I discovered this issue while using Azure DevOps, where it shows up if you edit a file with a .sh ending: image

I've been working on an MS-internal repo full of bash lately, and this bug has been making it a little tricky to review pull requests there.

pemj commented 4 years ago

It actually looks like this is an issue with the Monarch definition of shell script. When I checked the tokenization of line 3 in the Monaco playground, it looks like all of the blue text is being interpreted as a single variable: image

This ignores the opening quote and bracket contained therein, while the closing bracket gets treated as the end of the statement: image

That single quote gets treated as an opening quote instead of a closing quote, and the rest of the file gets interpreted as a string.

The tokenization in VS Code, by contrast, seems to do a pretty decent job of traversing the bash syntax tree.

pemj commented 4 years ago

Found where the issue happens. https://github.com/microsoft/monaco-languages/blob/master/src/shell/shell.ts#L219

The language definition for bash in the monaco-languages repo doesn't appear to support command substitution: just about anything preceded by the characters $( gets treated as a variable identifier instead of an expression, so the quotes, the single quote, the brackets, the open paren, all of that just kinda gets tossed in there, until the pop statement on line 222 catches the closing paren and decides to call it a day.

leantk commented 4 years ago

+1

dubrie commented 4 years ago

+1

Vanuan commented 3 years ago

FWIW, here's a shellscript language configuration in VSCode which doesn't appear to have this issue:

https://github.com/microsoft/vscode/tree/master/extensions/shellscript

It's a shame that commercial product (Azure DevOps) has worse support than the free one (VSCode)

pemj commented 3 years ago

just thought I'd swing by here for old time's sake, sit down for a spell and appreciate the lovely and pastoral beauty of the ol' prairie, appreciate the sound of crickets chirping in the distance. nothin' much ever changes 'round these parts.

anyway I still have to tell people to pull up a separate editor any time they want to review a PR I make to a bash library I help maintain.