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

fix: support ext convention #199

Merged
merged 2 commits into from
Feb 11, 2023

Conversation

nobuhikosawai
Copy link
Collaborator

@nobuhikosawai nobuhikosawai commented Feb 11, 2023

I saw this #191 and I like the idea of supporting *.test.js, *.stories.tsx, etc because these are quite common conventions.

The problem is currently this is not working as intended. (Only test.js highlights but not foo.test.js)

image

This PR is to fix this issue.
image

Note
Whether a plugin can reflect this change depends on the plugin's implementation.
(I see some attempt to support option to switch the way to get icons like this PR: akinsho/bufferline.nvim#669)

image

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Your timing is excellent; I've just been testing #197

This one's a bug, not a feature and doesn't break anything.

Tested OK for cases you referenced, as well as regular cases.

We'll merge that one first.

@@ -1736,7 +1736,7 @@ local function get_default_icon()
end

local function get_icon(name, ext, opts)
ext = ext or name:match("^.*%.(.*)$") or ""
ext = ext or name:match("^.*%.(.*%..*)$") or name:match("^.*%.(.*)$") or ""
Copy link
Member

Choose a reason for hiding this comment

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

Could we simply match "^.*%.(.*%..*)$" without "^.*%.(.*)$"? Greedy match should cover all existing cases...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@alex-courtis Thanks for your feedback. I've updated the regexp pattern to match both 'foo.test.js' and 'foo.js'. Could you double check?

@nobuhikosawai nobuhikosawai changed the title feat: support ext convention fix: support ext convention Feb 11, 2023
Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Tested OK:

bleh.stories.tsx
bleh.tsx
stories.tsx

Will merge #197 first

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Merged master, retested OK.

Many thanks for your contribution.

@alex-courtis alex-courtis merged commit bd7a222 into nvim-tree:master Feb 11, 2023
@hthuong09
Copy link

But then again it breaks other types.

For example, I have a file called cat.service.ts, the extension is now service.ts and it does not match any icon.

I would suggest adding another variable to this case and checking it first before the actual extension.

@alex-courtis
Copy link
Member

Reported #205

Reverting.

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.

None yet

3 participants