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

Improve custom patching #1219

Merged
merged 3 commits into from
May 10, 2023
Merged

Improve custom patching #1219

merged 3 commits into from
May 10, 2023

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented May 9, 2023

Description

This changes how custom patching (i.e. with --custom) is handled.
When I tried to use the feature myself several shortcomings became obvious.

Requirements / Checklist

What does this Pull Request (PR) do?

How should this be manually tested?

Any background context you can provide?

What are the relevant tickets (if any)?

This is mostly triggered by @siduck 's problem

Screenshots

@Finii Finii force-pushed the feature/improve-custom-patching branch from a2cab92 to b996a43 Compare May 9, 2023 14:50
@siduck
Copy link

siduck commented May 9, 2023

@Finii thanks for adding file-dir icon back man :D

@siduck
Copy link

siduck commented May 9, 2023

Sorry to disappoint you, but this is not putting the old fashioned icon back into the Nerd Font release, but instead is aimed to enable you to create your very own fonts with the icon.

cool, would be lot of work on my side tho! there are thousands of nvchad ( my nvim config ) users probably and they'd have to do the same. I guess we'd have to do it after updating our nerdfont package too

@Finii
Copy link
Collaborator Author

Finii commented May 9, 2023

cool, would be lot of work on my side tho! there are thousands of nvchad ( my nvim config ) users probably and they'd have to do the same. I guess we'd have to do it after updating our nerdfont package too

This is not what this was intended for. Why in the world do you insist on an obsolete old fashioned icon. I could understand that you personally have a strong(er) preference, but 'the users' in general should get the updates that they want and deserve.
And how bad are fonts where ONE directory icon looks different then all the matching others?

That ugly icon is gone. Go with the times and use a current style icon. At least if you repackage.

If you prepare DIFFERENT Nerd Fonts packages this will lead to maintenance hell. All the users will come here and complain why Octicons have been updated but obviously one icon has been forgotten. That is a nightmare.

@Finii Finii closed this May 9, 2023
@Finii Finii deleted the feature/improve-custom-patching branch May 9, 2023 16:09
@Finii Finii restored the feature/improve-custom-patching branch May 9, 2023 16:22
@Finii Finii reopened this May 9, 2023
@Finii Finii closed this May 9, 2023
@Finii Finii force-pushed the feature/improve-custom-patching branch from b996a43 to 3445d2d Compare May 9, 2023 16:24
@Finii Finii reopened this May 9, 2023
@Finii Finii closed this May 9, 2023
@Finii Finii force-pushed the feature/improve-custom-patching branch from 1700577 to 3445d2d Compare May 9, 2023 16:38
@Finii
Copy link
Collaborator Author

Finii commented May 9, 2023

