ryanoasis / nerd-fonts

Iconic font aggregator, collection, & patcher. 3,600+ icons, 50+ patched fonts: Hack, Source Code Pro, more. Glyph collections: Font Awesome, Material Design Icons, Octicons, & more
https://NerdFonts.com
Other
53.79k stars 3.62k forks source link

Add Neovim icon (fixes #1383) #1391

Closed georgeguimaraes closed 10 months ago

georgeguimaraes commented 10 months ago

Description

I decided to take a shot into implementing #1383. Not sure if this is it, or if I should run bin/scripts/gotta-patch-em-all-font-patcher!.sh and commit all the changes. Let me know if this is ok.

Requirements / Checklist

What does this Pull Request (PR) do?

Adds the neovim icon as discussed in #1383

How should this be manually tested?

The generated original-source.otf now has the Neovim icon:

CleanShot 2023-10-20 at 15 04 30

Any background context you can provide?

What are the relevant tickets (if any)?

1383

Screenshots (if appropriate or helpful)

Finii commented 10 months ago

Thanks for the PR!

To add a icon one just needs to throw the svg into the correct directory and add a line to icons.tsv. The workflow than automagically updates the i_seti.sh and the original-source.otf (workflow PackSVGs). Maybe we should add that to the comments in icons.tsv?

Manually one needs to

In the original issue you mentioned font-logos. There is a PR pending to add the neovim icon there. You correctly pointed out that the repo seems dead. But in fact, after #1391 has been raised I investigated and it seems we can revive the repo (see Issue 95 over there). That would of course be preferred.

So maybe we should wait a bit and see if we can get that going again. The messages seemed rather promising.

Finii commented 10 months ago

I believe the font-logos neovim icon has a solid lift-side-bar thing, while here it is hollow (to reflect that it is blue instead of green, a guess). This is just an observation.

Screenshot 2023-10-21 at 12 44 55

Taken from font-logos fork release

Finii commented 10 months ago

The original black-and-white logo published by neovim themselves is like THIS logo, i.e. with a hollow left leg:

https://github.com/neovim/neovim.github.io/blob/master/logos/neovim-logo-1color.svg

georgeguimaraes commented 10 months ago

In the original issue you mentioned font-logos. There is a PR pending to add the neovim icon there. You correctly pointed out that the repo seems dead. But in fact, after https://github.com/ryanoasis/nerd-fonts/pull/1391 has been raised I investigated and it seems we can revive the repo (see https://github.com/lukas-w/font-logos/issues/95). That would of course be preferred.

That's interesting! Let's wait then if font-logos comes back to activity. It seems that the maintainer has not given access to other people.

However, I think there may be an underlying question here: Should Nerd Fonts rely on upstream repos that (as you mentioned in that issue) appear to have no standalone use? As oppose to, I'd say, Material Design. Of course, that's just me thinking out loud, it's a questions for nerd fonts' maintainers to discuss. :)

Just hoping to get Neovim icon soon :)

georgeguimaraes commented 10 months ago

The original black-and-white logo published by neovim themselves is like THIS logo, i.e. with a hollow left leg:

https://github.com/neovim/neovim.github.io/blob/master/logos/neovim-logo-1color.svg

Yes, that's the one I used (hollow left leg).

georgeguimaraes commented 10 months ago

To add a icon one just needs to throw the svg into the correct directory and add a line to icons.tsv. The workflow than automagically updates the i_seti.sh and the original-source.otf (workflow PackSVGs). Maybe we should add that to the comments in icons.tsv?

I guess this paragraph in the contributing file, in a entry regarding new icons, should be enough.

georgeguimaraes commented 10 months ago

btw @Finii, tks for the interactions here! I really appreciate the care you have with nerd fonts and all the work

georgeguimaraes commented 10 months ago

Manually one needs to

  • expand the patching range in font-patcher

You mean this line https://github.com/ryanoasis/nerd-fonts/blob/18ff974fbab4eaf56d0c02d1a3e49492a44197bd/font-patcher#L1054?

It seems that U+E5AE is already included in that range. Is that so?

Finii commented 10 months ago

It seems that U+E5AE is already included in that range. Is that so?

Oh, I anticipated that already in https://github.com/ryanoasis/nerd-fonts/commit/869d6f93513ddca7a0bddfd999002daaf2d00589, so there is nothing to do in font-patcher :-)

Screenshot 2023-10-22 at 23 51 08
Finii commented 10 months ago

Thanks again for the PR.

I hope the changes of the documentation is readable and helps a bit.

Here you can see the workflow in action that crates the new original font: https://github.com/ryanoasis/nerd-fonts/actions/runs/6612722696/job/17959104734

And here the automatic commits added after your PR:

image

So the changes are now in the zip and the docker patcher. All the prepatched fonts will only follow on the next release - to have it already now you need to self patch (or tell me which font(s) you need, I can prepare them)

georgeguimaraes commented 10 months ago

Nice changes to contributing.md. Much clearer to understand that all the heavy lifting of patching the fonts and committing them to the repo is done by the workflows.

I'll try to patch Victor Mono by myself. If I don't succeed I'll ask for help. Thanks so much!

Finii commented 10 months ago

@allcontributors please add @georgeguimaraes for code

allcontributors[bot] commented 10 months ago

@Finii

I've put up a pull request to add @georgeguimaraes! :tada:

Finii commented 10 months ago

Nearly forgot...

I always struggle with the type; code seems not fitting, but all other are also not really good. I believe we used code before for this kind of contribution.

berlinx2104 commented 10 months ago

Screenshot from 2023-10-27 20-57-47 Can you guys reconsider that the left part should be overpainted so it's actually easier to see at the terminal? That edge is really small to see at the terminal. Anyway, thank you so much for taking the time to make it and I respect all your decisions.

Finii commented 10 months ago

@berlinx2104 Thanks for the feedback!

I will try to improve the visibility.

The completely filled symbol is already in Font Logos (see links above) which I expect will pull the PR that contains it soon and afterwards we update that here, so both variants will end up in the patched fonts. (But unfortunately Lukas did not make one of us Maintainer, yet)

georgeguimaraes commented 10 months ago

CleanShot 2023-10-27 at 13 51 07

I kinda got used to see it as a modified "V", but yes, the left leg is too subtle. Maybe it was a design decision by the Neovim team?

Finii commented 10 months ago

Do you think this is better, or not enough?

image

Finii commented 10 months ago

Corrected the aspect ratio:

image

Finii commented 10 months ago

Maybe it was a design decision by the Neovim team?

Well, it was designed to be displayed at a much bigger size. That is why icons come in different sizes, because rescaling usually is not good enough. Here I increased the gap and increased the 'stroke width' of the left leg. In that process I changed the angle of the middle-bar. If I do not change that the ascpect ratio changes, see image above.

georgeguimaraes commented 10 months ago

That looks way better (the 2nd version)

Finii commented 10 months ago

Ok, I will push that, so that you (both) can try it out in real...

Finii commented 10 months ago

Changes pushed via 27f6ea142d40

image

ZIP and docker ready.

georgeguimaraes commented 10 months ago

CleanShot 2023-10-27 at 15 16 10

It is certainly more visible. I can clearly see the hollow leg on my terminal now. Thanks, @Finii

berlinx2104 commented 10 months ago

Thanks, @Finii Screenshot from 2023-10-29 00-29-42