You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/09/08 14:34:36 UTC
[GitHub] [commons-csv] garydgregory commented on a diff in pull request #257: [CSV-304] Accessors for header/trailer comments
garydgregory commented on code in PR #257:
URL: https://github.com/apache/commons-csv/pull/257#discussion_r964979116
##########
src/main/java/org/apache/commons/csv/CSVParser.java:
##########
@@ -480,10 +483,12 @@ private Headers createHeaders() throws IOException {
final CSVRecord nextRecord = this.nextRecord();
if (nextRecord != null) {
headerRecord = nextRecord.values();
+ headerComment = nextRecord.getComment();
}
} else {
if (this.format.getSkipHeaderRecord()) {
- this.nextRecord();
+ final CSVRecord csvRecord = this.nextRecord();
Review Comment:
NPE waiting to happen?
##########
src/main/java/org/apache/commons/csv/CSVParser.java:
##########
@@ -352,6 +352,9 @@ public static CSVParser parse(final URL url, final Charset charset, final CSVFor
return new CSVParser(new InputStreamReader(url.openStream(), charset), format);
}
+ private String headerComment;
Review Comment:
Add a blank line after the close brace.
##########
src/test/java/org/apache/commons/csv/CSVParserTest.java:
##########
@@ -1375,4 +1375,147 @@ private void validateRecordPosition(final String lineSeparator) throws IOExcepti
parser.close();
}
+ // CSV with no header comments
+ static private final String CSV_INPUT_NO_COMMENT = "A,B"+CRLF+"1,2"+CRLF;
+ // CSV with a header comment
+ static private final String CSV_INPUT_HEADER_COMMENT = "# header comment" + CRLF + "A,B" + CRLF + "1,2" + CRLF;
+ // CSV with a single line header and trailer comment
+ static private final String CSV_INPUT_HEADER_TRAILER_COMMENT = "# header comment" + CRLF + "A,B" + CRLF + "1,2" + CRLF + "# comment";
+ // CSV with a multi-line header and trailer comment
+ static private final String CSV_INPUT_MULTILINE_HEADER_TRAILER_COMMENT = "# multi-line" + CRLF + "# header comment" + CRLF + "A,B" + CRLF + "1,2" + CRLF + "# multi-line" + CRLF + "# comment";
+ // Format with auto-detected header
+ static private final CSVFormat FORMAT_AUTO_HEADER = CSVFormat.Builder.create(CSVFormat.DEFAULT).setCommentMarker('#').setHeader().build();
+ // Format with explicit header
+ static private final CSVFormat FORMAT_EXPLICIT_HEADER = CSVFormat.Builder.create(CSVFormat.DEFAULT)
+ .setSkipHeaderRecord(true)
+ .setCommentMarker('#')
+ .setHeader("A", "B")
+ .build();
+ // Format with explicit header that does not skip the header line
+ CSVFormat FORMAT_EXPLICIT_HEADER_NOSKIP = CSVFormat.Builder.create(CSVFormat.DEFAULT)
+ .setCommentMarker('#')
+ .setHeader("A", "B")
+ .build();
+ @Test
+ public void testGetHeaderComment_NoComment1() throws IOException {
+
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_NO_COMMENT, FORMAT_AUTO_HEADER)) {
+ parser.getRecords();
+ // Expect no header comment
+ assertFalse(parser.hasHeaderComment());
+ assertNull(parser.getHeaderComment());
+ }
+ }
+ @Test
+ public void testGetHeaderComment_HeaderComment1() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_COMMENT, FORMAT_AUTO_HEADER)) {
+ parser.getRecords();
+ // Expect a header comment
+ assertTrue(parser.hasHeaderComment());
+ assertEquals("header comment", parser.getHeaderComment());
+ }
+ }
+ @Test
+ public void testGetHeaderComment_HeaderTrailerComment() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_MULTILINE_HEADER_TRAILER_COMMENT, FORMAT_AUTO_HEADER)) {
+ parser.getRecords();
+ // Expect a header comment
+ assertTrue(parser.hasHeaderComment());
+ assertEquals("multi-line"+LF+"header comment", parser.getHeaderComment());
+ }
+ }
+ @Test
+ public void testGetHeaderComment_NoComment2() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_NO_COMMENT, FORMAT_EXPLICIT_HEADER)) {
+ parser.getRecords();
+ // Expect no header comment
+ assertFalse(parser.hasHeaderComment());
+ assertNull(parser.getHeaderComment());
+ }
+ }
+ @Test
+ public void testGetHeaderComment_HeaderComment2() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_COMMENT, FORMAT_EXPLICIT_HEADER)) {
+ parser.getRecords();
+ // Expect a header comment
+ assertTrue(parser.hasHeaderComment());
+ assertEquals("header comment", parser.getHeaderComment());
+ }
+ }
+ @Test
+ public void testGetHeaderComment_NoComment3() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_NO_COMMENT, FORMAT_EXPLICIT_HEADER_NOSKIP)) {
+ parser.getRecords();
+ // Expect no header comment
+ assertFalse(parser.hasHeaderComment());
+ assertNull(parser.getHeaderComment());
+ }
+ }
+ @Test
+ public void testGetHeaderComment_HeaderComment3() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_COMMENT, FORMAT_EXPLICIT_HEADER_NOSKIP)) {
+ parser.getRecords();
+ // Expect no header comment - the text "comment" is attached to the first record
+ assertFalse(parser.hasHeaderComment());
+ assertNull(parser.getHeaderComment());
+ }
+ }
+
+ @Test
+ public void testGetTrailerComment_HeaderComment1() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_COMMENT, FORMAT_AUTO_HEADER)) {
+ parser.getRecords();
+ assertFalse(parser.hasTrailerComment());
+ assertNull(parser.getTrailerComment());
+ }
+ }
+ @Test
+ public void testGetTrailerComment_HeaderTrailerComment1() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_TRAILER_COMMENT, FORMAT_AUTO_HEADER)) {
+ parser.getRecords();
+ assertTrue(parser.hasTrailerComment());
+ assertEquals("comment", parser.getTrailerComment());
+ }
+ }
+ @Test
+ public void testGetTrailerComment_MultilineComment() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_MULTILINE_HEADER_TRAILER_COMMENT, FORMAT_AUTO_HEADER)) {
+ parser.getRecords();
+ assertTrue(parser.hasTrailerComment());
+ assertEquals("multi-line"+LF+"comment", parser.getTrailerComment());
+ }
+ }
+ @Test
+ public void testGetTrailerComment_HeaderComment2() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_COMMENT, FORMAT_EXPLICIT_HEADER)) {
+ parser.getRecords();
+ assertFalse(parser.hasTrailerComment());
+ assertNull(parser.getTrailerComment());
+ }
+ }
+ @Test
+ public void testGetTrailerComment_HeaderTrailerComment2() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_TRAILER_COMMENT, FORMAT_EXPLICIT_HEADER)) {
+ parser.getRecords();
+ assertTrue(parser.hasTrailerComment());
+ assertEquals("comment", parser.getTrailerComment());
+ }
+ }
+ @Test
+ public void testGetTrailerComment_HeaderComment3() throws IOException {
+ try (CSVParser parser = CSVParser.parse(CSV_INPUT_HEADER_COMMENT, FORMAT_EXPLICIT_HEADER_NOSKIP)) {
+ parser.getRecords();
+ assertFalse(parser.hasTrailerComment());
+ assertNull(parser.getTrailerComment());
+ }
+ }
+ @Test
Review Comment:
Please separate methods with a blank line.
##########
src/main/java/org/apache/commons/csv/CSVParser.java:
##########
@@ -713,6 +764,10 @@ CSVRecord nextRecord() throws IOException {
case EOF:
if (this.reusableToken.isReady) {
this.addRecordValue(true);
+ } else {
Review Comment:
Extra block level is not needed, IOW use "else if" instead of "else { if ... }"
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org