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

Update icons to use Nerd Fonts v3. #2604

Closed
wants to merge 1 commit into from

Conversation

gotgenes
Copy link

@gotgenes gotgenes commented May 6, 2023

  • PR Description

This updates icons to use Nerd Fonts v3 which had breaking changes. I used all the recommendations from nerdfix check except for the Azure icon, which I changed to nf-md-microsoft_azure.

Fixes #2603.

Output from nerdfix

note: Found obsolete icon U+F81A
   ┌─ ./pkg/gui/presentation/icons/file_icons.go:92:34
   │
92 │     ".cs":             "\uf81a", // 
   │                                     ^ Icon 'nf-mdi-language_csharp' is marked as obsolete
   │
   = You could replace it with:
         1. 󰌛 U+F031B nf-md-language_csharp
         2. 󰙲 U+F0672 nf-md-language_cpp
         3. 󰙱 U+F0671 nf-md-language_c
         4. 󰌜 U+F031C nf-md-language_css3

note: Found obsolete icon U+F81A
   ┌─ ./pkg/gui/presentation/icons/file_icons.go:95:34
   │
95 │     ".csproj":         "\uf81a", // 
   │                                     ^ Icon 'nf-mdi-language_csharp' is marked as obsolete
   │
   = You could replace it with:
         1. 󰌛 U+F031B nf-md-language_csharp
         2. 󰙲 U+F0672 nf-md-language_cpp
         3. 󰙱 U+F0671 nf-md-language_c
         4. 󰌜 U+F031C nf-md-language_css3

note: Found obsolete icon U+F81A
   ┌─ ./pkg/gui/presentation/icons/file_icons.go:98:34
   │
98 │     ".csx":            "\uf81a", // 
   │                                     ^ Icon 'nf-mdi-language_csharp' is marked as obsolete
   │
   = You could replace it with:
         1. 󰌛 U+F031B nf-md-language_csharp
         2. 󰙲 U+F0672 nf-md-language_cpp
         3. 󰙱 U+F0671 nf-md-language_c
         4. 󰌜 U+F031C nf-md-language_css3

note: Found obsolete icon U+F718
    ┌─ ./pkg/gui/presentation/icons/file_icons.go:187:34
    │
187 │     ".license":        "\uf718", // 
    │                                     ^ Icon 'nf-mdi-file_document' is marked as obsolete
    │
    = You could replace it with:
          1. 󰈙 U+F0219 nf-md-file_document
          2. 󰷈 U+F0DC8 nf-md-file_document_edit
          3. 󱪗 U+F1A97 nf-md-file_document_alert
          4. 󱪝 U+F1A9D nf-md-file_document_plus

note: Found obsolete icon U+F898
    ┌─ ./pkg/gui/presentation/icons/file_icons.go:213:34
    │
213 │     ".node":           "\uf898", // 
    │                                     ^ Icon 'nf-mdi-nodejs' is marked as obsolete
    │
    = You could replace it with:
          1. 󰎙 U+F0399 nf-md-nodejs
          2.  U+E719 nf-dev-nodejs
          3. 󰡄 U+F0844 nf-md-vuejs

note: Found obsolete icon U+F718
    ┌─ ./pkg/gui/presentation/icons/file_icons.go:254:34
    │
254 │     ".rtf":            "\uf718", // 
    │                                     ^ Icon 'nf-mdi-file_document' is marked as obsolete
    │
    = You could replace it with:
          1. 󰈙 U+F0219 nf-md-file_document
          2. 󰷈 U+F0DC8 nf-md-file_document_edit
          3. 󱪗 U+F1A97 nf-md-file_document_alert
          4. 󱪝 U+F1A9D nf-md-file_document_plus

note: Found obsolete icon U+FD42
    ┌─ ./pkg/gui/presentation/icons/file_icons.go:293:34
    │
