You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by jhorcicka <gi...@git.apache.org> on 2016/08/05 08:15:05 UTC
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
GitHub user jhorcicka opened a pull request:
https://github.com/apache/metamodel/pull/122
METAMODEL-1109: FixedWidthReader diacritics bug fix
Refactoring of FixedWidthReader in PR #103 introduced a bug related to charset (which was not used). So, input containing diacritic characters was not read correctly.
JUnit FixedWidthReaderTest was updated with charset tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jhorcicka/metamodel METAMODEL-1109-FixedWidthReader-diacritics-bug-fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metamodel/pull/122.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #122
----
commit e23f4877b93bd39e4daf7dd17740c252f6f39ab2
Author: jhorcicka <ja...@humaninference.com>
Date: 2016-08-05T07:51:18Z
METAMODEL-1109: FixedWidthReader diacritics bug fix (_charsetName was not used). Stream was replaced by a reader.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
Posted by jhorcicka <gi...@git.apache.org>.
Github user jhorcicka commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/122#discussion_r74198110
--- Diff: fixedwidth/src/main/java/org/apache/metamodel/fixedwidth/EbcdicReader.java ---
@@ -26,13 +26,17 @@
*/
class EbcdicReader extends FixedWidthReader {
+ private final BufferedInputStream _stream;
+ private final String _charsetName;
private final boolean _skipEbcdicHeader;
private final boolean _eolPresent;
private boolean _headerSkipped;
public EbcdicReader(BufferedInputStream stream, String charsetName, int[] valueWidths,
boolean failOnInconsistentLineWidth, boolean skipEbcdicHeader, boolean eolPresent) {
super(stream, charsetName, valueWidths, failOnInconsistentLineWidth);
+ _stream = stream;
+ _charsetName = charsetName;
--- End diff --
But they are, lines: 61, 70, 76.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
Posted by jhorcicka <gi...@git.apache.org>.
Github user jhorcicka commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/122#discussion_r73830409
--- Diff: fixedwidth/src/main/java/org/apache/metamodel/fixedwidth/FixedWidthReader.java ---
@@ -43,6 +47,7 @@
private final boolean _constantWidth;
private volatile int _rowNumber;
protected final BufferedInputStream _stream;
+ protected Reader _reader;
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
Posted by jhorcicka <gi...@git.apache.org>.
Github user jhorcicka commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/122#discussion_r73656991
--- Diff: fixedwidth/src/main/java/org/apache/metamodel/fixedwidth/FixedWidthReader.java ---
@@ -85,6 +92,15 @@ public FixedWidthReader(InputStream stream, String charsetName, int[] valueWidth
_expectedLineLength = expectedLineLength;
}
+ private void initReader() {
+ try {
+ InputStreamReader inputStreamReader = new InputStreamReader(_stream, _charsetName);
--- End diff --
This was the missing part in the previous version, _charsetName was not used anywhere.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metamodel/pull/122
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/122#discussion_r73817859
--- Diff: fixedwidth/src/main/java/org/apache/metamodel/fixedwidth/FixedWidthReader.java ---
@@ -43,6 +47,7 @@
private final boolean _constantWidth;
private volatile int _rowNumber;
protected final BufferedInputStream _stream;
+ protected Reader _reader;
--- End diff --
I would suggest to keep having final fields only, and since `_stream` and `_charset` are no longer really used, I'd suggest to remove those and instead make the `initReader` return the created Reader with the needed arguments, like this:
```
_reader = initReader(stream, charsetName);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/122#discussion_r74183535
--- Diff: fixedwidth/src/main/java/org/apache/metamodel/fixedwidth/EbcdicReader.java ---
@@ -26,13 +26,17 @@
*/
class EbcdicReader extends FixedWidthReader {
+ private final BufferedInputStream _stream;
+ private final String _charsetName;
private final boolean _skipEbcdicHeader;
private final boolean _eolPresent;
private boolean _headerSkipped;
public EbcdicReader(BufferedInputStream stream, String charsetName, int[] valueWidths,
boolean failOnInconsistentLineWidth, boolean skipEbcdicHeader, boolean eolPresent) {
super(stream, charsetName, valueWidths, failOnInconsistentLineWidth);
+ _stream = stream;
+ _charsetName = charsetName;
--- End diff --
These two don't seem to ever be used?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #122: METAMODEL-1109: FixedWidthReader diacritics bug...
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/122#discussion_r74330944
--- Diff: fixedwidth/src/main/java/org/apache/metamodel/fixedwidth/EbcdicReader.java ---
@@ -26,13 +26,17 @@
*/
class EbcdicReader extends FixedWidthReader {
+ private final BufferedInputStream _stream;
+ private final String _charsetName;
private final boolean _skipEbcdicHeader;
private final boolean _eolPresent;
private boolean _headerSkipped;
public EbcdicReader(BufferedInputStream stream, String charsetName, int[] valueWidths,
boolean failOnInconsistentLineWidth, boolean skipEbcdicHeader, boolean eolPresent) {
super(stream, charsetName, valueWidths, failOnInconsistentLineWidth);
+ _stream = stream;
+ _charsetName = charsetName;
--- End diff --
Ah yes, it was fooling me because it used to be the super-class' fields and now they've moved here... Makes sense.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---