Closed JonasLeonhard closed 5 months ago
Excited for this! I still need to build and try it myself but otherwise looks straightforward to me. My only thought is src/icon/nerdfont.rs
is practically identical to src/icon/vscode.rs
. I wonder if a single src/icon/font.rs
accepting paths would be easier to maintain?
I wonder if a single
src/icon/font.rs
accepting paths would be easier to maintain?
Ping @JonasLeonhard đđź
LMK if I can help get it across the finish line đ
Icons is one of the wide topics I can't really find much time to allocate for.
What this means is I need several voices to tell me at some point that they tested it and it looks ready so that I just have to do a few checks then merge it.
(consider I'm dumb and I may miss subtle signals)
Hey @texastoland.
I refactored the code into a single src/font.rs FontPlugin according to your earlier suggestion. In the process, I've renamed the existing VsCodeIconPlugin to FontPlugin and changed the "new" function to accept !include references to icon mappings.
Please make sure to test both the nerdfont setting, aswell as the vscode font setting!
Let me know if my changes are suboptimal *tipofthehat
LGTM and nerd fonts still working locally đ¨đźâđ Someone using the vscode
icons should double check it. @Canop ready for your eyes I think!
It seems to work.
Here's a comparison, with nerdfont at left and vscode at right:
Interesting that Cargo.toml appears with the rust icon.
Can you please update the documentation at https://dystroy.org/broot/icons/ (this is the file website/docs/icons.md
, and you can check the site with mkdocs serve
, more info if needed on https://miaou.dystroy.org/3490) ?
@Canop. I added some docs for the nerdfont setting.
Merged. There should be a release in a few days, maybe one week.
Thanks to all who helped.
This was a lot of work for a long pending request and obviously well done but, and I hope I don't sound negative but this has a lot of duplication and frankly feels a bit sloppy.
1) FAQ is duplicated. FAQ could have had a font specific section but duplication should have been removed.
2) wherever (and if) possible, everything except icon_name_to_icon_code_point.rs should have been a common file, unless work already exists
3) https://github.com/JonasLeonhard/broot/blob/c2d2dbb7be2021531f493053ab869f0fd1c1fdb0/resources/icons/nerdfont/data/double_extension_to_icon_name_map.rs#L1 (and all other files), please fix the reference to README. It should be ../README.md
Sorry I was negative but in a big project it's important to avoid duplication and reuse as much as possible. Right now it feels like a javascript/typescript/angular developer scratched his personal itch and left the result for everybody to maintain ...
Thanks @asdf8dfafjk I must confess that I didn't look much into the icons feature, from the start. Action plan, PRs, welcome.
It is my fault all along, I didn't prepare the blog post you asked me to ... (you've been nothing but really kind and supportive, and I hope that at least some people find the feature useful ...)
I'll come back with a plan outline soon ...
@Canop As a first step, I created a rough wiki entry with the code that I had used here: https://github.com/Canop/broot/wiki/Icons ,please let me know if you need this "more finished" ...
It is my fault all along, I didn't prepare the blog post you asked me to
Don't worry. Assuming it's subpar (I didn't look enough), it's still an isolated feature with only low chances to degrade the rest. We're here to become better and build beautiful things, let's just improve that and not care about whose fault it was, and there's probably no real urgency. A new issue dedicated to the component's quality would be better suited to track it.
(I'm about to deconnect for a week, don't worry if I don't answer more for a while)
@Canop Enjoy your vacation! I'll post a few things here in the meanwhile, I'm myself only here sporadically but I think you know how to reach me if you need to ;-)
Some doc cleanup here: https://github.com/Canop/broot/pull/871/
- wherever (and if) possible, everything except
icon_name_to_icon_code_point.rs
should have been a common file, unless work already exists
I did the code review. May I know what you meant here?
@texastoland
basically there are 2 classes of mappings, first is the detection of file type and another is file type to icon point.
extension/double extension/filename to filetype should be common across all ideally.
file type to icon point will changed based on the "font file".
Good news everyone.
The first version of the "icon_theme: nerdfont" settings is now available for testing. I added a "icon_theme: nerdfont" setting to the broot conf.hjson file.
I would love for someone to test this branch before i create a pull request!
Issue
https://github.com/Canop/broot/issues/333
Installation
Branch: https://github.com/JonasLeonhard/broot/tree/feature/nerdfont
In order for the nerdfont setting to work you need to have a patched font installed. See: https://github.com/ryanoasis/nerd-fonts
After a successful installation, you need to add "icon_theme: nerdfont" to your broot conf.hjson file:
How it works:
This commit adds a new NerdfontIconPlugin to icon/mod.rs. The structure of the plugin is based on the existing vscode icon plugin. It maps incoming filenames based on mappings inside of icons/nerdfont/data/*_map.rs files.
Limitations:
The iconset is limited by available nerdfont icons. Some icons simply don't exist as of yet. Eg. a vite or prettier icon. You can find available icons here: https://www.nerdfonts.com/cheat-sheet.
How to do i test this?
I found a file that is not mapped or wrongly mapped!
Lets say you find some file icon you think i forgot or mapped wrong. Please go to https://www.nerdfonts.com/cheat-sheet and search for an icon you would like to set in its place.
In order to correctly fix the icon mapping i need: and .
Lets say you think the .json icon is incorrect.
Credits:
Thank you for putting me into the right direction:
Screenshots