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: fix Windows paths #246

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

sid-6581
Copy link
Contributor

@sid-6581 sid-6581 commented Dec 3, 2024

This PR addresses two issues with Windows paths:

  1. AppData/Local and AppData/Roaming were changed to use backslashes instead of forward slashes.
  2. When checking if the installation directory has already been added to the path, the paths are normalized by changing forward slashes to backslashes as well as doing a case-insensitive comparison to avoid duplicate paths being added with different casing or slashes.

@MordechaiHadad
Copy link
Owner

Is there an actual issue that arises because of the fact that currently it uses forward slashes? I believe I already fixed the issue with duplication inside the $PATH

@sid-6581
Copy link
Contributor Author

sid-6581 commented Dec 5, 2024

It causes problems when using an application or script to normalize and deduplicate the Path environment variable, which unfortunately is necessary because many other applications also don't properly check if the path has already been added. So when doing that normalization before the dedupe, then running bob after, bob will just add the path again with a forward slash even though it's already there with a backslash.

That's the exact scenario I ran into. Also, Windows doesn't really like forward slashes in the Path environment variable. If you try to edit it in the settings and there's a forward slash in the path, it will complain that the forward slash isn't an allowed character and won't let you save it.

@MordechaiHadad
Copy link
Owner

hmmm i see

src/handlers/use_handler.rs Outdated Show resolved Hide resolved
src/handlers/use_handler.rs Outdated Show resolved Hide resolved
@MordechaiHadad MordechaiHadad merged commit 93c82b6 into MordechaiHadad:master Dec 17, 2024
20 checks passed
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