293 │     ".vue":            "\ufd42", // ﵂
    │                                     ^ Icon 'nf-mdi-vuejs' is marked as obsolete
    │
    = You could replace it with:
          1. 󰡄 U+F0844 nf-md-vuejs
          2. 󰎙 U+F0399 nf-md-nodejs

note: Found obsolete icon U+FB2B
   ┌─ ./pkg/gui/presentation/icons/git_icons.go:10:36
   │
10 │     BRANCH_ICON         = "\ufb2b" // שׂ
   │                                       ^ Icon 'nf-mdi-source_branch' is marked as obsolete
   │
   = You could replace it with:
         1. 󰘬 U+F062C nf-md-source_branch
         2. 󱓊 U+F14CA nf-md-source_branch_plus
         3. 󱓎 U+F14CE nf-md-source_branch_sync
         4. 󱓍 U+F14CD nf-md-source_branch_refresh

note: Found obsolete icon U+FC16
   ┌─ ./pkg/gui/presentation/icons/git_icons.go:13:36
   │
13 │     COMMIT_ICON         = "\ufc16" // ﰖ
   │                                       ^ Icon 'nf-mdi-source_commit' is marked as obsolete
   │
   = You could replace it with:
         1. 󰜘 U+F0718 nf-md-source_commit
         2. 󰜝 U+F071D nf-md-source_commit_start
         3. 󰜙 U+F0719 nf-md-source_commit_end
         4. 󰜛 U+F071B nf-md-source_commit_local

note: Found obsolete icon U+FB2C
   ┌─ ./pkg/gui/presentation/icons/git_icons.go:14:36
   │
14 │     MERGE_COMMIT_ICON   = "\ufb2c" // שּׁ
   │                                       ^ Icon 'nf-mdi-source_merge' is marked as obsolete
   │
   = You could replace it with:
         1. 󰘭 U+F062D nf-md-source_merge
         2. 󰽜 U+F0F5C nf-md-merge
         3. 󱓠 U+F14E0 nf-md-set_merge
         4. 󰃸 U+F00F8 nf-md-call_merge

note: Found obsolete icon U+F7A1
   ┌─ ./pkg/gui/presentation/icons/git_icons.go:15:36
   │
15 │     DEFAULT_REMOTE_ICON = "\uf7a1" // 
   │                                       ^ Icon 'nf-mdi-git' is marked as obsolete
   │
   = You could replace it with:
         1. 󰊢 U+F02A2 nf-md-git
         2.  U+E65D nf-seti-git
         3.  U+F1D3 nf-fa-git
         4.  U+E702 nf-dev-git

note: Found obsolete icon U+FD03
   ┌─ ./pkg/gui/presentation/icons/git_icons.go:28:48
   │
28 │     {domain: "dev.azure.com", icon: "\ufd03"}, // ﴃ
   │                                                   ^ Icon 'nf-mdi-azure' is marked as obsolete
   │
   = You could replace it with:
         1.  U+EBD8 nf-cod-azure
         2. 󰎎 U+F038E nf-md-nature
         3. 󰟋 U+F07CB nf-md-gesture
         4. 󰔌 U+F050C nf-md-texture
  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
  • Code has been formatted (see here) No, this file is not formatted by gofumpt and formatting creates a diff on the whole file.
  • Tests have been added/updated (see here for the integration test guide) No, I found no tests for these files.
  • Text is internationalised (see here)
  • Docs (specifically docs/Config.md) have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@uloco
Copy link

uloco commented May 11, 2023

Please please merge this soon, I need this so badly :P

@github-actions
Copy link
Contributor

github-actions bot commented May 11, 2023

Uffizzi Ephemeral Environment deployment-24927

☁️ https://app.uffizzi.com/github.com/jesseduffield/lazygit/pull/2604

📄 View Application Logs etc.

What is Uffizzi? Learn more!

@jesseduffield
Copy link
Owner

Thanks for making this. This PR will break icons for anybody on nerd fonts v2 (like myself). We should add a way for the user to specify what version they're on in the config, and we should support both versions.

