Skip to content

make XDG_XXX_HOME work for Windows #1650

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

Closed
wants to merge 4 commits into from
Closed

Conversation

pm100
Copy link
Contributor

@pm100 pm100 commented Apr 12, 2023

This Pull Request fixes/closes #1498.

  • If XDG_CONFIG_HOME is set on Windows then use it instead of what dirs_next says
  • if XDG_CACHE_HOME is set on Windows then use it instead of what dirs_next says

Note that dirs_next already honors XDG_XXX_HOME on Linux

It is arguable that not doing this for Mac is odd, but that not whats asked for in the bug.

I followed the checklist:

  • I added unittests
  • [ x] I ran make check without errors
  • [ x] I tested the overall application
  • I added an appropriate item to the changelog

@extrawurst
Copy link
Collaborator

extrawurst commented Apr 13, 2023

Note that dirs_next already honors XDG_XXX_HOME on Linux

It is arguable that not doing this for Mac is odd, but that not whats asked for in the bug.

Can we try and be good citizens and file this upstream and try making it a PR there?

@pm100
Copy link
Contributor Author

pm100 commented Apr 13, 2023

I am not sure what you are saying.

Are you responding to my Macos comment, do you want me to add the same feature for Mac. In this PR? Or do you want a new issue for Mac to support XDG.
Or did I do something wrong with this PR in terms of how it intersects with branches etc (I see the word 'upstream', which I am just now understanding). I am very limited in my git/github/PR experience and could have made a mistake.

@extrawurst
Copy link
Collaborator

I m saying if you think it should be a Feature of dirs_next let’s try to add it as a PR over there (upstream) first before, WDYT?

@pm100
Copy link
Contributor Author

pm100 commented Apr 15, 2023

@extrawurst OK I understand.

No I dont thing that dirs_next should do it. Its a linux thing. Do what they do is fine.

Note that git explicitly describes the support of XDG, without qualifying it by platform. https://git-scm.com/docs/git-config#FILES. So we should too

I have verified that libgit2 obeys XDG cross platform. So in fact we do honor it. The code change I made makes it work for 2 things that we implement without libgit2, log file location and keyboard map location. And I do think we should do that for Mac too

However I am now surprised about the original issue. Its a blanket statement about not supporting XDG on windows (but with no mention of those 2 items, log and keyboard). I have asked the original poster to clarify, because maybe there is something else going on.

I am going to close this PR while awaiting the response. And I will add Mac support too

@pm100 pm100 closed this Apr 15, 2023
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.

Follow XDG spec on windows
2 participants