Skip to content

fix: fix Windows paths#246

Merged
MordechaiHadad merged 2 commits into
MordechaiHadad:masterfrom
sid-6581:fix/winpath
Dec 17, 2024
Merged

fix: fix Windows paths#246
MordechaiHadad merged 2 commits into
MordechaiHadad:masterfrom
sid-6581:fix/winpath

Conversation

@sid-6581

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

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

sid-6581 commented Dec 5, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Owner

hmmm i see

Comment thread src/handlers/use_handler.rs Outdated
Comment thread src/handlers/use_handler.rs Outdated
@MordechaiHadad MordechaiHadad merged commit 93c82b6 into MordechaiHadad:master Dec 17, 2024
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