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: update ts, cts, mts and tsconfig.json icon #376

Closed
wants to merge 1 commit into from

Conversation

ofseed
Copy link
Contributor

@ofseed ofseed commented Jan 12, 2024

The original ts and js icons keeps the same style:

image

But 808627b and 33e27b8 break this:

image

So I recommend to update the ts icon as well:

image

And use the original ts icon for tsconfig.json, because they are very similar.

image

But what I recommend most is to revert these two PRs directly. There is no reason to do this update, and this also affects the uniform style of other icons, for test.js

image

This new icon is a completely different style:

image

Note:
I'm not swapping icons for tsconfig and ts, These two icons look the same but they have very subtle differences. These are two icons with different code points. Compare to ts:

e69d(nf-seti-tsconfig):
image

f06e6(nf-md-language_typescript):
image

@alex-courtis
Copy link
Member

alex-courtis commented Jan 13, 2024

Thank you for finding all these inconsistencies in the various script icons.

The test icons were added #215 based on vscode and are not "official" javascript icons. It appears that they are the material icons rather than the seti icons.

@alex-courtis
Copy link
Member

alex-courtis commented Jan 13, 2024

We appear to be in a loop here, moving between icon sets.

We are using seti and material icons however we are inconsistent.

nvim-web-devicons current:
20240113_151658

VSCode default seti icons:
20240113_151358

VSCode material icons:
20240113_151415

@alex-courtis
Copy link
Member

alex-courtis commented Jan 13, 2024

Idea: audit the icons noting the class and hex.

This could be done during generate_colors.lua and comments added to the icon. Lua 5.3+ utf8 support would provide this.

@gegoune
Copy link
Collaborator

gegoune commented Jan 13, 2024

Maybe it's time to introduce icon sets? Or pick one style and support only that one. Consistency is a good thing.

@alex-courtis
Copy link
Member

Maybe it's time to introduce icon sets? Or pick one style and support only that one. Consistency is a good thing.

Yes. We seem to mainly use seti, octicons, devicons and material.

Let's see how an audit goes.

@alex-courtis
Copy link
Member

Some analysis, mapping the icons to their nerd-font classes:

https://gist.github.com/alex-courtis/26271d581e390fe26058bf0743539cc3

TL;DR: we are using mostly seti and dev with some material design and font awesome.

Maybe it's time to introduce icon sets? Or pick one style and support only that one. Consistency is a good thing.

I'm happy to pick seti and use that for the defaults.

Users can add different icon sets via setup and I think there is a material configuration out there somewhere. A contributor could add icon sets with plumbing to nvim-web-devicons if they are motivated.

It might be time to reopen #192 which will be simpler now that we have the generation scripts.

@alex-courtis
Copy link
Member

Many thanks @ofseed and all for the great discussion.

I've updated #192 to move to css class names.
#391 will then consolidate on seti icons
#392 will add a material set

I'll close this and #377 - we will resolve all the js/ts icons in one change.

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