microsoft / microsoft/vscode-textmate

Lexical pre-sorting decides selector precedence when specificity comparison ties

Open
#293 0 comments 0 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

Theme rules are lexically sorted before insertion into the theme trie.

Matching rules are later sorted by _cmpBySpecificity. When that comparator returns 0, the stable sort preserves the earlier lexical order. Theme.match() then selects the first matching rule.

As a result, lexical spelling can decide precedence even though it is not part of TextMate selector semantics.

Incorrect behavior

The initial ordering compares parent-scope arrays before consulting source index:

r = strArrCmp(a.parentScopes, b.parentScopes);
if (r !== 0) {
    return r;
}

return a.index - b.index;

Later, the specificity comparator can return 0. The first matching rule is then selected:

const effectiveRule = matchingTrieElements.find((rule) =>
    _scopePathMatchesParentScopes(scopePath.parent, rule.parentScopes)
);

The lexical order introduced for trie construction has therefore become an undocumented precedence rule.

Expected behavior

Selector precedence should be determined by semantic specificity against the actual scope stack.

A parent match nearer the terminal scope should rank above an otherwise equivalent match farther away. If two selectors have equal semantic rank, later theme source order should resolve the tie.

Lexical comparison of scope names should not affect the result.

Reproduction

This test can be added to src/tests/themes.test.ts:

test('lexical parent ordering does not decide precedence', () => {
    const theme = Theme.createFromRawTheme({
        settings: [
            {
                scope: 'meta.aaa punctuation.definition.test',
                settings: { foreground: '#FF0000' },
            },
            {
                scope: 'meta.zzz punctuation.definition.test',
                settings: { foreground: '#00FF00' },
            },
        ],
    });

    const result = theme.match(ScopeStack.from(
        'source.test',
        'meta.aaa.test',
        'meta.zzz.test',
        'punctuation.definition.test',
    ));

    assert.strictEqual(
        theme.getColorMap()[result!.foregroundId],
        '#00FF00',
    );
});

Actual result: #FF0000

Expected result: #00FF00

Both parent selectors contain two atoms and eight characters, so _cmpBySpecificity returns a tie.

meta.aaa sorts before meta.zzz, so the first rule wins. However, meta.zzz matches the nearer ancestor and therefore has the higher semantic rank. It is also the later rule in the theme.

Renaming the scopes without changing their structure can reverse the current result.

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

Start with Theme.match(), _cmpBySpecificity, and the trie ordering described in the issue; the reproduction belongs in src/tests/themes.test.ts. Run the added test with the existing theme tests and trace how matching rules are ranked. Done means the nearer parent wins, later source order resolves equal semantic ranks, and lexical scope names no longer affect the result.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.