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(config): add Windows home dir path #88

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

Acamol
Copy link
Contributor

@Acamol Acamol commented Jan 9, 2024

On PowerShell / CMD the HOME environment variable is often not defined, so $HOME/.config/qbt will evaluate to C:/.config/qbt instead of C:/<username>/.config/qbt as one would expect.

@ludviglundgren ludviglundgren changed the title fix: Windows home dir path fix(config): add Windows home dir path Jan 23, 2024
Copy link
Owner

@ludviglundgren ludviglundgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Needs a change to it does not break linux compatibility.

internal/config/config.go Outdated Show resolved Hide resolved
On PowerShell / CMD the HOME environment variable is often not defined, so $HOME/.config/qbt will evaluate to C:/.config/qbt instead of C:/<username>/.config/qbt as one would expect.
@Acamol Acamol force-pushed the fix-windows-config branch from 3039703 to bc5431d Compare February 22, 2024 00:47
@ludviglundgren
Copy link
Owner

Hey @Acamol , sorry for this taking so long to get back to. I made a change to use the built in filepath.join to build the path which should make it use the correct separator for the environment. On Windows that's backslash \.

Could you try this out and report back and if it's ok? Then we can merge this and get a new release out.

@Acamol
Copy link
Contributor Author

Acamol commented May 10, 2024

It seems to be working fine on my system with your change.

@ludviglundgren
Copy link
Owner

Sorry about the delay, will get this merged and some other PRs and then make a new release 🥳

@ludviglundgren ludviglundgren merged commit 81a4c06 into ludviglundgren:master Aug 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants