kinow commented on a change in pull request #236: URL: https://github.com/apache/commons-compress/pull/236#discussion_r775080280
########## File path: src/main/java/org/apache/commons/compress/utils/FileNameUtils.java ########## @@ -50,6 +51,29 @@ public static String getExtension(final String filename) { return name.substring(extensionPosition + 1); } + /** + * Returns the extension (i.e. the part after the last ".") of a file. + * <p>Will return an empty string if the file name doesn't contain + * any dots. Only the last segment of a the file name is consulted + * - i.e. all leading directories of the {@code filename} + * parameter are skipped.</p> + * @return the extension of filename + * @param path the path of the file to obtain the extension of. + * @since 1.22 + */ + public static String getExtensionFrom(final Path path) { + if (path == null) { + return null; + } + + final String name = path.getFileName().toString(); + final int extensionPosition = name.lastIndexOf('.'); + if (extensionPosition < 0) { Review comment: Was going to comment on the public vs package/private here too, but I see @garydgregory already asked about it :point_up: @PostelnicuGeorge I think we can change here to ```suggestion if (extensionPosition < 0 || extensionPosition == name.length() - 1) { ``` ? This way we avoid calling `.substring` when the strings ends with a dot. ########## File path: src/main/java/org/apache/commons/compress/archivers/zip/ZipSplitReadOnlySeekableByteChannel.java ########## @@ -155,30 +158,40 @@ public static SeekableByteChannel forOrderedSeekableByteChannels(final SeekableB * the beginning of a split archive */ public static SeekableByteChannel buildFromLastSplitSegment(final File lastSegmentFile) throws IOException { - final String extension = FileNameUtils.getExtension(lastSegmentFile.getCanonicalPath()); + return buildFromLastSplitSegment(lastSegmentFile.toPath()); + } + + /** + * Concatenates zip split files from the last segment(the extension SHOULD be .zip) Review comment: Missing space between segment and parenthesis, and would SHOULD be replaced by MUST? It repeats in other javadocs further down in this file :point_down: -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org