Skip to content

Add support for windows paths #97

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

Merged
merged 2 commits into from
Feb 12, 2019
Merged

Add support for windows paths #97

merged 2 commits into from
Feb 12, 2019

Conversation

eliadh
Copy link

@eliadh eliadh commented Feb 10, 2019

Hi.
Added support for using backslash plugin from within Windows, which doesn't have unified root but multiple drives instead.

Thanks!

@vmalloc
Copy link
Member

vmalloc commented Feb 10, 2019

@eliadh the build is failing on Travis...

@eliadh
Copy link
Author

eliadh commented Feb 10, 2019

@eliadh the build is failing on Travis...

OK, I'll check it out. thanks.

@eliadh
Copy link
Author

eliadh commented Feb 10, 2019

@vmalloc: well, it appears that the fixture for testing the normalized path does not account for Windows file separators which are backslash instead of the usual slash. since the assert is on the exact string, I believe that the unit test may be updated to support windows as well.

BTW, this test is the only test that failed (on the local run at my desktop)

test name: def test_normalize_file_path
fixture: file_path_and_expected_path
expected value: 'c/d/filename.py'
windows expected value c\d\filename.py

@eliadh
Copy link
Author

eliadh commented Feb 10, 2019

@vmalloc : Updated. please review. thanks!

@vmalloc
Copy link
Member

vmalloc commented Feb 12, 2019

@eliadh Travis job is still failing...

@eliadh
Copy link
Author

eliadh commented Feb 12, 2019

@eliadh Travis job is still failing...

@vmalloc: the Travis log indicates that the tests on python 3.5 actually passed and python 2.7, 3.6 failed on pytest_cov plugin load failure. what am I missing?

@vmalloc
Copy link
Member

vmalloc commented Feb 12, 2019

I will make sure develop passes and ping you to rebase the branch.
In the mean time - did you make sure the updated code works properly on Windows?

@vmalloc
Copy link
Member

vmalloc commented Feb 12, 2019

@eliadh develop fixed. You can rebase on top of it now and push again

@eliadh
Copy link
Author

eliadh commented Feb 12, 2019

I will make sure develop passes and ping you to rebase the branch.
In the mean time - did you make sure the updated code works properly on Windows?

Sure. except that the relevant unit test itself doesn't consider windows paths (backslash instead of slash), however, it will pass on Linux.

@vmalloc
Copy link
Member

vmalloc commented Feb 12, 2019

I meant did you try it with the fix on Windows and saw that it works correctly?

@eliadh
Copy link
Author

eliadh commented Feb 12, 2019

I meant did you try it with the fix on Windows and saw that it works correctly?

Yes.

@vmalloc
Copy link
Member

vmalloc commented Feb 12, 2019

Ok. Can you please rebase on develop and push again? Once the build passes I will merge it.

@eliadh
Copy link
Author

eliadh commented Feb 12, 2019

Ok. Can you please rebase on develop and push again? Once the build passes I will merge it.

Done.

@vmalloc
Copy link
Member

vmalloc commented Feb 12, 2019

Thanks!

@vmalloc vmalloc merged commit 4fe6f71 into getslash:develop Feb 12, 2019
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