mfontanini / presenterm

A markdown terminal slideshow tool
https://mfontanini.github.io/presenterm/
BSD 2-Clause "Simplified" License
1.19k stars 29 forks source link

[Request] Add support for `XDG_CONFIG_HOME` #192

Closed maddawik closed 7 months ago

maddawik commented 7 months ago

With the recent breaking change for config directory locations in 0.6.0, it would be really convenient to add support for the XDG_CONFIG_HOME env var (the is the same approach that lazygit uses for example) - https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html#variables

Also just want to take the opportunity to express my thanks for this project - it is my go-to tool for presentations and it is excellent

mfontanini commented 7 months ago

I think this should be respected already. I'm using the directories crate which claims to respect the XDG spec. Could you double check? I'll update the docs to make that more clear tho.

Also just want to take the opportunity to express my thanks for this project - it is my go-to tool for presentations and it is excellent

Thanks a lot! I'm glad people find it useful :heart:

maddawik commented 7 months ago

Ah so maybe this is more of a bug! I just double-checked and on 0.6.0 it doesn't seem to be respecting the env var I have set - if I pass the config via the -c flag it works, but it still errors out with error: invalid theme name - I'm just assuming since my custom theme is in ~/.config/presenterm/themes/ and it's still looking in ~/Library/Application Support/presenterm/

mfontanini commented 7 months ago

I just created #193 which seems to fix the issue. Could you double check please? Thanks!

maddawik commented 7 months ago

I tried doing a cargo run from the xdg-config-home branch but it looks like I still get the same result - and similarly a invalid theme error when trying cargo run -- -c ~/.config/presenterm/config.yaml

mfontanini commented 7 months ago

Hmm weird... This works for me:

root@acdcf0c7d06b:/tmp/presenterm# ls -R /tmp/xdg
/tmp/xdg:
presenterm

/tmp/xdg/presenterm:
config.yaml
root@acdcf0c7d06b:/tmp/presenterm# XDG_CONFIG_HOME=/tmp/xdg ./target/debug/presenterm examples/demo.md

If I do that, it will load the config file under the path pointed at by the env var and respect it. Do note that -c is unrelated to this; custom themes will still be looked up under your config directory.

maddawik commented 7 months ago

Hmm, let me try the same and share here (and thanks for your patience, I'm not a rustacean). Also should mention I'm using fish shell 3.7.0 in my testing

❯ ls -R /tmp/xdg-test/
presenterm

/tmp/xdg-test/presenterm:
config.yaml

❯ cat /tmp/xdg-test/presenterm/config.yaml
defaults:
  theme: light

bindings:
  exit: ["<c-c>", "q"]

Then after doing cargo build, from the root on xdg-config-home branch

❯ env XDG_CONFIG_HOME=/tmp/xdg-test ./target/debug/presenterm examples/demo.md

With these steps I still find it is not respecting the XDG_CONFIG_HOME env var i.e. not picking the config up (no light theme, can't quit with q), but I don't doubt the fact I could be doing something dumb 🙂 Let me know if there's anything else I can share or something else I can try.

maddawik commented 7 months ago

Another update: wanted to share I also just tried this on the official docker image for rust (realizing that's what you did initially) and I'm not able to reproduce the problem, so maybe there is something platform/shell specific going on (or again, maybe I'm doing something wrong here^) - I'll keep trying to debug

mfontanini commented 7 months ago

I see the problem: the directories crate only follows XDG on linux...

mfontanini commented 7 months ago

I just pushed a fix on the same branch. Can you test it? Now it should be respected in all OSes. Not sure what's the rationale behind only supporting it in linux.

maddawik commented 7 months ago

Huzzah, that worked! Thanks - and sorry I failed to mention way sooner that I'm on MacOS, that would've been helpful info.

mfontanini commented 7 months ago

No worries, I got it from the path in your comment. I just didn't expect this to be platform dependent. Alright great! I'll release 0.6.1 soon to include this fix.

maddawik commented 7 months ago

You're a legend sir, thank you again 🙏🏻