Repository navigation
syntax: javascript: Highlight regex literals - #4277
Open
ShryukGrandhi wants to merge 1 commit into
Open
ShryukGrandhi wants to merge 1 commit into
ShryukGrandhi wants to merge 1 commit into
Conversation
Quotes inside a regex literal such as /['"]/ were treated as the start of a string, breaking the highlighting of the rest of the file. Add a region for regex literals, recognized by a slash that follows an operator, opening bracket, comma, semicolon or return. Fixes micro-editor#1110 Signed-off-by: ShryukGrandhi <85529990+ShryukGrandhi@users.noreply.github.com>
Andriamanitra
left a comment
Andriamanitra
left a comment
Collaborator
There was a problem hiding this comment.
I've thought about this problem before and I don't think there is an acceptable solution that doesn't involve changing or extending the syntax highlighting system to support either a context stack or regex look-behind.
| # regex literal: a slash that follows an operator, opening bracket, | ||
| # comma, semicolon or `return` and is not the start of a comment | ||
| - constant.regex: | ||
| start: "(^|[(,=:\\[!&|?{};]|\\breturn)\\s*/([^/*\\\\]|\\\\.)" |
Collaborator
There was a problem hiding this comment.
This also colors the preceding operator/keyword as constant.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The JavaScript syntax file had no notion of regex literals, so a quote inside one (e.g.
.match(/['"]/)) opened a string region that ran on through the rest of the file.Changes
constant.regexregion toruntime/syntax/javascript.yaml. Since Go regexps have no lookbehind, a regex literal is detected as a/that follows an operator-like character (( , = : [ ! & | ? { ;),return, or the start of the line, and whose next character isn't/or*(so//and/*comments are unaffected). Escaped slashes don't end the region, and it always ends at end of line so a false positive can't spill into the following lines.a / b / cdoesn't match and is left alone./(e.g. the(in.match(/x/)) gets the regex color too, because it is part of the region's start match.Testing
runtime/syntax_javascript_test.go, which runs the embedded JavaScript syntax through the highlighter. It checks the example from the issue (the next line isn't highlighted as a string), division, an escaped\/in a regex, and//comments after(,,and code. It fails before the change and passes after it.go test ./...,go vet ./runtime/andgofmt -l runtime/are clean.Fixes #1110