-
Notifications
You must be signed in to change notification settings - Fork 611
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
Config: apply xdg system config files #4845
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for building on @jcollie's work here and keeping the commit credits. I made some notes.
src/config/Config.zig
Outdated
// We must reverse the order of the iterator because the first xdg config | ||
// dir must takes importance per the spec https://specifications.freedesktop.org/basedir-spec/latest/#variables | ||
var xdg_config_dirs = std.ArrayList([]const u8).init(alloc); | ||
defer xdg_config_dirs.deinit(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of all this machinery, the DirIterator
could do a reverse iteration using std.mem.lastIndexOf
to implement your own iterator. I would recommend this route.
Also please note my feedback on @jcollie's PR, it applies here as well. |
bd5bc99
to
266d069
Compare
Re. API design Current API
This APITo get all the xdg dirs in order from least to most important
Single directory access is possible ala
Ditto for system directories
I've just gone ahead and used splitBackwardsScalar instead of splitScalar, I think wrapping an std iterator is better here. Better API
I think splitting XDG/Windows/MacOS known directories logic into their own modules and unifying these into a programdirs enum could make sense something akin to ProjectDirs from this stack overflow post. |
266d069
to
65f3ab2
Compare
fixes: #4506
Applies the config files inside the XDG_CONFIG_DIRS with the first file taking precedence.