support for theme.yml file #840

PThorpe92 · 2024-02-14T17:00:31Z

This is the base of what we will need for a config.yml, which will build on top of this. This includes the changes from #744 as it was necessary in order to serialize the Style struct.

hasecilu · 2024-02-17T18:28:04Z

Hello, is it planned that config.yml file takes care of custom icons? As of now if a user requieres to add a new icon or change one that conflicts with an existing one it needs to compile from source, to get latest changes need to be rebased from time to time, so having overrides via config file could be awesome.

PThorpe92 · 2024-02-17T20:16:55Z

Hello, is it planned that config.yml file takes care of custom icons? As of now if a user requieres to add a new icon or change one that conflicts with an existing one it needs to compile from source, to get latest changes need to be rebased from time to time, so having overrides via config file could be awesome.

Yes! that is the plan, and I even have a local branch with progress made on it. As it happens, there are lots of significant changes happening in the codebase right now (clap, major dependency changes), so I didn't want to push it and try to do too much at once.
But custom icons is actually the first feature that the config file will support, and will be included in the PR introducing it 👍

That has been a very common request, so it's on top of my list

cafkafk

There seems to be a failing test (sorry for noise if you're still working on this, just saw review was requested so didn't wanna let it hang in case!)

MartinFillon

Could you clarify some of my concerns please ?

src/options/flags.rs

src/options/config.rs

MartinFillon · 2024-03-27T22:16:34Z

I was wondering how you felt about, as we have the env var, the default value, adding also a command line argument such as --theme or --config as this is kinda a standard ?

PThorpe92 · 2024-03-27T22:47:25Z

note: i have more local changes now so further review can wait till prob tomorrow

I am certainly open to the command line arg. My thought was that XDG_CONFIG_HOME was pretty standard, with the option of giving an env var for fallback if someone wanted to go somewhere else.

I would really like to get clap merged because it seems like this arg parser is just cursed lately 💀

MartinFillon · 2024-03-28T10:38:25Z

I would really like to get clap merged because it seems like this arg parser is just cursed lately 💀

same especially since #832 but we waiting on @cafkafk

gierens · 2024-03-30T23:59:32Z

The commit I added fixes the nocolor/color=never option.

The gist of it is, that when UseColour never uses NoFileStyle and UiStyles::plain, and during unwrapping in Theme.colour_file the NoFileStyle.get_style returns None which makes the colour_file fall back to the UiStyles. Since UiStyles::plain simply calls default the default colors remain.

By actually setting all colors to the default style in UiStyles::plain this is mitigated. And since it is only called in this case, this doesn't break anything either.

gierens · 2024-03-31T10:21:31Z

@PThorpe92 Since we talked about icon overrides yesterday, I couldn't help but play around with this awesome PR and thus also tried porting my very simplistic idea from the #914 PR which addresses #909 😇

So this basically adds an optional section icon_overrides to the theme.yml:

Which is translated to two HashMap<String, char> that override the internal icons when applicable:

This has the advantage that you don't need to redesign the icon handling in any way and that users can add icons for any extension or filename they like without them needing to be present in the internal representation.

If anyone is worried about performance, I tested this via giving all files in /usr/lib a different icon to get a big-ass hashmap and this doesn't seem to change the runtime over 100 repetitions in any noticeable way.

Anyway, I hope it's fine I just pushed this here, let me now what you think of the idea. If you want to implement this in a different way you can obviously remove my commits 🙂

MartinFillon

Well lgtm, if you dont wanna wait for clap that would be understandable.

PThorpe92 · 2024-03-31T11:32:17Z

@gierens

fuckin right that is awesome, I really cannot believe I didn't think of that 😅, my attempt was too far decoupled from the actual theme, and involved all the icons in a hashmap instead of just the overridden ones. (which then required either complete iteration or cloning to merge with the overridden ones).

Icon overrides are easily the most requested feature so it's great to be able to offer that as part of this 🚀 good stuff

gierens · 2024-03-31T12:01:04Z

@PThorpe92 This is totally understandable that one would try it this way when working with the themes.

I had the advantage of having implemented this icon color PR which gave me a good understanding of the involved code. The key idea with the overrides I got from an old exa issue that just popped up again the other day. Also, I saw this static compile-time map stuff in the icons module and thought, I do not want to meddle with that 😆

I'm really glad we can make it part of the config file 🚀 rather than some additional environment variable 🙂

hasecilu · 2024-04-06T21:26:19Z

Hi, I can't provide a technical review but I tested this branch, the config file provides a very extensive list of options which is awesome because enable users to tweak icons without hardcoding them, very handy. I understand that a lot of extensions on my icon PR are very niche so this is very helpful.