Let's deprecate the 'showIcons' config value (such that if that's set to true, it'll assume you're using nerd fonts v2) and add a new key called nerdFontsVersion which can be either 2 or 3. nerdFontsVersion can take precedence over showIcons.

Let me know if you need any pointers for that :)

@gotgenes
Copy link
Author

I'm not sure your particular reasons for not upgrading Nerd Fonts, however, in general, I would recommend all users upgrade. Probably the most important breaking change for the global community is that it fixes overwriting glyphs, which negatively impacts some of our users in Asia and other world regions.

If what's holding back a user from upgrading is use of v2 code points in their personal configuration files (e.g., Neovim configuration), nerdfix provides tooling around the translation from Nerd Fonts v2 to v3. It can even be automated.

If what's holding a user back from upgrading is they generated their own patched font using the v2 code points, I expect the user to also be able to patch the font with v3 just as easily, if not more so.

Other projects are approaching this situation by having users either upgrade to v3 or pinning to the prior release of the tool/plugin/library prior to updating backwards incompatible code points.

I'm not in agreement that it is worth the effort to create a (potentially breaking) change in the Lazygit config just to support both Nerd Fonts v2 and v3 in the same version of Lazygit, or any software, at this point.

v3 is a net win for the global community. It's unclear there will ever be a v4 and so I don't think this is a useful as an extensible solution to a what-if scenario. At that point, there may be a new and better solution than Nerd Fonts.

@brneor
Copy link

brneor commented May 11, 2023

Even though I agree that updating NerdFont to v3 and updating software so support it is the "correct" way to do things, there's a caveat.

User @Finii stated in a discussion somewhere (maybe Lunarvim's repo, I couldn't find it) about the instability of Material Icons codepoint changes (something like that). In resume, there's no way to guarantee that there'll be no more breaking changes like this in the future.

I don't know if this information changes the way you're planning things, but I thought it was important to share.

@gotgenes
Copy link
Author

User @Finii stated in a discussion somewhere (maybe Lunarvim's repo, I couldn't find it) about the instability of Material Icons codepoint changes (something like that). In resume, there's no way to guarantee that there'll be no more breaking changes like this in the future.

I don't know if this information changes the way you're planning things, but I thought it was important to share.

I would definitely be in agreement with @jesseduffield's requested approach of supporting multiple versions of Nerd Font simultaneously if I had high confidence that Nerd Fonts expected breaking changes to come more frequently than they have.

As it stands, the last major version release was over five years ago. Without other substantial information, the trend looks like years-long cycles.

Has @ryanoasis published plans for Nerd Font's roadmap and anticipated stability or instability? That would be informative.

@Finii
Copy link

Finii commented May 11, 2023

This PR will break icons for anybody on nerd fonts v2 (like myself).

Well, the new codepoints have already been introcuded with Nerd Fonts v2.3.?, so it will only break if you are still on v2.2.x. Usually people are on v2.3.3. Well, except one person just wrote he/she uses v2.2; for them a switch would be indeed needed.

Sorry for the breaking change, but that has been simmering for several years now, and we have to set this right at some point.

the instability of Material Icons codepoint changes

That is correct, they do not claim codepoint stability, but if you have a look at the past releases (after they adopted the F0000 range) the codepoints do look stable. They just do not write it down as goal.

image
Table from ryanoasis/nerd-fonts#365 (comment)

And then, if you look closely Octicons is marked as 'not stable'. But we also updated them.
Nerd Fonts set something up to somehow guarantee the codepoint stability as best as possible.
That means only dropped icons will ... be dropped, and their spaces are reused by new icons.
We even kept/modernized one Octo Icon that has been dropped (ryanoasis/nerd-fonts#1215).

Without other substantial information, the trend looks like years-long cycles.

Yes, I do not expect a weekly monthly once-a-year MDI update ;-)

@Finii
Copy link

Finii commented May 11, 2023

It's unclear there will ever be a v4

That's correct. We accumulated several (breaking) changes over years for v3 and/but I see no reason to break again.
There is enough un-breaking stuff we can do to keep us entertained the next years :-)

