feat: add child combinator ">" (and fix a specificity bug)

src/tests/themes.test.ts

@@ -606,6 +606,63 @@ test('Theme resolving rules with parent scopes', () => {
 	assertThemeEqual(actual, expected);
 });
 
+test('Theme resolving a rule with child combinator', () => {
+	let theme = Theme.createFromRawTheme({
+		settings: [
+			{ settings: { foreground: '#100000' } },
+			{ scope: 'b a', settings: { foreground: '#200000' } },
+			{ scope: 'b > a', settings: { foreground: '#300000' } },
+			{ scope: 'c > b > a', settings: { foreground: '#400000' } },
+			{ scope: 'a', settings: { foreground: '#500000' } },
+		]
+	});
+
+	const colorMap = theme.getColorMap();
+	const match = (...path: string[]) => {
+		const result = theme.match(ScopeStack.from(...path));
+		if (!result) {
+			return null;
+		}
+		return colorMap[result.foregroundId];
+	};
+
+	assert.equal(match('b', 'a'), '#300000', 'b a');
+	assert.equal(match('b', 'c', 'a'), '#200000', 'b c a');
+	assert.equal(match('c', 'b', 'a'), '#400000', 'c b a');
+	assert.equal(match('c', 'b', 'd', 'a'), '#200000', 'c b d a');
+});
+
+test('Theme resolving should give deeper scopes higher specificity (#233)', () => {
+	let theme = Theme.createFromRawTheme({
+		settings: [
+			{ settings: { foreground: '#100000' } },
+			{ scope: 'y.z a.b', settings: { foreground: '#200000' } },
+			{ scope: 'x y a.b', settings: { foreground: '#300000' } },
+		]
+	});
+
+	const colorMap = theme.getColorMap();
+	const defaults = theme.getDefaults();
+
+	const match = (...path: string[]) => {
+		const result = theme.match(ScopeStack.from(...path));
+		if (!result || result.foregroundId === 0) {
+			return null;
+		}
+		return colorMap[result.foregroundId];
+	};
+
+	// Sanity check
+	assert.equal(match('x', 'a.b'), null, 'x a.b');
+	assert.equal(match('y', 'a.b'), null, 'y a.b');
+	assert.equal(match('y.z', 'a'), null, 'y.z a');
+	assert.equal(match('x', 'y', 'a.b'), '#300000', 'x y a.b');
+
+	// Even though the "x y a.b" rule has more scopes in its path, the "y.z a.b" rule has
+	// a deeper match, so it should take precedence.
+	assert.equal(match('x', 'y.z', 'a.b'), '#200000', 'y.z a.b');
+});
+
 test('Theme resolving issue #38: ignores rules with invalid colors', () => {
 	let actual = parseTheme({
 		settings: [{

src/theme.ts

@@ -161,26 +161,46 @@ export class ScopeStack {
 	}
 }
 
-function _scopePathMatchesParentScopes(scopePath: ScopeStack | null, parentScopes: ScopeName[] | null): boolean {
-	if (parentScopes === null) {
+function _scopePathMatchesParentScopes(scopePath: ScopeStack | null, parentScopes: readonly ScopeName[]): boolean {
+	if (parentScopes.length === 0) {
 		return true;
 	}
 
-	let index = 0;
-	let scopePattern = parentScopes[index];
+	// Starting with the deepest parent scope, look for a match in the scope path.
+	for (let index = 0; index < parentScopes.length; index++) {
+		let scopePattern = parentScopes[index];
+		let scopeMustMatch = false;
 
-	while (scopePath) {
-		if (_matchesScope(scopePath.scopeName, scopePattern)) {
-			index++;
-			if (index === parentScopes.length) {
-				return true;
+		// Check for a child combinator (a parent-child relationship)
+		if (scopePattern === '>') {
+			if (index === parentScopes.length - 1) {
+				// Invalid use of child combinator
+				return false;
 			}
-			scopePattern = parentScopes[index];
+			scopePattern = parentScopes[++index];
+			scopeMustMatch = true;
+		}
+
+		while (scopePath) {
+			if (_matchesScope(scopePath.scopeName, scopePattern)) {
+				break;
+			}
+			if (scopeMustMatch) {
+				// If a child combinator was used, the parent scope must match.
+				return false;
+			}
+			scopePath = scopePath.parent;
+		}
+
+		if (!scopePath) {
+			// No more potential matches
+			return false;
 		}
 		scopePath = scopePath.parent;
 	}
 
-	return false;
+	// All parent scopes were matched.
+	return true;
 }
 
 function _matchesScope(scopeName: ScopeName, scopePattern: ScopeName): boolean {
@@ -427,17 +447,19 @@ export class ColorMap {
 	}
 }
 
+const emptyParentScopes= Object.freeze(<ScopeName[]>[]);
+
 export class ThemeTrieElementRule {
 
 	scopeDepth: number;
-	parentScopes: ScopeName[] | null;
+	parentScopes: readonly ScopeName[];
 	fontStyle: number;
 	foreground: number;
 	background: number;
 
-	constructor(scopeDepth: number, parentScopes: ScopeName[] | null, fontStyle: number, foreground: number, background: number) {
+	constructor(scopeDepth: number, parentScopes: readonly ScopeName[] | null, fontStyle: number, foreground: number, background: number) {
 		this.scopeDepth = scopeDepth;
-		this.parentScopes = parentScopes;
+		this.parentScopes = parentScopes || emptyParentScopes;
 		this.fontStyle = fontStyle;
 		this.foreground = foreground;
 		this.background = background;
@@ -489,55 +511,77 @@ export class ThemeTrieElement {
 		this._rulesWithParentScopes = rulesWithParentScopes;
 	}
 
-	private static _sortBySpecificity(arr: ThemeTrieElementRule[]): ThemeTrieElementRule[] {
-		if (arr.length === 1) {
-			return arr;
-		}
-		arr.sort(this._cmpBySpecificity);
-		return arr;
-	}
-
 	private static _cmpBySpecificity(a: ThemeTrieElementRule, b: ThemeTrieElementRule): number {
-		if (a.scopeDepth === b.scopeDepth) {
-			const aParentScopes = a.parentScopes;
-			const bParentScopes = b.parentScopes;
-			let aParentScopesLen = aParentScopes === null ? 0 : aParentScopes.length;
-			let bParentScopesLen = bParentScopes === null ? 0 : bParentScopes.length;
-			if (aParentScopesLen === bParentScopesLen) {
-				for (let i = 0; i < aParentScopesLen; i++) {
-					const aLen = aParentScopes![i].length;
-					const bLen = bParentScopes![i].length;
-					if (aLen !== bLen) {
-						return bLen - aLen;
-					}
-				}
+		// First, compare the scope depths of both rules. The “scope depth” of a rule is
+		// the number of segments (delimited by dots) in the rule's deepest scope name
+		// (i.e. the final scope name in the scope path delimited by spaces).
+		if (a.scopeDepth !== b.scopeDepth) {
+			return b.scopeDepth - a.scopeDepth;
+		}
+
+		// Traverse the parent scopes depth-first, comparing the specificity of both
+		// rules' parent scopes, which matches the behavior described by ”Ranking Matches”
+		// in TextMate 1.5's manual: https://macromates.com/manual/en/scope_selectors
+		// Start at index 0 for both rules, since the parent scopes were reversed
+		// beforehand (i.e. index 0 is the deepest parent scope).
+		let aParentIndex = 0;
+		let bParentIndex = 0;
+
+		while (true) {
+			// Child combinators don't affect specificity.
+			if (a.parentScopes[aParentIndex] === '>') {
+				aParentIndex++;
+			}
+			if (b.parentScopes[bParentIndex] === '>') {
+				bParentIndex++;
+			}
+
+			// This is a scope-by-scope comparison, so we need to stop once a rule runs
+			// out of parent scopes.
+			if (aParentIndex >= a.parentScopes.length || bParentIndex >= b.parentScopes.length) {
+				break;
+			}
+
+			// When sorting by scope name specificity, it's safe to treat a longer parent
+			// scope as more specific. If both rules' parent scopes match a given scope
+			// path, the longer parent scope will always be more specific.
+			const parentScopeLengthDiff =
+				b.parentScopes[bParentIndex].length - a.parentScopes[aParentIndex].length;
+
+			if (parentScopeLengthDiff !== 0) {
+				return parentScopeLengthDiff;
 			}
-			return bParentScopesLen - aParentScopesLen;
+
+			aParentIndex++;
+			bParentIndex++;
 		}
-		return b.scopeDepth - a.scopeDepth;
+
+		// If a depth-first, scope-by-scope comparison resulted in a tie, the rule with
+		// more parent scopes is considered more specific.
+		return b.parentScopes.length - a.parentScopes.length;
 	}
 
 	public match(scope: ScopeName): ThemeTrieElementRule[] {
-		if (scope === '') {
-			return ThemeTrieElement._sortBySpecificity((<ThemeTrieElementRule[]>[]).concat(this._mainRule).concat(this._rulesWithParentScopes));
-		}
-
-		let dotIndex = scope.indexOf('.');
-		let head: string;
-		let tail: string;
-		if (dotIndex === -1) {
-			head = scope;
-			tail = '';
-		} else {
-			head = scope.substring(0, dotIndex);
-			tail = scope.substring(dotIndex + 1);
-		}
+		if (scope !== '') {
+			let dotIndex = scope.indexOf('.')
+			let head: string
+			let tail: string
+			if (dotIndex === -1) {
+				head = scope
+				tail = ''
+			} else {
+				head = scope.substring(0, dotIndex)
+				tail = scope.substring(dotIndex + 1)
+			}
 
-		if (this._children.hasOwnProperty(head)) {
-			return this._children[head].match(tail);
+			if (this._children.hasOwnProperty(head)) {
+				return this._children[head].match(tail);
+			}
 		}
 
-		return ThemeTrieElement._sortBySpecificity((<ThemeTrieElementRule[]>[]).concat(this._mainRule).concat(this._rulesWithParentScopes));
+		const rules = this._rulesWithParentScopes.concat(this._mainRule);
+		rules.sort(ThemeTrieElement._cmpBySpecificity);
+		return rules;
 	}
 
 	public insert(scopeDepth: number, scope: ScopeName, parentScopes: ScopeName[] | null, fontStyle: number, foreground: number, background: number): void {

src/utils.ts

@@ -105,7 +105,7 @@ export function strcmp(a: string, b: string): number {
 	return 0;
 }
 
-export function strArrCmp(a: string[] | null, b: string[] | null): number {
+export function strArrCmp(a: readonly string[] | null, b: readonly string[] | null): number {
 	if (a === null && b === null) {
 		return 0;
 	}