New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APIs to read and write code points. #145

Merged
merged 1 commit into from Apr 26, 2015

Conversation

2 participants
@swankjesse
Copy link
Member

swankjesse commented Apr 25, 2015

The String APIs transcode UTF-16 to UTF-8 and back.

These APIs avoid the UTF-16 intermediate form altogether, and
go right from UTF-8 to a codepoint and back.


} else if (codePoint < 0x10000) {
if (codePoint >= 0xd800 && codePoint <= 0xdfff) {
throw new IllegalArgumentException(

This comment has been minimized.

@swankjesse

swankjesse Apr 25, 2015

Member

Interesting design decision here. What to do when the input's not quite right:

  • we could throw
  • we could encode it (it's still recoverable on the other end, though encoders might return the replacement char)
  • we could encode the replacement character ourselves

Thoughts?

This comment has been minimized.

@JakeWharton

JakeWharton Apr 26, 2015

Collaborator

Tough. Option 2 doesn't seem reasonable to me. I'm inclined to lean towards 1 (as implemented), but I see the value in 3. I think it's hard to be too forgiving at this level. A higher-level API could handle the replacement character writing, but when you're at this level I think throwing is correct.

@swankjesse

This comment has been minimized.

Copy link
Member

swankjesse commented Apr 25, 2015

HttpUrl wants this. It's defined in terms of code points, and wants to parse codepoint-by-codepoint. I'm currently doing it Java char-by-char, but it's awkward.
square/okhttp#1486

* Removes and returns a single UTF-8 code point, reading between 1 and 4 bytes as necessary.
*
* <p>If this source is exhausted before a complete code point can be read, this throws an {@link
* java.io.EOFException} and consumes no input.

This comment has been minimized.

@JakeWharton

JakeWharton Apr 26, 2015

Collaborator

Nice doc. I don't think we're very good about documenting the behavior when methods throw (related to the contents of the underlying buffer).

@@ -203,6 +203,10 @@ public RealBufferedSource(Source source) {
return buffer.readUtf8Line(newline);
}

@Override public int readUtf8CodePoint() throws IOException {
return buffer.readUtf8CodePoint();

This comment has been minimized.

@JakeWharton

JakeWharton Apr 26, 2015

Collaborator

You need request/require logic here.

Source s = new Buffer().writeByte('a');
BufferedSource bs = Okio.buffer(source);
bs.readUtf8CodePoint(); // <-- throws EOF

This comment has been minimized.

@JakeWharton

JakeWharton Apr 26, 2015

Collaborator

Note: BufferedSourceTest would catch this!

This comment has been minimized.

@swankjesse

swankjesse Apr 26, 2015

Member

Done.

Also changed BufferedSourceTest to support a special buffered source that receives data one byte at a time. That found a bug in readFully's exception handling.

This comment has been minimized.

@swankjesse

swankjesse Apr 26, 2015

Member

(strangely, I had it the right way initially, but got confused by the fact that I'm throwing EOFException anyway when the buffer is exhausted. Forgot about refilling the buffer.)

min = 0x0;

} else if ((b0 & 0xe0) == 0xc0) {
// 0x110xxxxx

This comment has been minimized.

@JakeWharton

JakeWharton Apr 26, 2015

Collaborator

You can use binary literals if you want:

((b0 & 0b1110000) == 0b1100000

This comment has been minimized.

@swankjesse

swankjesse Apr 26, 2015

Member

Oooh, that's interesting. I might circle back on that. I'm going to leave it as-is for now though.

writeByte(codePoint & 0x3f | 0x80); // 10xxxxxx

} else if (codePoint < 0x10000) {
if (codePoint >= 0xd800 && codePoint <= 0xdfff) {

This comment has been minimized.

@JakeWharton

JakeWharton Apr 26, 2015

Collaborator

You use hex here for the values but chars above.

This comment has been minimized.

@swankjesse

swankjesse Apr 26, 2015

Member

Good catch. Fixed to use integers in both places.

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Apr 26, 2015

LGTM

APIs to read and write code points.
The String APIs transcode UTF-16 to UTF-8 and back.

These APIs avoid the UTF-16 intermediate form altogether, and
go right from UTF-8 to a codepoint and back.

@swankjesse swankjesse force-pushed the jwilson_0425_utf8_code_points branch from 6529430 to 85e9f94 Apr 26, 2015

swankjesse added a commit that referenced this pull request Apr 26, 2015

Merge pull request #145 from square/jwilson_0425_utf8_code_points
APIs to read and write code points.

@swankjesse swankjesse merged commit 3c61fdb into master Apr 26, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@swankjesse swankjesse deleted the jwilson_0425_utf8_code_points branch May 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment