Skip to content

Parent selector specificity uses character count instead of scope depth #291

Description

@5cover

theme.ts ranks the terminal scope by scopeDepth, but ranks each parent scope by the number of characters in its name.

This makes spelling length part of selector precedence. Theme authors generally reason about scope hierarchy through dot-separated atoms, not identifier length.

Incorrect behavior

Consider these parent selectors:

Selector Characters Scope atoms
meta.tag.xml 12 3
string.quoted 13 2

The current comparator ranks string.quoted higher because it has one more character:

const parentScopeLengthDiff = b.parentScopes[bParentIndex].length - a.parentScopes[aParentIndex].length

if (parentScopeLengthDiff !== 0) {
  return parentScopeLengthDiff
}

This is inconsistent with the terminal comparison, which uses scope depth.

Expected behavior

Parent scopes should be compared by their number of dot-separated scope atoms at every level of the selector.

In the example above, meta.tag.xml should outrank string.quoted because it has three atoms rather than two. Character count should not affect specificity.

Reproduction

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

test('parent selector specificity uses scope depth', () => {
  const theme = Theme.createFromRawTheme({
    settings: [
      {
        scope: 'meta.tag.xml punctuation.definition.string.begin.xml',
        settings: { foreground: '#FF0000' },
      },
      {
        scope: 'string.quoted punctuation.definition.string.begin.xml',
        settings: { foreground: '#00FF00' },
      },
    ],
  })

  const result = theme.match(
    ScopeStack.from(
      'text.xml.xsl',
      'meta.tag.xml.template',
      'string.quoted.double.xml',
      'punctuation.definition.string.begin.xml',
    ),
  )

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

Actual result: #00FF00
Expected result: #FF0000

Both selectors match. string.quoted wins only because 13 is greater than 12.

Impact analysis

I measured grammar exposure from direct parent-child edges in grammar scope graphs for the built-in grammar corpus. For every graph edge reachable from a grammar root, I generated valid dot-prefixes of the parent and child scopes and looked for pairs where:

depth(a) > depth(b)
length(a) < length(b)

A grammar was counted when at least one reachable graph edge contained such an inversion opportunity. This found 79 affected grammars out of 83 analyzed grammars, or 95.2%.

I then analyzed theme source rules without estimating author intent:

  1. Expand string, array, and comma-separated selectors.
  2. Preserve each rule's source index and complete settings.
  3. Use the scope graph to find a representative grammar-reachable path where both rules match.
  4. Compare rules with the same terminal selector and identical nearer parent components.
  5. At the first differing parent component, retain pairs where character count and scope depth select opposite winners.
  6. Compare the effective style produced by the current winner with the style produced by the depth-based winner.

This found two style-changing conflicts across 55 themes. Both are the same Atom One Dark rule interaction, once for JavaScript and once for TypeScript.

The staged ignore/analysis/themes/OneDark.json file is minified, so its physical line number is line 1. In a pretty-printed copy of the same JSON, the relevant rules are:

tokenColors[136], pretty lines 1374, 1376-1377
name: [VSCODE-CUSTOM] JS/TS Array variables
scope:
  meta.array-binding-pattern-variable.js variable.other.readwrite.js
  meta.array-binding-pattern-variable.ts variable.other.readwrite.ts
settings:
  foreground: #D19A66

tokenColors[147], pretty lines 1495, 1497-1498
name: [VSCODE-CUSTOM] JS/TS Export Variable
scope:
  meta.export.default.js variable.other.readwrite.js
  meta.export.default.ts variable.other.readwrite.ts
settings:
  foreground: #E06C75

One representative graph-reachable JavaScript path is:

source.js
meta.export.default.js
meta.arrow.js
meta.block.js
meta.var.expr.js
meta.array-binding-pattern-variable.js
variable.other.readwrite.js

You can see the unexpected highlighting with Atom One Dark by pasting this into a JavaScript file:

export default () => {
  let [item] = values
  return item
}

The item binding inside [item] is the affected token. The TypeScript variant is the same shape:

Both Atom One Dark selectors match this path:

meta.array-binding-pattern-variable.js variable.other.readwrite.js
meta.export.default.js variable.other.readwrite.js

The likely intent is that destructured array variables are orange in general, while exported default variables are red. The path is both: it is a variable in an array binding pattern inside an export-default context.

The current comparator chooses the array-binding rule because meta.array-binding-pattern-variable.js has 38 characters, while meta.export.default.js has 22 characters. A scope-depth comparison would choose the export-default rule because meta.export.default.js has four scope atoms and meta.array-binding-pattern-variable.js has three.

Current result:

meta.array-binding-pattern-variable.js variable.other.readwrite.js
foreground: #D19A66

Depth-based result:

meta.export.default.js variable.other.readwrite.js
foreground: #E06C75

The current result is probbably accurate, but it was only because meta.array-binding-pattern-variable.js happens to be longer in characters, while generally authors reason in terms of atom count for specificity.

See this reference on scope selector semantics

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions