Hi B,
Random day today, I am not sure I can work on this. Feel free to proceed in
what ever way you best see fit. I trust you :-)
Gary
On Oct 30, 2016 10:57 AM, "Benedikt Ritter" <brit...@apache.org> wrote:
> Hello Gary,
>
> please see my comments in CSV-201.
>
> Thank you,
> Benedikt
>
> Gary Gregory <garydgreg...@gmail.com> schrieb am Fr. 28. Okt. 2016 um
> 17:05:
>
> > I do not think reverting is right since the previous is obviously bogus,
> to
> > me, that is. Throwing a RE is never useful, again IMO. We should just
> > evolve the code from here.
> >
> > Would you like to outline choices?
> >
> > 1) NSEE
> > 2) Custom
> > 3) ISE
> > 4) ?
> >
> > Gary (will be offline most of today)
> >
> > On Oct 28, 2016 5:36 AM, "Benedikt Ritter" <brit...@apache.org> wrote:
> >
> > Hello,
> >
> > Benedikt Ritter <brit...@apache.org> schrieb am Mi., 26. Okt. 2016 um
> > 21:38 Uhr:
> >
> > > Hello,
> > >
> > > <ggreg...@apache.org> schrieb am Di., 25. Okt. 2016 um 22:59 Uhr:
> > >
> > > Repository: commons-csv
> > > Updated Branches:
> > > refs/heads/master b2a50d4a3 -> 1c7a9910b
> > >
> > >
> > > [CSV-201] Do not use RuntimeException in CSVParser.iterator().new
> > > Iterator() {...}.getNextRecord(). Use an IllegalStateException instead
> > > (KISS for now, no need for a custom exception)
> > >
> > > Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
> > > Commit:
> > http://git-wip-us.apache.org/repos/asf/commons-csv/commit/1c7a9910
> > > Tree: http://git-wip-us.apache.org/repos/asf/commons-csv/tree/1c7a9910
> > > Diff: http://git-wip-us.apache.org/repos/asf/commons-csv/diff/1c7a9910
> > >
> > > Branch: refs/heads/master
> > > Commit: 1c7a9910be857a4f33ed216701407806e1d20670
> > > Parents: b2a50d4
> > > Author: Gary Gregory <ggreg...@apache.org>
> > > Authored: Tue Oct 25 13:59:32 2016 -0700
> > > Committer: Gary Gregory <ggreg...@apache.org>
> > > Committed: Tue Oct 25 13:59:32 2016 -0700
> > >
> > > ----------------------------------------------------------------------
> > > src/changes/changes.xml | 1 +
> > > src/main/java/org/apache/commons/csv/CSVParser.java | 16
> > ++++++++++------
> > > 2 files changed, 11 insertions(+), 6 deletions(-)
> > > ----------------------------------------------------------------------
> > >
> > >
> > >
> > > http://git-wip-us.apache.org/repos/asf/commons-csv/blob/
> > 1c7a9910/src/changes/changes.xml
> > > ----------------------------------------------------------------------
> > > diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> > > index d5d8c8a..e49265b 100644
> > > --- a/src/changes/changes.xml
> > > +++ b/src/changes/changes.xml
> > > @@ -43,6 +43,7 @@
> > > <action issue="CSV-193" type="fix" dev="ggregory"
> due-to="Matthias
> > > Wiehl">Fix incorrect method name 'withFirstRowAsHeader' in user
> > > guide.</action>
> > > <action issue="CSV-171" type="fix" dev="ggregory" due-to="Gary
> > > Gregory, Michael Graessle, Adrian Bridgett">Negative numeric values in
> > the
> > > first column are always quoted in minimal mode.</action>
> > > <action issue="CSV-187" type="update" dev="ggregory"
> due-to="Gary
> > > Gregory">Update platform requirement from Java 6 to 7.</action>
> > > + <action issue="CSV-201" type="update" dev="ggregory"
> > > due-to="Benedikt Ritter, Gary Gregory">Do not use RuntimeException in
> > > CSVParser.iterator().new Iterator() {...}.getNextRecord()</action>
> > > <action issue="CSV-189" type="add" dev="ggregory" due-to="Peter
> > > Holzwarth, Gary Gregory">CSVParser: Add factory method accepting
> > > InputStream.</action>
> > > <action issue="CSV-190" type="add" dev="ggregory" due-to="Gary
> > > Gregory">Add convenience API CSVFormat.print(File, Charset)</action>
> > > <action issue="CSV-191" type="add" dev="ggregory" due-to="Gary
> > > Gregory">Add convenience API CSVFormat.print(Path, Charset)</action>
> > >
> > >
> > > http://git-wip-us.apache.org/repos/asf/commons-csv/blob/
> > 1c7a9910/src/main/java/org/apache/commons/csv/CSVParser.java
> > > ----------------------------------------------------------------------
> > > diff --git a/src/main/java/org/apache/commons/csv/CSVParser.java
> > > b/src/main/java/org/apache/commons/csv/CSVParser.java
> > > index f5829cc..410e6a4 100644
> > > --- a/src/main/java/org/apache/commons/csv/CSVParser.java
> > > +++ b/src/main/java/org/apache/commons/csv/CSVParser.java
> > > @@ -501,10 +501,14 @@ public final class CSVParser implements
> > > Iterable<CSVRecord>, Closeable {
> > > /**
> > > * Returns an iterator on the records.
> > > *
> > > - * <p>IOExceptions occurring during the iteration are wrapped in a
> > > - * RuntimeException.
> > > - * If the parser is closed a call to {@code next()} will throw a
> > > - * NoSuchElementException.</p>
> > > + * <p>
> > > + * An {@link IOException} caught during the iteration are
> re-thrown
> > > as an
> > > + * {@link IllegalStateException}.
> > > + * </p>
> > > + * <p>
> > > + * If the parser is closed a call to {@link Iterator#next()} will
> > > throw a
> > > + * {@link NoSuchElementException}.
> > > + * </p>
> > > */
> > > @Override
> > > public Iterator<CSVRecord> iterator() {
> > > @@ -515,8 +519,8 @@ public final class CSVParser implements
> > > Iterable<CSVRecord>, Closeable {
> > > try {
> > > return CSVParser.this.nextRecord();
> > > } catch (final IOException e) {
> > > - // TODO: This is not great, throw an ISE instead?
> > > - throw new RuntimeException(e);
> > > + throw new IllegalStateException(
> > > + e.getClass().getSimpleName() + " reading
> > next
> > > record: " + e.toString(), e);
> > >
> > >
> > > As discussed in CSV-201, I don't think this fix is complete. We should
> > > rather throw a custom RuntimeException to make it easy for calling code
> > to
> > > react to exactly this problem.
> > >
> >
> > I think this needs to be reverted until we have consensus about how to
> > solve this issue.
> >
> >
> > >
> > > Benedikt
> > >
> > >
> > > }
> > > }
> > >
> > >
> > >
> >
>
