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

Ensure we normalize to the correct path separator #73

Merged
merged 1 commit into from
Aug 2, 2018
Merged

Ensure we normalize to the correct path separator #73

merged 1 commit into from
Aug 2, 2018

Conversation

alanmcgovern
Copy link
Contributor

git rev-parse --show-toplevel emits unix style separator characters.
This breaks our logic later down the line as we compare two paths
and that causes the comparison to fail.

for example: git rev-parse --show-toplevel produces C:/Projects/whatever
and GitRoot is c:\Projects\whatever

Now both will normalize to c:\Projects\whatever\

git rev-parse --show-toplevel emits unix style separator characters.
This breaks our logic later down the line as we compare two paths
and that causes the comparison to fail.

for example: `git rev-parse --show-toplevel` produces `C:/Projects/whatever`
and `GitRoot` is `c:\Projects\whatever`

Now both will normalize to `c:\Projects\whatever\`
@alanmcgovern
Copy link
Contributor Author

@kzu should be good to go. It worked for me locally

@kzu kzu merged commit 492c5e2 into devlooped:master Aug 2, 2018
@kzu
Copy link
Member

kzu commented Aug 2, 2018

Thanks! 💯

@kzu
Copy link
Member

kzu commented Aug 3, 2018

Sigh, it was NormalizePath, not NormalizeDirectory :(

@alanmcgovern
Copy link
Contributor Author

alanmcgovern commented Aug 6, 2018

Normalise directory exists too - it ensures the paths end in a directory separator char

@kzu
Copy link
Member

kzu commented Aug 6, 2018 via email

@kzu
Copy link
Member

kzu commented Aug 14, 2018

Fixed this in #75

@kzu
Copy link
Member

kzu commented Aug 14, 2018 via email

@devlooped devlooped locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants