[icon-themes] Support glob keys in file icon associations

[zm-cttae]

Closes #12493.

Background

The feature request is for glob support in the key of icon mappings for file icon themes

This PR sets up a number of glob use cases - see table:

Key to target a file or folder	Example filename
Jenkinsfile.*	Jenkinsfile.prod
test_*.py (special case for segment.ext)	test_component.py
*_test.go (special case for segment.ext)	component_test.go
.env.*	.env.development
web.*.config	web.prod.config
webpack.*.config.js	webpack.dev.config.js
pop goes the weasel.*	pop goes ...................................... the weasel.md

Specification

In file icon theme keys, add * to match A. 1 dot segment OR B. dash-separated basename.

• For a filename with max 4 dot segments, we can use * to glob second, third or fourth segment (wildcard-glob)
• Support for globbing dash separator with segment.ext using prefix and suffix globs (e.g. test_*.py or *_test.go)
  • Suffix glob using test_*.py (basename-prefix-with-undescore)
  • Prefix glob using *_test.go (basename-suffix-with-undescore)

Implementation

File/folder icon association keys that can be augmented by globs: folderNames, folderNamesExpanded, fileNames

Any * in the key of the file/folder icon mapping:

• changes the CSS class in the file icon theme into a glob (-glob-file-icon or -glob-folder-icon) suffix
• removes the +1 name class score boost

We generate class names for globs on the icon CSS service side. This is necessary to prevent a FOUC when opening any collapsed folder or any text document. See algorithm complexity writeup @ #12493 (comment)
The output classes are demoed here: #174286 (comment)

EDIT:

This was updated to remove ** from this spec and sync with vscode.GlobPattern spec.
It previously meant >=2 dot segments - #174286 (comment)

EDIT 2:

Apply changeset from #174286 (comment)

[zm-cttae]

@microsoft-github-policy-service agree

[aeschli]

Can you explain what css classes you generate?

[zm-cttae]

Done in PR description

[zm-cttae]

While we are here:

• -name-dir-icon was not the proposed CSS class suffix in [icon-themes] Support for folder / directory names in file associations (Icon themes) #136656 and the current -name-dir-icon looks ugly..
• Can I make a new PR to clean that up (into -dir-${kind}-icon)?

[aeschli]

Sorry, can you give examples how this works (maybe a table covering the various cases, prefix, suffix, **)

• file/folder pattern in the icon theme
• all classes in the css rules that are generated

Give some example of resources in the explorer and how the class names match. The tricky part is that file name matches are always stronger than file extension matches and stronger than fold name matches.

[zm-cttae]

Fair enough: updated replit with table

File name	Glob classes before escaping (getIconClasses)	Number of globs generated	Time (ms)
"Makefile"		0	0.3
"Makefile"		0	0.1
"Jenkinsfile.prod"	Jenkinsfile.*-glob-file-icon	1	0.1
"test_component.py"	test_component.*-glob-file-icon test_*.py-glob-file-icon *_component.py-glob-file-icon	3	0.1
"component_test.go"	component_test.*-glob-file-icon component_*.go-glob-file-icon *_test.go-glob-file-icon	3	0.0
".env.development"	.*.development-glob-file-icon .env.*-glob-file-icon	2	0.0
"web.prod.config"	web.*.config-glob-file-icon web.prod.*-glob-file-icon	2	0.0
"webpack.dev.config.js"	webpack.*.config.js-glob-file-icon webpack.dev.*.js-glob-file-icon webpack.dev.config.*-glob-file-icon	3	0.0
"shared.dev.webpack.config.js"		0	0.0
".env.development.docker.example.env"		0	0.0
"pop goes ...................................... the weasel.md"	pop goes the weasel.*-glob-file-icon	1	0.0

[zm-cttae]

Now ready for review

[zm-cttae]

Made some small tweaks in my work lunch, but please feel free to review now @aeschli

[aeschli]

ok, now I understand. So * is only possible at the beginning or end of dot, underscore or minus separated segments.

The problem is to make sure the matching order holds:

'full file match' beats 'file match with pattern' beats 'file extension match' beats 'file extension with pattern' beats language beats folder name beats folder name with

How this is implemented is the number of classes matching

IMO we're at a dead end with CSS class name matching. I'm reluctant in adding even more class names.

[zm-cttae]

NB: I wanted to add an extra level of depth in generating folderNames, folderNamesExpanded, fileNames but its not needed
I've had a change of tack ("let's use name-${kind}-icon") and we can bypass this issue in a saner manner

[zm-cttae]

That's done. BTW:

* is only possible at the beginning or end of dot, underscore or minus separated segments.

