-
Notifications
You must be signed in to change notification settings - Fork 17
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
Improve StudentFileAwareZipper #76
Conversation
c04726f
to
a508667
Compare
2 similar comments
@@ -110,10 +111,13 @@ private void writeToZip(Path currentPath, ZipArchiveOutputStream zipStream, Path | |||
log.trace("Writing {} to zip", currentPath); | |||
|
|||
String name = projectPath.getParent().relativize(currentPath).toString(); | |||
// zip standard says "/" is to be used as the path separator | |||
name = FilenameUtils.separatorsToUnix(name); |
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.
In Unix-based systems it is legal to have \
-characters in the filename. We don't want to accidentally interpret those as path separators. That's why I would prefer the implementation I proposed earlier.
ee75d66
to
a455afc
Compare
1 similar comment
2 similar comments
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.
One nitpick, otherwise looks good :)
log.trace("Generating zip-compliant filename from Path \"{}\", isDirectory: {}", | ||
path, isDirectory); | ||
// zip standard says "/" is to be used as the path separator. | ||
final char separator = '/'; |
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.
Can you move this out of the method and make it private static final char ZIP_SEPARATOR = '/'
?
The StudentFileAwareZipper includes the provided root directory in the zip archive as a parent directory. File system roots aren't named directories, so they cannot be added as such.
1 similar comment
a455afc
to
1b2b5d2
Compare
2 similar comments
10375a6
to
7998eb0
Compare
Closes #75