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

use dirs::home instead of expanduser #5

Merged
merged 2 commits into from
Feb 5, 2025
Merged

use dirs::home instead of expanduser #5

merged 2 commits into from
Feb 5, 2025

Conversation

x807x
Copy link
Contributor

@x807x x807x commented Feb 4, 2025

fix #4 by replacing expanduser

@qtfkwk qtfkwk changed the title use dirs::home instead of extanduser use dirs::home instead of expanduser Feb 4, 2025
@qtfkwk
Copy link
Owner

qtfkwk commented Feb 4, 2025

s/extanduser/expanduser/

Good find! Asserting it is working for you in Windows with this change?

Please allow me to review and test it, but I've no general objections to merging it.

@x807x
Copy link
Contributor Author

x807x commented Feb 4, 2025

Sure! It will work on my Windows laptop. Feel free to give it a try.

@qtfkwk
Copy link
Owner

qtfkwk commented Feb 4, 2025

Have an idea that will achieve your ask with fewer changes to the code and avoids repetition of janky API.

Basically it'd be to add a function called expanduser that uses your dirs::home approach. This way DRY (don't repeat yourself) and the calls to expanduser can stay.

Feel free to modify this PR and I'll merge it... or I'll apply your idea next chance I get.

I appreciate your work on this!

@x807x
Copy link
Contributor Author

x807x commented Feb 4, 2025

Like this?

fn extenduser(path: &str) -> PathBuf {
    if path == "~" {
        home_dir().unwrap().join(&path[1..])
    }
    else if path.starts_with("~") {
        home_dir().unwrap().join(&path[2..])
    }
    else {
        PathBuf::from(&path)
    }
}

@x807x
Copy link
Contributor Author

x807x commented Feb 5, 2025

Thank you for helping this🎉

@qtfkwk
Copy link
Owner

qtfkwk commented Feb 5, 2025

Looks pretty good. There's a few other minor optimizations I'll add. Will merge this, fix those, and roll a new release shortly. Thanks!

@qtfkwk qtfkwk merged commit 2d6a421 into qtfkwk:main Feb 5, 2025
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.

Dependency Issue with expanduser
2 participants