Change UnbufferedCharStream to use code points #1796
Conversation
bhamiltoncx
force-pushed
the
unbuffered-char-stream-code-points
branch
from
8e90f0b
to
dd8ac6a
Compare
| @@ -82,7 +82,7 @@ public UnbufferedCharStream() { | |||
| /** Useful for subclasses that pull char from other than this.input. */ | |||
| public UnbufferedCharStream(int bufferSize) { | |||
| n = 0; | |||
| data = new char[bufferSize]; | |||
| data = new int[bufferSize]; | |||
| } | |||
|
|
|||
| public UnbufferedCharStream(InputStream input) { | |||
There was a problem hiding this comment.
Note this ends up calling new InputStreamReader(input) without specifying a Charset. That means this logic depends on the client's $LANG environment variable or equivalent, which is usually not what anyone wants.
There was a problem hiding this comment.
It seems like we should add an optional charset arg on one or more of the arguments. Also, doesn't the fill() method assume UTF-X? I.e., it would not work with some other encoding, right?
There was a problem hiding this comment.
Yes! We absolutely should add a Charset arg, or there's no way to use anything but the default in the calling environment. (We should probably also change to default to UTF-8..)
fill() uses the InputStreamReader to get UTF-16 encoded chars. It will work with any encoding, although there's no way to specify an encoding other than the environment default with the current UnbufferedCharStream API.
There was a problem hiding this comment.
ok, great! could you please make the changes? Does that mean we change this field to InputStream?
protected Reader input;
There was a problem hiding this comment.
Sure, I'll make the changes! Nope, we'll still use a Reader, we'll just use a different InputStreamReader constructor which allows us to specify a Charset.
bhamiltoncx
force-pushed
the
unbuffered-char-stream-code-points
branch
from
dd8ac6a
to
8108b34
Compare
| @@ -183,8 +206,8 @@ public int LA(int i) { | |||
| int index = p + i - 1; | |||
| if ( index < 0 ) throw new IndexOutOfBoundsException(); | |||
| if ( index >= n ) return IntStream.EOF; | |||
| char c = data[index]; | |||
| if ( c==(char)IntStream.EOF ) return IntStream.EOF; | |||
| int c = data[index]; | |||
There was a problem hiding this comment.
Oops, just realized this can be tidied up to just return data[index];.
| add(c); | ||
| if (c > Character.MAX_VALUE || c == IntStream.EOF) { | ||
| add(c); | ||
| } else { |
There was a problem hiding this comment.
reminder that our "style" is else starts a line ;)
Fixes #1795 .
This changes the Java implementation of
UnbufferedCharStreamto use code points, not UTF-16 code units, following the changes toCharStreams.For now, I didn't bother putting this in the
CharStreamsinterface, but if we end up changing that to use aBuilderstyle, we could make anunbuffered()option.I updated the tests. Note the weirdness around
\uFFFF— before, we were relying on(char)IntStream.EOFconverting-1to\uFFFF, but now we no longer have that, so the test code explicitly appends\uFFFF.