-
Notifications
You must be signed in to change notification settings - Fork 370
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: make sure destination uses posix separator instead of win #2304
Conversation
0a49589
to
a12c6ff
Compare
src/transfer-manager.ts
Outdated
@@ -396,12 +396,14 @@ export class TransferManager { | |||
...options.passthroughOptions, | |||
}; | |||
|
|||
passThroughOptionsCopy.destination = filePath; | |||
passThroughOptionsCopy.destination = filePath | |||
.split(path.win32.sep) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps using the OS-specific path.sep
would be preferred to prefer POSIX customers' ability to write with \
.
.split(path.win32.sep) | |
.split(path.sep) |
src/transfer-manager.ts
Outdated
.join(options.prefix, passThroughOptionsCopy.destination) | ||
.split(path.win32.sep) | ||
.join(path.posix.sep); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the prefix here should be split first to preserve the \
for POSIX (in case customers really want it)
.join(options.prefix, passThroughOptionsCopy.destination) | |
.split(path.win32.sep) | |
.join(path.posix.sep); | |
.posix.join(...options.prefix.split(path.sep), passThroughOptionsCopy.destination); |
a12c6ff
to
55ce5a7
Compare
470aca5
to
dc77605
Compare
dc77605
to
c0793bc
Compare
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #2301 🦕