Just 2 more notes:

It could be possible to specify the color of the icons per extension? An example of this behaviour is found on https://github.com/nvim-tree/nvim-web-devicons
It could be possible override the icons in a group basis? As in impl Icons {}; For example changing on const GIT: char = '\u{f1d3}'; //   -> 󰊢

Very nice job, thanks!

Example on LazyVim that uses Neotree that uses nvim-web-devicons

PThorpe92 · 2024-04-07T01:42:39Z

I think @gierens had an implementation of changing the icon colors in a previous PR if I'm not mistaken?

gierens · 2024-04-07T07:45:52Z

Point 1 (extension-based icon color overrides):
Yeah, PR #903 was in reaction to discussion #891 which basically asked for the ability to set the color of the icon column more or less. So this PR sets all icons to the same color.

While that differs from what is asked here, I think overriding the icon colors of individual extensions or filenames should be fairly straight forward from what I've already added here. The only thing I'm a little worried about is how adding a color string could affect the hashmap performance. I can take a look at it later and experiment around a bit.

Point 2 (group based icon overrides):
This one is little tricky I think for multiple reasons.

First of all this would need a more extesive design as to what the behaviour actually should be, how this should mix with the extension overrides and what takes priority ...

Second, an implementation might have to touch the exact part of the code I was trying to avoid meddling with here: impl Icons defines a bunch of constants that are then used in the DIRECTORY/FILENAME/EXTENSION_ICONS which are phf_maps meaning they are built at compile time.

Third, I'm not sure how much value this adds for the user in comparison to the complexity it might introduce. The current proposal here is imho quite elegant because it is extension/filename-based and thus completely circumvents the maps in the icon module which also makes it quite intuitive because the user has fine-grained control and does not need to know about how eza internally maps Icons to DIRECTORY/FILENAME/EXTENSION_ICONS. Sure, some stuff is left on the table at the moment like individualizing the directory icons but I think this is a good trade-off for the time being.

TLDR:
I think the extension-based icon color overrides is fairly doable and I'm gonna give that a shot later, but the group-based icon overrides introduces too much complexity and is thus out of the scope of this already large PR.

hasecilu · 2024-04-08T01:31:16Z

I think the extension-based icon color overrides is fairly doable and I'm gonna give that a shot later, but the group-based icon overrides introduces too much complexity and is thus out of the scope of this already large PR.

Thanks for the explanation, and sure for point 2, the worst case would be that user needs to repeat a few times the same glyph, is not a big deal, it's preferable that both the code and the config file remains simple.

gierens · 2024-04-08T19:32:24Z

@hasecilu I added a style field to icon overrides in the config file:

@PThorpe92 I first just wanted to add a color to icons but then thought, why not the whole Style. Then I got annoyed by specifying all Style fields in the config file and one thing lead to another ... Long story short, I wrote an "override" layer in the config module for the entire UiStyles struct. Meaning now every field in the config file is optional and users only have to specify the fields they wanna change and not this whole 500 plus line bloat.

This works by defining a mirror for every struct in and including UiStyles, starting with UiStylesOverride down to StyleOverride, and then defining a custom trait FromOverride which transforms those back to the normal ones basically overriding the defaults where requested. So ThemeConfig.to_theme deserializes into UiStylesOverride and this is then turned into the usual UiStyles.

While this bloats the config module a little, I think we rather wanna have bloat there than in the config file itself.

hasecilu · 2024-04-08T22:56:08Z

@hasecilu I added a style field to icon overrides in the config file:

Looks good, testing was ok

PThorpe92 · 2024-04-11T19:08:05Z

@cafkafk this should be all set

hasecilu · 2024-04-11T22:44:58Z

Hi again, I was changing some colors when I noticed that having a good config file resets the foreground color of the filename given by its type (document, video, image, source code, etc). When something fails (I used "Reda" as color) filename fg color is preserved, is this the desired behavior or fg color should be maintained?

EDIT: Tested with files that were already on eza and seems to work as expected with all files except "source code" files, my testing branch

this branch

my branch

gierens · 2024-06-26T20:00:02Z

As discussed in the chat, I adjusted the codeowners to add @PThorpe92 and me to the files that were created or very heavily modified by this PR.

…mplementation

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

This basically ports the PR eza-community#914 and thus also resolves eza-community#909 Signed-off-by: Sandro-Alessio Gierens <[email protected]>

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

cafkafk

This is super nice, heroic effort here, specially rebasing all this... Guess we have theme configuration now!

cafkafk · 2024-08-30T13:28:48Z

