Skip to content
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

Fix default hooks config path to conform to XDG #1644

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

milhnl
Copy link
Contributor

@milhnl milhnl commented Jan 30, 2024

Hey!

A very small patch that makes the default value for the hooks file use $XDG_CONFIG_HOME instead of ~/.config. The previous behaviour was non-conforming.

I did not update the documentation because of two reasons:

  • The documentation is currently inconsistent in calling out env variable support. so adding it should probably be in a different commit.
  • the table in docs/source/api/settings.rst would get a bit wider and thus unreadable in my editor.

@pazz
Copy link
Owner

pazz commented Jan 30, 2024

I don't think this is correct..
This hardcoded location is used as fallback after we've already tried to read the suggested location. That is, if the env var is set then we've already read it here.

@milhnl
Copy link
Contributor Author

milhnl commented Jan 30, 2024

The code you point to is finding the main configuration file. That's different from the path to the hooks file. You're correct in that it's only the fallback value.

It's easy to reproduce: copy your $XDG_CONFIG_HOME/alot to e.g. /tmp/config/alot, set XDG_CONFIG_HOME to /tmp/config. If you do not have hooksfile set, alot will try to read the hooks from ~/.config/alot/hooks.py instead of /tmp/config/alot/hooks.py. I observed this behaviour, and the patch fixes it.

Hopes this clears it up!

@pazz
Copy link
Owner

pazz commented Jan 31, 2024

Oh I see. Thanks for the explanation and the fix!

@pazz pazz merged commit 7454b4f into pazz:master Jan 31, 2024
9 checks passed
@milhnl milhnl deleted the fix-hooks-xdg-path branch January 31, 2024 12:35
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