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 abs windows path without volume #28

Merged
merged 2 commits into from
Dec 17, 2019

Conversation

idoru
Copy link
Contributor

@idoru idoru commented Dec 16, 2019

Treat patterns starting with path separators (notably without volume specifiers) on windows as absolute.

This fixes such patterns erroneously being evaluated in the current working dir.

Changes to tests:

  • Get windows tests running correctly by fixing standard pattern check in tests
  • Additionally test for absolute paths without leading volume specifier

Backslashes from calling filePath.FromSlash on windows were causing the
pattern to be treated as standard when it shouldn't be. This in turn
caused incorrect assertions to be made, failing the tests on windows.
In 1.1.5 Glob() on windows matched absolute patterns without volume specified with the current working dir's volume.
This broke in 1.2.x due to the use of filepath.IsAbs() which returns false if the volume is not present.
This was causing paths starting with / or \ on windows to be erroneously evaluated from the current directory.
@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

Merging #28 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #28   +/-   ##
=======================================
  Coverage   79.81%   79.81%           
=======================================
  Files           1        1           
  Lines         327      327           
=======================================
  Hits          261      261           
  Misses         41       41           
  Partials       25       25
Impacted Files Coverage Δ
doublestar.go 79.81% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fcc087...ac358a8. Read the comment docs.

@bmatcuk bmatcuk self-assigned this Dec 17, 2019
@bmatcuk bmatcuk added the bug label Dec 17, 2019
@bmatcuk
Copy link
Owner

bmatcuk commented Dec 17, 2019

Hey @idoru! Thanks for the PR! Looks good to me; I'll merge and cut a new release now. Sorry for the delay; been a busy few days! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants