microsoft / microsoft/vscode-textmate

Scopes on Recursive Regex Cause Problems

Open
#208 9 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
678
Forks
135
Avg merge
1d 14h
Merged PRs (30d)
2

Description

Non-issue example:

Adding scopes to the this capture group (a)+ will only apply the scopes to the last match. While annoying, that behavior is the expected behavior and it's easy enough to work around.

A similar behavior happens with recursion
(a)b\g<1>? this can match ababab but only tags the last a with a scope.

The issue

Let's say (a)\g<1>? is trivial recursion (just repetion)

And (basecase|(\[)\g<1>(\])) is basic recursion (balanced square brackets around the word "basecase")

Anything more complex than those, such as adding alternation, dual-recusion, branching recursion, becomes impossible to reliably highlight.

For illustration, applying a balanced parentheses pattern to [[basecase]] would only add scopes to "basecase". And while there might be a hacky way to color the outside brackets, that hack will fall apart as soon as there's a more complex matcher applied to something like [basecase lambda {basecase[basecase][basecase]}]. Only the last basecase will have scopes. It might only be possible to find "lambda" using a recursive pattern, but by using a recursive pattern we are unable to tag the "lambda", because it only adds scopes to the inner most basecase.

TLDR; this is fundamentally limiting, not just an inconvenience that can be worked around.

Not only is the behavior incredibly unintuitve, but add multiple-recusion (e.g. \g<1>|\g<2>) on top of it and it's debatably undefined behavior.

Rare? Well...

Even hello-world tutorials for defining languages (like a BNF grammar) use recursion like this. When defining a syntax it's easy to define it with this kind of recusion, so it's necessary that this work if those languages are ever going to be correctly parsed by textmate.

Requested solution

There are only a handful of textmate grammars that use complex recusion, and the ones that do almost certainly don't expect this behavior. So changing the scoping behavior shouldn't affect anyone negatively regardless of whether or matches the original textmate implementation.

With that in mind, having the FIRST recursive case be given scopes rather than the last basecase should avoid this problem.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the issue's trivial and balanced-recursion examples in the tokenizer and locate the recursive-regex scope handling; the payload names no files or tests. Add regression coverage for nested and multiple-recursion cases, and verify that scopes are applied consistently to the recursive matches rather than only the innermost case.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
compilers, devtools
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.