-
Notifications
You must be signed in to change notification settings - Fork 713
IO-600: fix getPrefixLength for Linux filename #178
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
Conversation
| <groupId>org.apache.commons</groupId> | ||
| <artifactId>commons-lang3</artifactId> | ||
| <version>3.11</version> | ||
| <scope>test</scope> |
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.
-1 we don't want this runtime dependency.
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.
thanks, remove runtime dependency
| ch0 = Character.toUpperCase(ch0); | ||
| if (ch0 >= 'A' && ch0 <= 'Z') { | ||
| if (len == 2 || isSeparator(fileName.charAt(2)) == false) { | ||
| if (len == 2 && SystemUtils.IS_OS_LINUX) { |
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.
I think you can use "!FilenameUtils.isSystemWindows()" instead. see https://github.com/apache/commons-io/blob/master/src/main/java/org/apache/commons/io/FilenameUtils.java#L146
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.
thanks, updated to use "!FilenameUtils.isSystemWindows()"
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.
I though about this a little more and since other OSs support driver letters, I've abstracted this away in a new method FileSystem#supportsDriveLetter(), so please rebase on master and use that method instead of testing for the OS directly.
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.
FileSystem.supportsDriveLetter() is non-static method getPrefixLength is static
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.
So?
|
@abchaubey |
|
@garydgregory I will create new PR for this from new fork |
|
@garydgregory new PR :#179 , closing this one |
No description provided.