Cargo.toml

 ansi-width = "0.1.0"
-
+serde = { version = "1.0.193", features = ["derive"] }
+serde_yaml = "0.9.29"


This was actually deprecated, I have a fork, but let's not switch to that rn, we can do that in another PR if we want (https://crates.io/crates/serde_norway)

cafkafk · 2024-08-30T13:31:29Z

src/options/config.rs

+#[rustfmt::skip]
+#[derive(Clone, Eq, Copy, Debug, PartialEq, Serialize, Deserialize)]
+pub struct FileKindsOverride {
+    pub normal: Option<StyleOverride>,        // fi
+    pub directory: Option<StyleOverride>,     // di
+    pub symlink: Option<StyleOverride>,       // ln
+    pub pipe: Option<StyleOverride>,          // pi
+    pub block_device: Option<StyleOverride>,  // bd
+    pub char_device: Option<StyleOverride>,   // cd
+    pub socket: Option<StyleOverride>,        // so
+    pub special: Option<StyleOverride>,       // sp
+    pub executable: Option<StyleOverride>,    // ex
+    pub mount_point: Option<StyleOverride>,   // mp
+}


For stuff like this, it probably makes more sense to format it like this to just avoid the headache with rustfmt

#[derive(Clone, Eq, Copy, Debug, PartialEq, Serialize, Deserialize)] pub struct FileKindsOverride { // fi pub normal: Option<StyleOverride>, // di pub directory: Option<StyleOverride>, ...

That said, I don't think it's important enough to block this PR

cafkafk · 2024-08-30T13:33:08Z

src/options/flags.rs

@@ -1,3 +1,4 @@
+#![cfg_attr(rustfmt, rustfmt_skip)]


I don't think we want to keep this around for long, but it's okay for now

cafkafk · 2024-08-30T13:34:34Z

src/options/vars.rs

@@ -148,6 +148,5 @@ impl MockVars {
            "NO_COLOR" => self.no_colors = value.clone(),
            _ => (),
        };
-        ()


Wonder why this was even here in the first place when the function doesn't return anything 🤷‍♀️

cafkafk · 2024-08-30T13:40:25Z

src/theme/ui_styles.rs

