Skip to content

feat(cli): default to XDG_CONFIG_HOME if set #745

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michaelBelsanti
Copy link

Uses $XDG_CONFIG_HOME/rustscan.toml if it is set, falls back to ~/.rustscan.toml otherwise.

@michaelBelsanti
Copy link
Author

closes #741

@PsypherPunk
Copy link
Collaborator

I think I'm generally in favour of this (and we can update the wiki to reflect limited support for the XDG spec.) but it'd be good to wrap this in a unit test.

We're currently using nextest which, according to their docs, allows altering "the environment within tests"; however, while that's fine for our CI environment, we can't guarantee that end users won't run cargo test and the resulting effect on their environment variables.

Thoughts?

@PsypherPunk
Copy link
Collaborator

Incidentally, poking around a little more, Go's xdg package "uses Known Folders as defaults" for Windows, FWIW.

@michaelBelsanti
Copy link
Author

Incidentally, poking around a little more, Go's xdg package "uses Known Folders as defaults" for Windows, FWIW.

Thanks, I'll look into using that.

I think I'm generally in favour of this (and we can update the wiki to reflect limited support for the XDG spec.) but it'd be good to wrap this in a unit test.

Can you explain a bit what this would entail? I don't do much coding outside scripting, so not very familiar with unit testing.

@bee-san
Copy link
Owner

bee-san commented Mar 4, 2025

IMO use Dirs https://crates.io/crates/dirs which is made for this :)

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.

3 participants