Add leading slash to remote paths #250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In support of #243 adds leading slash to remote paths. This allows users to naturally specify the top level 'directory' of a project with '/'. Previously this was omitted to match the DukeDS web portal. Changes the download command arguments include/exclude to allow leading slash or the current functionality of omitting the leading slash.
dleehr
suggested changes
Jun 12, 2019
There was a problem hiding this comment.
I see that there are many cases where os.sep is used as the path separator character when referring to remote file paths. Recalling that on Windows, os.sep will be '\\', I though this would lead to confusing behavior (e.g. Windows users running ddsclient list -p project would see remote paths delimited by backslashes). But that's not exactly what I'm seeing.
Instead, I'm seeing that the leading slash is now a \ on Windows, but the rest of the path separators are /. I'm not sure why exactly that is, but have added a screenshot:
I think when dealing with remote paths, we should always use an explicit '/', and not the OS'es path separator character.
dleehr
approved these changes
Jun 13, 2019
This was referenced Jun 13, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In support of #243 adds leading slash to remote paths.
This allows users to naturally specify the top level 'directory' of a remote project with '/'.
Previously this leading slash was omitted to match the DukeDS web portal.
Changes the
downloadanddelivercommands' arguments include/exclude to allow leading slash or the current functionality of omitting the leading slash. Thelistcommand will now print leading slashes.