The "underscore or minus separated segments" feature, that is matching targets filenames in this format:

alphanumeric.ext

This is for a specific use case with A11Y benefit as explained here - vscode-icons/vscode-icons#2754

Took 18 lines of code to implement, but I hope this feature is worth it.

[aeschli]

I'm all in favour of simplifying the syntax (and capabilities). The suggested solution is not trivial, complicates the current implementation but still doesn't provide the full glob support that (I suspect) file-icon-theme authors are hoping for.

One idea is to only support test_*.py (basename-prefix-with-undescore) and *_test.go (basename-suffix-with-undescore).
But then users will ask about support for -. camelCase, other separators...

If you have any ideas, let's discuss before implementing.

[zm-cttae]

But then users will ask about support for -. camelCase, other separators...

Because of laravel+php I will be implementing CamelCaseTest boundary separator. That one seems trivial...

[aeschli]

If you want this PR to be merged, we first need to first communicate and agree on an approach.
I want a proposal that is easy to understand for users and has a simple implementation that we can maintain.

I recommend you to wait with implementing before we agree.

[zm-cttae]

Okay. Would you like the commits on CamelCase and regexp class move to be reverted?
I have reverted these commits.
Happy to do anything if you wish, it's your Owned Product!

The suggested solution is not trivial, complicates the current implementation but still doesn't provide the full glob support that (I suspect) file-icon-theme authors are hoping for.

Not the feedback I was hoping for but I really think these use cases are all good to add. (With dash/camel as less key maybe).
But if this pull needs to be minimal i.e. less than +100 for code addition, I will revert features as necessary.

One idea is to only support test_*.py (basename-prefix-with-undescore) and *_test.go (basename-suffix-with-undescore).

I feel the . separator is the most popular use case - if you'd like to limit the PR to just that, we can impl+test . separator only. If you only want to support dash separator, I'd like to push for only . instead purely based on feature popularity & the maintainability of existing icon theme wiring.

Eagerly awaiting some direction here 🙂

[zm-cttae]

You might still think "well, . is very much covered by filename extensions, so lets just add dash support."
If so, I'm happy to go ahead and just limit ourselves to basename-*-with-underscore & non-coalescing-wildcard support.

[zm-cttae]

@aeschli please look at 5c5ec59...6256740 from a PR fork where I aggressively cut down the spec to a sane level (3 independent glob features).

• non-coalescing-wildcard
• basename-prefix-with-undescore
• basename-suffix-with-undescore

Simplified specification

• Can use * to match any 1 dot segment of a filename
• For a filename with max 4 dot segments, we can use * to glob second, third or fourth segment (base-glob).
• * works as an alias for two or more * dot segments in an icon key (coaelesced-glob)
• More than 4 dot segments reduces output significantly to $O(N)$ (2n-2) linear implementation:
  • Match any one segment with a * wildcard (base-glob)
  • Match any amount of dot segments via prefix - uses * wildcard (suffix-glob)
• Support for globbing dash separator with segment.ext using prefix and suffix globs (e.g. test_*.py or *_test.go)
  • Suffix glob using test_*.py or test-*.py (basename-prefix-glob)
  • Prefix glob using *-test.go or *-test.go (basename-suffix-glob)

Changeset

• Removal: globbing the first dot segment; *.md
• Removal: prefix coalescing glob; .env.* ->.env.development.docker.example.env
• Removal: glob support for >=5 filename dot segments
• Removal: wildcard chaining via "permutative-glob"; {webpack.*.*.js,webpack.config.*.*} -> webpack.dev.config.js

[aeschli]

@zm-cttae Sorry I didn't get back earlier, it was quite a busy week.

I like your last change. Thanks a lot!

I wanted to bring up an idea. Could we use the CSS Substring matching selectors) to further reduce the number of classes needed and allow some more ways on how theme authors can use the *?

See #177650 to see a sketch of this.

What do you think?

[zm-cttae]

I'll review by COB tomorrow (Tuesday 21/03). Merged in my reduced spec and will update PR description today

[seanCodes]

Hi @zm-cttae, is this something you're still working on?

[zm-cttae]

This is ready to go but we have a better impl that @aeschli demoed - not clear whether that's been packaged up. We're also both busy, it seems. (Currently there's endgame for VS Code & I'm wrangling with Textmate library migration).

[thelooter]

Whats the status on this?

[Repiteo]

Hoping this gets off the backburner soon. A lack of glob patterns has been the most obvious weakpoint of the icon API for nearly a decade, and it feels like we're so close to finally fixing that.

[muchisx]

🙏🏼 Looking forward for this being implemented soon @zm-cttae @aeschli