And as I said, upstream might ignore codepoint stability, but we care. Codepoint stability tool at work.

@gotgenes
Copy link
Author

@Finii Thank you, that was really informative!

@jesseduffield With this additional information, do you still want to take an approach with a nerdFontsVersion today? Or, is this perhaps an approach you want to defer to the next time Nerd Fonts makes a change that Lazygit needs to support before-and-after (should this happen)?

@jesseduffield
Copy link
Owner

My rationale for letting the user specify the version is less about preparing for future changes (though I would not be surprised if at some point another breaking change in nerd fonts happened), and more about allowing existing lazygit users to upgrade their lazygit version without having to do anything else.

Upgrading nerd fonts is enough of a hassle that I don't want to inflict it on users who upgrade without realising they're about to break their icons. As an example, here's the process I just followed to update my fonts:

  • check my preferences in iterm to see which font I was using
  • go to https://www.nerdfonts.com
  • go to the downloads page
  • search for 'DejaVu Sans Mono'
  • hit download (file explorer pops up)
  • realise I have no idea where my fonts are stored
  • google where fonts are stored on mac
  • go to that folder
  • realise that folder had barely anything in it
  • cancel the download
  • scroll to bottom where homebrew instructions are
  • attempt to download via homebrew, coming up with the name 'font-deja-vu-sans-mono-nerd-font' which seemed sensible
  • realise I got it wrong, switch to 'font-dejavu-sans-mono-nerd-font'
  • test lazygit, verify the icons now display correctly
  • realise I'm using a different font in my vscode terminal given my icons are broken there
  • check which font it's using: wtf it's actually the same font?
  • restart vscode
  • confirm icons work correctly

That took around 10-15 minutes and it wasn't that painful, but it's enough of a task that I want to allow users to defer doing it until they feel like it, without blocking them from updating lazygit.

To be clear here's what lazygit looks like when nerd fonts v2 is still being used (with this PR's code)
image

So I still want this done in a backwards-compatible way

@Saafo
Copy link
Contributor

Saafo commented May 27, 2023

At least can we make nerd font v3 users be able to use the new icons(really need that!!), like adding a showIconsV3 or nerdFontsVersion option to config.yml?

Copy link
Owner

@jesseduffield jesseduffield left a comment

Choose a reason for hiding this comment

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

Marking as 'changes requested' for my own bookkeeping :)

@bmichotte
Copy link

Hello @jesseduffield, do you have any update to share about this ?

Users with a nerd font 3 really have a bad experience with the characters and it will be nice if something was made :)

(btw, how do you have the "tree lines in your commits view" ? Cant' find the options)

@stefanhaller
Copy link
Collaborator

Hello @jesseduffield, do you have any update to share about this ?

This is not waiting for Jesse, he was very clear about how he wants this done. (And for the record, I agree.) This is waiting for someone to make a PR that introduces the option.

Users with a nerd font 3 really have a bad experience with the characters and it will be nice if something was made :)

This I totally agree with, I'm waiting for this too.

(btw, how do you have the "tree lines in your commits view" ? Cant' find the options)

git:
  log:
    showGraph: always

@bmichotte
Copy link

bmichotte commented Jun 14, 2023

This is not waiting for Jesse, he was very clear about how he wants this done. (And for the record, I agree.) This is waiting for someone to make a PR that introduces the option.

Ok, my bad, I misread the answer

showGraph: always

Thank you !

@gotgenes gotgenes force-pushed the fix-nerd-font-icons branch from 06e5250 to 562f690 Compare June 15, 2023 02:34
@gotgenes
Copy link
Author

I don't have the time to add optionality. I'm closing this to make that explicit. Lazygit users who want Nerd Fonts v3 support are welcome to use the patch linked to by this PR.

@stefanhaller
Copy link
Collaborator

As an alternative to @bmichotte's PR I made one with a slightly different approach: #2731.

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.

Adapt to Nerd Font 3.0
8 participants