feat: read credentials.ini from XDG_CONFIG_HOME (or os equivalents) in addition to bin directory

[RobBrazier]

📑 Description

Maybe it's just linux people but I'm not a fan of having configuration files in the same folder as the binary (as I installed in ~/.local/bin)

Each OS has its own 'assigned' configuration directory:

Platform	Value	Example
Linux	$XDG_CONFIG_HOME or $HOME/.config	/home/alice/.config
macOS	$HOME/Library/Application Support	/Users/Alice/Library/Application Support
Windows	{FOLDERID_LocalAppData}	C:\Users\Alice\AppData\Local

Ideally the credentials.ini file would be managed by the cli tool and placed there, rather than the user doing it themselves, however I thought I'd keep the changes small.

I don't have a mac to test this on and haven't on windows yet (just linux)

NOTE: This does introduce a small new dependency dirs, not sure how you'd feel about that - there's probably other ways to do it, but this seemed to be the 'easiest'

✅ Checks

• code: My pull request adheres to the code style of this project
• docs: Added / updated
• tests: All the tests have passed

Note: all 0 tests passed via cargo test

ℹ Additional Information

Note: I've only done a tiny bit of rust, apologies for any noobiness :)

Showing the error with multiple locations

$ cargo run --release
   Compiling discrakt v2.2.3 (/home/rob/Code/github.com/RobBrazier/discrakt)
    Finished `release` profile [optimized] target(s) in 1.52s
     Running `target/release/discrakt`
Could not find credentials.ini in ["/home/rob/.config/discrakt", "/home/rob/Code/github.com/RobBrazier/discrakt/target/release"]
thread 'main' panicked at src/utils.rs:171:28:
Could not find credentials.ini
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Reading credentialas from bin directory (current behaviour)

$ ls target/release/credentials.ini
/home/rob/Code/github.com/RobBrazier/discrakt/target/release/credentials.ini
$ ls ~/.config/discrakt/credentials.ini
ls: cannot access '/home/rob/.config/discrakt/credentials.ini': No such file or directory
$ cargo run --release
    Finished `release` profile [optimized] target(s) in 0.02s
     Running `target/release/discrakt`
2024-08-11T16:14:49Z : One Piece - S4E109 - The Key to a Great Comeback Escape! The Wax-Wax Ball! | 7.64%

Reading credentials from XDG_CONFIG_HOME and equivalents

$ ls target/release/credentials.ini
ls: cannot access 'target/release/credentials.ini': No such file or directory
$ ls ~/.config/discrakt/credentials.ini
/home/rob/.config/discrakt/credentials.ini
$ cargo run --release
    Finished `release` profile [optimized] target(s) in 0.02s
     Running `target/release/discrakt`
2024-08-11T16:18:20Z : One Piece - S4E109 - The Key to a Great Comeback Escape! The Wax-Wax Ball! | 22.29%

It may be worth having additional support specifically for scoop now that I think of it, as that has a dedicated persist folder for config - something like %USERPROFILE%/scoop/persist/discrakt/credentials.ini - I see credentials.ini is configured to persist, not sure how that works with the current relative directory stuff though

[afonsojramos]

"linux people", as you put it, are definitely welcome here 😄 I'll try to give it a try today! Thanks for the PR!

[afonsojramos]

Looking good! And the new dependency seems super useful too!

[afonsojramos]

Suggested change

	let xdg_config_path = dirs::config_local_dir().unwrap().join("discrakt");
	let mut exe_path = env::current_exe().unwrap();
	exe_path.pop();
	let locations = vec![xdg_config_path, exe_path];
	let config_path = dirs::config_local_dir().unwrap().join("discrakt");
	let mut exe_path = env::current_exe().unwrap();
	exe_path.pop();
	let locations = vec![config_path, exe_path];

If dirs::config_local_dir() works for all OSs, probably the best naming is simply config_path?

[afonsojramos]

Isn't it this:

Suggested change

	|Windows|`%LOCALAPPDATA%`\discrakt|C:\Users\Alice\AppData\Local\discrakt\credentials.ini|
	|Windows|`%LOCALAPPDATA%`\discrakt|C:\Users\Alice\AppData\Roaming\discrakt\credentials.ini|

[RobBrazier]

dirs::config_dir() gives you Roaming, but dirs::config_local_dir() gives you Local - not sure what the windows standard is to be honest - I just went with Local
%LOCALAPPDATA% points to Local as well, think it's %APPDATA% for Roaming

[BrutuZ]

Why not go with Roaming instead of Local?
Not only is it easier to type %AppData% to navigate to it, but would also allow the config be backed up or migrated with the profile or even stay automatically synced across different devices in the same domain if the profile is attached to one.

[RobBrazier]

I'll be honest I didn't really know the difference between Local and Roaming! makes more sense to use Roaming then - cheers for mentioning

[afonsojramos]

I assume that the user needs to move the credentials.ini file inside a manually created distrakt folder inside one of the config folders that are now documented in the readme?

Would be nice if it was created automatically on the first run before closing. This way the need to create/download the file manually would disappear! And we could probably open the file explorer in said file's location!

[RobBrazier]

so they're not required to move it into the new discrakt folder - if there is a config dir in the currently documented location it'll use that, but will also check the standard config dir

I was thinking a discrakt config or discrakt init flow might be a nice idea, but thought I'd keep it small and not make any big sweeping changes :D

Could absolutely put the config file in the default location and tell the user where it is and to modify it, maybe even have a flag to specify the location of the config file if they want to override for some reason

I'm just unsure how friendly the UX of telling someone that hadn't necessarily dug into the LOCALAPPDATA (or the macos Library location to go there and start tweaking files

could even maybe go through a setup flow on first launch if the file doesn't exist, to populate the file, put it in the location and then tell the user where it is - that might be the friendlier option

[RobBrazier]

cheers - I've added a few replies, haven't had much time to look at it this week but will make some updates hopefully over the weekend

happy to give the config population a go if you want unless you want to (no hard feelings either way)

I feel like clap may be beneficial to pull in, to allow for flags and/or subcommands to create/update the config, might make life easier, but should be possible without too

like I said in my replies - I'm cautious about making sweeping changes as a first time PR on the project :)