Skip to content

Fix the patterns for code blocks and the handling of newlines when matching patterns #9921

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.

Sign up for GitHub

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

Open
DanTup wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix the patterns for code blocks and the handling of newlines when matching patterns #9921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a2cc27
Fix the patterns for code blocks and the handling of newlines when ma…
DanTup Jul 28, 2026
b2dd71d
Update NEXT_RELEASE_NOTES.md
DanTup Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/devtools_app/assets/dart_syntax.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "Dart",
"version": "1.5.0",
"version": "1.6.0",
"fileTypes": [
"dart"
],
Expand Down Expand Up @@ -77,12 +77,12 @@
"repository": {
"dartdoc-codeblock-triple": {
"begin": "^\\s*///\\s*(?!\\s*```)",
"end": "\n",
"end": "$",
"contentName": "variable.other.source.dart"
},
"dartdoc-codeblock-block": {
"begin": "^\\s*\\*\\s*(?!(\\s*```|\/))",
"end": "\n",
"end": "$",
"contentName": "variable.other.source.dart"
},
"dartdoc": {
Expand Down
54 changes: 43 additions & 11 deletions packages/devtools_app/lib/src/screens/debugger/span_parser.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,13 @@ class ScopeSpan {
);

while (!splitScanner.isDone) {
if (splitScanner.matches(cond)) {
if (splitScanner.matchesOnCurrentLine(cond)) {
// Update the end position for this span as it's been fully processed.
current._endLocation = splitScanner.location;
splitSpans.add(current);

// Move the scanner position past the matched condition.
splitScanner.scan(cond);
splitScanner.scanOnCurrentLine(cond);

// Create a new span based on the current position.
current = ScopeSpan(
Expand Down Expand Up @@ -273,7 +273,7 @@ class _SimpleMatcher extends GrammarMatcher {
@override
bool scan(Grammar grammar, LineScanner scanner, ScopeStack scopeStack) {
final location = scanner.location;
if (scanner.scan(match)) {
if (scanner.scanOnCurrentLine(match)) {
scopeStack.push(name, location);
_applyCapture(grammar, scanner, scopeStack, captures, location);
scopeStack.pop(name, scanner.location);
Expand Down Expand Up @@ -307,7 +307,7 @@ class _MultilineMatcher extends GrammarMatcher {
: RegExp(json['while'] as String, multiLine: true),
patterns = (json['patterns'] as List<Object?>?)
?.cast<Map<String, Object?>>()
.map((e) => GrammarMatcher.fromJson(e))
.map(GrammarMatcher.fromJson)
.toList(),
super._();

Expand Down Expand Up @@ -361,7 +361,7 @@ class _MultilineMatcher extends GrammarMatcher {

void _scanBegin(Grammar grammar, LineScanner scanner, ScopeStack scopeStack) {
final location = scanner.location;
if (!scanner.scan(begin)) {
if (!scanner.scanOnCurrentLine(begin)) {
// This shouldn't happen since we've already checked that `begin` matches
// the beginning of the string.
throw StateError('Expected ${begin.pattern} to match.');
Expand Down Expand Up @@ -403,7 +403,9 @@ class _MultilineMatcher extends GrammarMatcher {
LineScanner scanner,
ScopeStack scopeStack,
) {
while (!scanner.isDone && end != null && !scanner.matches(end!)) {
while (!scanner.isDone &&
end != null &&
!scanner.matchesOnCurrentLine(end!)) {
bool foundMatch = false;
for (final pattern in patterns ?? <GrammarMatcher>[]) {
if (pattern.scan(grammar, scanner, scopeStack)) {
Expand All @@ -420,7 +422,7 @@ class _MultilineMatcher extends GrammarMatcher {

void _scanEnd(Grammar grammar, LineScanner scanner, ScopeStack scopeStack) {
final location = scanner.location;
if (end != null && !scanner.scan(end!)) {
if (end != null && !scanner.scanOnCurrentLine(end!)) {
return;
}
_processCaptureHelper(grammar, scanner, scopeStack, endCaptures, location);
Expand All @@ -446,7 +448,7 @@ class _MultilineMatcher extends GrammarMatcher {

@override
bool scan(Grammar grammar, LineScanner scanner, ScopeStack scopeStack) {
if (!scanner.matches(begin)) {
if (!scanner.matchesOnCurrentLine(begin)) {
return false;
}

Expand All @@ -461,7 +463,9 @@ class _MultilineMatcher extends GrammarMatcher {
// Find the range of the string that is matched by the while condition.
final start = scanner.position;
_skipLine(scanner);
while (!scanner.isDone && whileCond != null && scanner.scan(whileCond!)) {
while (!scanner.isDone &&
whileCond != null &&
scanner.scanOnCurrentLine(whileCond!)) {
_skipLine(scanner);
}
final end = scanner.position;
Expand All @@ -482,7 +486,7 @@ class _MultilineMatcher extends GrammarMatcher {
// Process each line until the `while` condition fails.
while (!contentScanner.isDone &&
whileCond != null &&
contentScanner.scan(whileCond!)) {
contentScanner.scanOnCurrentLine(whileCond!)) {
_scanToEndOfLine(grammar, contentScanner, scopeStack);
}

Expand Down Expand Up @@ -520,7 +524,7 @@ class _PatternMatcher extends GrammarMatcher {
_PatternMatcher(super.json)
: patterns = (json['patterns'] as List<Object?>?)
?.cast<Map<String, Object?>>()
.map((e) => GrammarMatcher.fromJson(e))
.map(GrammarMatcher.fromJson)
.toList(),
super._();

Expand Down Expand Up @@ -758,4 +762,32 @@ class ScopeStackLocation {
extension LineScannerExtension on LineScanner {
ScopeStackLocation get location =>
ScopeStackLocation(position: position, line: line, column: column);

/// Returns whether [pattern] matches at the scanner's current position
/// without consuming a line break.
///
/// Use this for all TextMate grammar regular expressions. The TextMate
/// [Language Rules documentation](https://macromates.com/manual/en/language_grammars#language_rules)
/// specifies that regular expressions are matched against one document line
/// at a time, while [LineScanner] searches the remaining source text. Without
/// this check, a pattern containing whitespace such as `\s*` can consume text
/// from the next line and produce scopes that TextMate would not.
bool matchesOnCurrentLine(RegExp pattern) {
if (!matches(pattern)) return false;

final match = lastMatch![0]!;
return !match.contains('\n') && !match.contains('\r');
}

/// Scans [pattern] only when it matches without consuming a line break.
///
/// Use this instead of [LineScanner.scan] when consuming a TextMate grammar
/// regular expression. See [matchesOnCurrentLine] for the rule requiring
/// line-local matching. Use the scanner directly only for parser operations
/// that intentionally move across lines, such as skipping a physical line.
bool scanOnCurrentLine(RegExp pattern) {
if (!matchesOnCurrentLine(pattern)) return false;

return scan(pattern);
}
Comment on lines +788 to +792

@gemini-code-assist gemini-code-assist Bot Jul 28, 2026

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.

medium

[CONCERN] In scanOnCurrentLine, calling scan(pattern) after matchesOnCurrentLine(pattern) causes the regular expression to be matched twice. Since matchesOnCurrentLine already performs the match and updates lastMatch, we can directly advance the scanner's position to lastMatch!.end to avoid this redundant overhead.

This is important for maintaining high performance on the UI thread during syntax highlighting.

  bool scanOnCurrentLine(RegExp pattern) {
    if (!matchesOnCurrentLine(pattern)) return false;

    position = lastMatch!.end;
    return true;
  }
References
  1. Prioritize logic, performance on the UI thread, and architectural consistency. (link)

@DanTup DanTup Jul 28, 2026 •
edited
Loading

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.

This doesn't work, it causes a lot of test failures. My guess is that it's because _column is also tracking the position via some whitespace-handling in scan(). Unless there's evidence of bad performance, I think we should try to avoid duplicating that logic here.

}
3 changes: 3 additions & 0 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ TODO: Remove this section if there are not any updates.
[#9885](https://github.com/flutter/devtools/pull/9885)
* Update to latest version of the Dart syntax highlighting grammar
[#9920](https://github.com/flutter/devtools/pull/9920).
* Fix a bug in the TextMate grammar parser that could result in code after
comments being classified as comments.
[#9921](https://github.com/flutter/devtools/pull/9921).

## Network profiler updates

Expand Down
3 changes: 2 additions & 1 deletion packages/devtools_app/test/test_infra/goldens/syntax_highlighting/open_code_block.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
>///
#^^^ comment.block.documentation.dart
>/// This should not cause parsing to fail.
#^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ comment.block.documentation.dart variable.other.source.dart
#^^^^ comment.block.documentation.dart
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ comment.block.documentation.dart variable.other.source.dart
>
>void main() {
#^^^^ storage.type.primitive.dart
Expand Down
Loading