Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 46 commits into
base: main
Choose a base branch
from

Conversation

zm-cttae
Copy link

@zm-cttae zm-cttae commented Feb 13, 2023

Closes #12493.

Background

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

This PR allows multiple use cases - see table:

Key to target a file or folder Example filename
* Makefile, Dockerfile
Jenkinsfile.* Jenkinsfile.prod
test_*.py test_component.py
*_test.go component_test.go
*Test.cs CalculatorTest.cs
.env.* .env.development
web.*.config web.prod.config
webpack.*.config.js webpack.dev.config.js
*.webpack.config.js shared.dev.webpack.config.js
.env.*.* .env.development.docker.example.env
*.md pop goes ...................................... the weasel.md

Specification

File icon theme keys are separated by the last dot into basename and ext.

The basename segment allows any number of . characters.

Both segments can be globbed like so:

  • Wildcard glob * accepts any value.
  • Prefix glob prefix*.
  • Suffix glob *suffix.
  • Prefix-suffix glob *middle*.
  • Middle glob prefix*suffix

Implementation

File/folder icon association keys that can be augmented by globs: folderNamesfolderNamesExpandedfileNames

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

  • generates a different CSS attribute selector for basename and/or extname attr values in the file icon theme
  • removes the +2 score boost contributed by the name class

This PR introduces extraAttributes option to icon label APIs. These attributes are anchors that the CSS from the file icon theme side can target. This is necessary to prevent a FOUC after opening any collapsed folder or any text document, caused by file icon themes running 100s of globs per each file label.

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)

EDIT 3:

Redesign implementation in line with #177650 - moves business logic of globs back to the frontend

@zm-cttae
Copy link
Author

@microsoft-github-policy-service agree

@aeschli
Copy link
Contributor

aeschli commented Feb 14, 2023

Can you explain what css classes you generate?

@zm-cttae
Copy link
Author

Done in PR description

@zm-cttae
Copy link
Author

While we are here:

@aeschli
Copy link
Contributor

aeschli commented Feb 14, 2023

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

This comment was marked as outdated.

@zm-cttae
Copy link
Author

Now ready for review

@zm-cttae
Copy link
Author

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

@aeschli
Copy link
Contributor

aeschli commented Feb 14, 2023

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 zm-cttae marked this pull request as draft February 14, 2023 15:11
@zm-cttae
Copy link
Author

zm-cttae commented Feb 14, 2023

NB: I wanted to add an extra level of depth in generating folderNamesfolderNamesExpandedfileNames 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

Refs: [#issuecomment-1429874059](github.com/microsoft/pull/174286#issuecomment-1429874059)
@zm-cttae zm-cttae marked this pull request as ready for review February 14, 2023 16:26
@zm-cttae
Copy link
Author

zm-cttae commented Feb 14, 2023

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.

@zm-cttae zm-cttae changed the title Support globs in file icon themes [icon-themes] Support globs in file icon associations Feb 14, 2023
@aeschli
Copy link
Contributor

aeschli commented Mar 20, 2023

@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
Copy link
Author

zm-cttae commented Mar 20, 2023

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

@seanCodes
Copy link

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

@zm-cttae
Copy link
Author

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
Copy link

Whats the status on this?

@Repiteo
Copy link

Repiteo commented Apr 21, 2024

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
Copy link

muchisx commented Aug 21, 2024

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

@JoaquinFernandez
Copy link

Hope to see this merged soon!

@zm-cttae zm-cttae marked this pull request as draft November 8, 2024 20:16
@zm-cttae zm-cttae marked this pull request as ready for review November 10, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[icon-themes] Support for globs in file associations (Icon themes)
8 participants