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 #221 flattened_basedirs on Windows by ; instead of : #307

Merged
merged 3 commits into from
Feb 9, 2019

Conversation

fliiiix
Copy link
Member

@fliiiix fliiiix commented Feb 8, 2019

Splitting paths on Windows by : is not
a good idea. Since it looks like this
C:\something. That is why we disable
this feature on Windows.

Signed-off-by: fliiiix hi@l33t.name

Splitting paths on Windows by : is not
a good idea. Since it looks like this
C:\\something. That is why we disable
this feature on Windows.

Signed-off-by: fliiiix <hi@l33t.name>
@fliiiix
Copy link
Member Author

fliiiix commented Feb 8, 2019

@7ep let me know if this works for you as well.

@codecov-io
Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #307 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #307      +/-   ##
==========================================
+ Coverage   89.46%   89.46%   +<.01%     
==========================================
  Files          89       89              
  Lines        2686     2687       +1     
==========================================
+ Hits         2403     2404       +1     
  Misses        283      283
Impacted Files Coverage Δ
radish/utils.py 77.38% <100%> (+0.27%) ⬆️

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 ab7e7a6...b40c523. Read the comment docs.

Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like Windows uses a semi-colon to separate different paths in it's PATH variable.
Should we do the same?
I think we'd only need to change flattened_basedir for that and choose the appropriate separator depending on the OS. What do you think?

docs/commandline.rst Outdated Show resolved Hide resolved
@fliiiix
Copy link
Member Author

fliiiix commented Feb 9, 2019

Should we do the same?

It sounds like the right thing to do. So why not.

@fliiiix fliiiix changed the title Fix #221 Don't flattened_basedirs on Windows Fix #221 flattened_basedirs on Windows by ; instead of : Feb 9, 2019
@timofurrer timofurrer merged commit 2723dbf into master Feb 9, 2019
@fliiiix fliiiix deleted the bugfix/windows_basepaths branch February 17, 2019 10:52
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.

3 participants