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 2019/05/22 02:20:48 UTC

[GitHub] [commons-csv] davidmoten opened a new pull request #41: [CSV-239] Add CSVRecord.getHeaderNames and allow duplicate headers

davidmoten opened a new pull request #41: [CSV-239] Add CSVRecord.getHeaderNames and allow duplicate headers
URL: https://github.com/apache/commons-csv/pull/41
 
 
   These are the changes:
   
   * add `getHeaderNames` returns all headers in column order including repeats which are allowed in general as per RFC 4180
   * add `CSVFormat.withAllowDuplicateHeaderNames()`. `CSVFormat.DEFAULT` now allows duplicate header names because RFC 4190 allows non-unique header names. This is a behavioural change but not a breaking API change anywhere because there is no API contract for it (e.g. javadoc). Because `CSVFormat.DEFAULT` should reflect RFC 4190 I'd classify this as a bug fix.
   * `CSVFormat` is `Serializable` which means adding new fields to it (`allowDuplicateHeaderNames`) is theoretically a breaking change. I propose we allow this minor breaking change and also propose that `CSVFormat` does not implement `Serializable` in 2.x
   * fix `CSVRecord.toMap` javadoc
   * fix bug in `CSVParser` where an IAE is thrown with a message about duplicate headers when the problem was actually a missing header name
   * add test coverage 
   
   Question:
   * do we need to talk about HeaderNames when we could just say Header?
   
   Not addressed:
   * would be nice if `CSVRecord.toMap` returned a Map whose entries are iterable in column order but this involves quite a bit of rework so will leave for another PR (probably for 2.x).
   * `CSVRecord.get(String)` should ideally throw when two columns with that header name exist
   
   Notes for 2.x: 
   * for consistency `CSVFormat.withAllowMissingColumnNames` should be `CSVFormat.withAllowMissingHeaderNames`
   * remove `Serializable` from `CSVFormat`
   * `CSVFormat.withIgnoreHeaderCase` creates problems and lacks flexibility. I'd suggest `CSVRecord.getIgnoreCase(int)` instead
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services