Skip to content

Comments

Use default key config and theme when values are missing#735

Closed
hpcsc wants to merge 3 commits intogitui-org:masterfrom
hpcsc:default-key-config-and-theme
Closed

Use default key config and theme when values are missing#735
hpcsc wants to merge 3 commits intogitui-org:masterfrom
hpcsc:default-key-config-and-theme

Conversation

@hpcsc
Copy link

@hpcsc hpcsc commented May 24, 2021

Background

  • I have gitui key and theme config files versioned in my dotfiles repository. Whenever I update gitui, there are a few new keys that are supported by gitui and not in my config files. gitui refuses to load my config files and creates new default ones instead. This is very inconvenient since I need to compare my config files and default config files to see what values are missing to update

What is in this PR?

  • Use serde(default = "path") for fields in key config and theme to specify default values if they are missing from config files during deserialization. This does create a bunch of private functions (default_xxx()) but this is the only way that I can see from serde documentation

Copy link
Collaborator

@extrawurst extrawurst left a comment

Choose a reason for hiding this comment

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

ok I checked this out and I like that we try to fix this issue of breaking the configs every time we add stuff. but the files get bloated a lot using this approach where we need a separate method for each field. I wonder if we can find a more elegant solution using custom serialise: https://serde.rs/deserialize-struct.html

in a perfect world we can detect a missing field and only pick that off the Default::default and keep this as is.

let me know if you can find a solution like this (I did not try it myself yet), for the time being I mark this PR as draft again.

rc::Rc,
};

use anyhow::Result;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this reordering good for?

rc::Rc,
};

use anyhow::Result;
Copy link
Collaborator

Choose a reason for hiding this comment

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

needless reorders again, this confuses the review

@extrawurst extrawurst marked this pull request as draft May 27, 2021 19:26
@extrawurst
Copy link
Collaborator

Superseded by optional overwrites in key config

@extrawurst extrawurst closed this Jan 30, 2022
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