+    pub extensions: Option<HashMap<String, FileNameStyle>>,
+}
+// Macro to generate .unwrap_or_default getters for each field to cut down boilerplate
+macro_rules! field_accessors {


PThorpe92 requested review from MartinFillon and cafkafk as code owners February 14, 2024 17:00

PThorpe92 force-pushed the config_file branch from 379fb26 to 0cbcf8f Compare February 14, 2024 17:02

cafkafk previously requested changes Mar 3, 2024

View reviewed changes

This was referenced Mar 16, 2024

Config file? #897

Open

Migration from ansiterm to nu_ansi_term #898

Closed

PThorpe92 force-pushed the config_file branch from 6ea17dd to c194d5e Compare March 27, 2024 16:05

MartinFillon reviewed Mar 27, 2024

View reviewed changes

src/options/flags.rs Outdated Show resolved Hide resolved

src/options/config.rs Outdated Show resolved Hide resolved

MartinFillon previously approved these changes Mar 31, 2024

View reviewed changes

This was referenced Mar 31, 2024

Icon Overrides via Environment #914

Closed

feat: LS_ICONS for icon overrides #909

Closed

gierens dismissed MartinFillon’s stale review via cda9f1d April 8, 2024 19:21

gierens force-pushed the config_file branch from cda9f1d to d563362 Compare April 8, 2024 19:23

gierens self-requested a review June 26, 2024 06:52

gierens previously approved these changes Jun 26, 2024

View reviewed changes

MartinFillon previously approved these changes Jun 26, 2024

View reviewed changes

gierens dismissed stale reviews from MartinFillon and themself via 81ed3c4 June 26, 2024 19:58

PThorpe92 mentioned this pull request Jul 14, 2024

feat: Choice of icon #1055

Closed

cafkafk linked an issue Jul 15, 2024 that may be closed by this pull request

feat: Choice of icon #1055

Closed

PThorpe92 and others added 18 commits August 29, 2024 22:47

feat: begin implementation of config file

da6f59c

feat: allow writing default theme.yml file for eventual config file i…

03653e1

…mplementation

feat: theme file configuration base

729d9a3

fix: adjust grid details for CI tests

545f2da

fix: imports and merge conflicts

2790c10

fix: rustfmt issues

bc12e11

fix: clippy issues

236a6cb

fix(theme): revise UiStyles::plain to have no style at all

8ac052c

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

fix: pr reviews fixes for theme file

4120868

feat(theme): add IconOverrides struct and UiStyles.icon_overrides

d77116f

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

feat(output): add icon_override function to FileNameColours trait

ffd8f4b

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

feat(theme): implement FileNameColours.icon_override for Theme

46fd6f2

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

feat(output): handle icon overrides in FileName.paint

8ca6c83

This basically ports the PR eza-community#914 and thus also resolves eza-community#909 Signed-off-by: Sandro-Alessio Gierens <[email protected]>

feat(themes): add example config for icon_overrides

d34a69a

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

feat: rename UiStyles.icon_override to icons and add Style field

cdd4538

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

refactor(config): make every setting optional with override layer

402b0b8

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

refactor(themes): simplify sample theme.yml

88008a0

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

style(output): fix clippy issue in FileName.paint

a1521d7

Signed-off-by: Sandro-Alessio Gierens <[email protected]>

PThorpe92 force-pushed the config_file branch from 81ed3c4 to a1521d7 Compare August 30, 2024 12:59

cafkafk approved these changes Aug 30, 2024

View reviewed changes

cafkafk merged commit 17982a8 into eza-community:main Aug 30, 2024
19 checks passed

cafkafk mentioned this pull request Aug 30, 2024

ci: allow MPL-2.0 #1107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support for theme.yml file #840

support for theme.yml file #840

PThorpe92 commented Feb 14, 2024

hasecilu commented Feb 17, 2024

PThorpe92 commented Feb 17, 2024

cafkafk left a comment

MartinFillon left a comment

MartinFillon commented Mar 27, 2024

PThorpe92 commented Mar 27, 2024

MartinFillon commented Mar 28, 2024

gierens commented Mar 30, 2024

gierens commented Mar 31, 2024 •

edited

Loading

MartinFillon left a comment

PThorpe92 commented Mar 31, 2024

gierens commented Mar 31, 2024 •

edited

Loading

hasecilu commented Apr 6, 2024 •

edited

Loading

PThorpe92 commented Apr 7, 2024

gierens commented Apr 7, 2024 •

edited

Loading

hasecilu commented Apr 8, 2024

gierens commented Apr 8, 2024 •

edited

Loading

hasecilu commented Apr 8, 2024

PThorpe92 commented Apr 11, 2024

hasecilu commented Apr 11, 2024 •

edited

Loading

gierens commented Jun 26, 2024

cafkafk left a comment

cafkafk Aug 30, 2024

cafkafk Aug 30, 2024

cafkafk Aug 30, 2024

cafkafk Aug 30, 2024

cafkafk Aug 30, 2024

support for theme.yml file #840

support for theme.yml file #840

Conversation

PThorpe92 commented Feb 14, 2024

hasecilu commented Feb 17, 2024

PThorpe92 commented Feb 17, 2024

cafkafk left a comment

Choose a reason for hiding this comment

MartinFillon left a comment

Choose a reason for hiding this comment

MartinFillon commented Mar 27, 2024

PThorpe92 commented Mar 27, 2024

MartinFillon commented Mar 28, 2024

gierens commented Mar 30, 2024

gierens commented Mar 31, 2024 • edited Loading

MartinFillon left a comment

Choose a reason for hiding this comment

PThorpe92 commented Mar 31, 2024

gierens commented Mar 31, 2024 • edited Loading

hasecilu commented Apr 6, 2024 • edited Loading

PThorpe92 commented Apr 7, 2024

gierens commented Apr 7, 2024 • edited Loading

hasecilu commented Apr 8, 2024

gierens commented Apr 8, 2024 • edited Loading

hasecilu commented Apr 8, 2024

PThorpe92 commented Apr 11, 2024

hasecilu commented Apr 11, 2024 • edited Loading

this branch

my branch

gierens commented Jun 26, 2024

cafkafk left a comment

Choose a reason for hiding this comment

cafkafk Aug 30, 2024

Choose a reason for hiding this comment

cafkafk Aug 30, 2024

Choose a reason for hiding this comment

cafkafk Aug 30, 2024

Choose a reason for hiding this comment

cafkafk Aug 30, 2024

Choose a reason for hiding this comment

cafkafk Aug 30, 2024

Choose a reason for hiding this comment

gierens commented Mar 31, 2024 •

edited

Loading

gierens commented Mar 31, 2024 •

edited

Loading

hasecilu commented Apr 6, 2024 •

edited

Loading

gierens commented Apr 7, 2024 •

edited

Loading

gierens commented Apr 8, 2024 •

edited

Loading

hasecilu commented Apr 11, 2024 •

edited

Loading