This is so utterly disappointing. Create your own set of fonts, but call them differently. Maybe NvchatFont or whatever, but DO NOT publish packages with tampered-with Nerd Fonts :-(

Spent several hours on you problem and this is how you thank me.

@Finii
Copy link
Collaborator Author

Finii commented May 9, 2023

nvchad users probably and they'd have to do the same

Why on earth should they want an icon that has been dropped from Octo years ago and does not fit nicely with all others 🙄

@Finii Finii deleted the feature/improve-custom-patching branch May 9, 2023 16:56
@siduck
Copy link

siduck commented May 9, 2023

nvchad users probably and they'd have to do the same

Why on earth should they want an icon that has been dropped from Octo years ago and does not fit nicely with all others roll_eyes

it looks better imo and the users are already complaining when their fav folder icon got removed! And that icon does matches with other folder related icons we use ( cod* ), it looks like a filled version of the cod-folder, thats why I like it

image

I wouldnt be bothered much if I was the only one using this icon, it has become a part of our UI since the beginning and made our folders unique ( similar to doom emacs filetree icons )

I really appreciate your work tho, no need to get mad! I never meant to be rude / disrespectful in our conversations. Just wanted to bring that icon back on a large scale

Also the new oct-file-directory icon is too close to the text & its icon is bigger than the text itself.

image

old one had better padding by default & isnt bigger than the text

image
I know i could use the mono fonts as they're well centered but small icons :(

image

@Finii
Copy link
Collaborator Author

Finii commented May 9, 2023

The old icon could only move into nf-custom- which is where we preserved some other old ones, but that means a codepoint change.

@siduck
Copy link

siduck commented May 9, 2023

The old icon could only move into nf-custom- which is where we preserved some other old ones, but that means a codepoint change.

a codepoint change wouldnt do any harm as the icon's still preserved, I'd just change it here https://github.com/NvChad/NvChad/blob/v2.0/lua/plugins/configs/nvimtree.lua#L51 and it'd get updated for all nvchad users

@Finii
Copy link
Collaborator Author

Finii commented May 10, 2023

What will happen when Microsoft updates its Codicons?
At the moment they kept the symbols, but what if.
Other icon changes will occur in the future with modernization I'm sure.

Finii added 2 commits May 10, 2023 12:01
[why]
If you add a custom icon it probably has to fit with the rest of the
font. So why should we patch it in completely unscaled?

[how]
Just scale the custom icon like all other icons at least.

If someone wants to add just a symbol or two and keep the glyphs exactly
(more or less) unscaled there are simpler ways than this patcher
script. I believe persons that are able to pre-scale the custom font are
also able to just patch it in. So this option is more for the generic
patching of extra glyphs but they need to be scaled somehow.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
When a user want to patch some glyph in, and at that position is already
a glyph, there is no way to overwrite it.

The reason is that --custom glyphs are always patched in --careful, and
there is no --not-careful option to override that.

[how]
Add glyphs via --custom always, except --careful has also been given.

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii restored the feature/improve-custom-patching branch May 10, 2023 10:03
@Finii Finii reopened this May 10, 2023
…ath [skip ci]

[why]
While the commit
  7cda326 font-patcher: Allow to specify custom symbolfont with absolute path

would have allowed to specify the custom glyph font with an absolute
path, it is in fact still not possible.

[how]
The file-exists checks also need to observe the absolute path.

[note]
Normally (with a relative path) the custom font is search for in the
ordinary glyphs directory - but that would mean people need to copy it
there.

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii force-pushed the feature/improve-custom-patching branch from fa6bb72 to af2573a Compare May 10, 2023 10:05
@Finii Finii marked this pull request as ready for review May 10, 2023 10:06
@Finii Finii merged commit 252ac6b into master May 10, 2023
@Finii Finii deleted the feature/improve-custom-patching branch May 10, 2023 10:07
@siduck
Copy link

siduck commented May 10, 2023

What will happen when Microsoft updates its Codicons? At the moment they kept the symbols, but what if. Other icon changes will occur in the future with modernization I'm sure.

we use those folders only at few places and they're rarely visible, so not much of big deal. but that file-directory icon was our main one, we used it at the statusline etc too

@Finii
Copy link
Collaborator Author

Finii commented May 10, 2023

Found other bugs while moving the icon to custom ... 🙄
Some custom icons are broken slightly deformed ?!

I fear it (the folder icon) will be patched a bit more to the right, because the custom icons are all centered (on something) while the octicons were left aligned. Need to check how much.

@Finii
Copy link
Collaborator Author

Finii commented May 10, 2023

Hmm, it is still left aligned but a bit smaller 🤔

image

@siduck
Copy link

siduck commented May 10, 2023

custom icons are all centered

didnt know! I thought only the fonts that end with "Mono" are centered 🤔

@siduck
Copy link

siduck commented May 10, 2023

Hmm, it is still left aligned but a bit smaller thinking

image

oops looks a bit smaller 😨

LNKLEO pushed a commit to LNKLEO/Nerd that referenced this pull request Nov 24, 2023
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.

2 participants