Skip to content

Match multiple file extensions #52

Merged
Claudiohbsantos merged 2 commits intomaterial-extensions:masterfrom
zm-cttae:feat-multiple-file-extensions
Apr 27, 2023
Merged

Match multiple file extensions #52
Claudiohbsantos merged 2 commits intomaterial-extensions:masterfrom
zm-cttae:feat-multiple-file-extensions

Conversation

@zm-cttae
Copy link
Contributor

Copy link
Member

@Claudiohbsantos Claudiohbsantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @zm-cttae ! Thanks for the pull request, and I'm very sorry it has sat open for so long.

I took a look at the ticket and I'm not entirely clear on what problem we're trying to solve here. Could you give me a couple of examples of files that are being missed by the current logic but that would be correctly picked up with these changes?

Thanks!

// https://github.com/microsoft/vscode/issues/116199
if (fileName.length <= 255) {
for (let i = 0; i < fileName.length; i += 1) {
if (fileName[i] === '.') fileExtensions.push(fileName.slice(i + 1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm the intended behavior, for a file name.ext1.ext2.ext3 we expect fileExtensions to be ['ext1.ext2.ext', 'ext2.ext3', 'ext.3'] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds right. This is the same implementation that's present in core VS Code UI

return iconMap.fileExtensions[fileExtension];
if (iconMap.languageIds[fileExtension] && !isDir && !isSubmodule)
return iconMap.languageIds[fileExtension];
for (const ext of fileExtensions) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming that the intention here is to match the first (left-most) extension that matches a known icon. Is that the idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

@zm-cttae
Copy link
Contributor Author

Before this PR, any fileExtensions entry with one or more dot was broken. This fixes all of them. You can test this on a repository with spec.ts test.ts spec.js test.js extensions

@Claudiohbsantos Claudiohbsantos merged commit bb53d96 into material-extensions:master Apr 27, 2023
@zm-cttae zm-cttae deleted the feat-multiple-file-extensions branch April 27, 2023 07:56
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.

Matching multiple file extensions automatically

2 participants