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: normalize input paths on posix; simplify code a bit #45

Merged
merged 2 commits into from
Jan 20, 2020

Conversation

timotheecour
Copy link
Collaborator

@timotheecour timotheecour commented Jan 19, 2020

/cc @citycide
reattempts #37 which had been reverted because it was causing issues on windows (due to lack of windows being tested back then)

note

I'm now only doing normalizedPath on posix, because it would cause complications on windows as it changes / to \
if someone needs a fix for #35 on windows, it can be handled in subsequent PR's; but right now the test suite is a bit weird for reasons unrelated to this PR: see #46

note2:

pathnorm.normalizePath might help with that but I don't want it in same PR, eg see proc normalizePath*(path: string; dirSep = DirSep): string =

@timotheecour timotheecour changed the title re-attempt: fix #35; simplify code a bit [WIP] re-attempt: fix #35; simplify code a bit Jan 19, 2020
@timotheecour timotheecour changed the title [WIP] re-attempt: fix #35; simplify code a bit e-attempt: fix #35; simplify code a bit Jan 20, 2020
@timotheecour timotheecour changed the title e-attempt: fix #35; simplify code a bit re-attempt: fix #35; simplify code a bit Jan 20, 2020
@haltcase haltcase changed the title re-attempt: fix #35; simplify code a bit fix: normalize input paths on posix; simplify code a bit Jan 20, 2020
@haltcase haltcase merged commit 28e5f93 into haltcase:master Jan 20, 2020
@haltcase
Copy link
Owner

LGTM, thanks.

@timotheecour timotheecour deleted the pr_fix_35_3 branch January 20, 2020 22:21
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