-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Capitalize error messages #3955
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sija
force-pushed
the
capitalize-error-messages
branch
2 times, most recently
from
January 30, 2017 11:23
ee6b2ad
to
1527a2d
Compare
…xisting code. Related to crystal-lang#3922
…Char, String or a block
This method lets you peek into possibly buffered data held by an IO. With this, some methods, for example `gets`, can be performed more efficiently: if we can see which bytes come next we can see if there's the delimiter we are looking for. Before this, this optimization for `gets` was implemented in `IO::Buffered`, which has direct access to this buffer, with some other IO wrappers forwarding their `gets` method to the wrapped IOs. By introducing `peek`, `gets` can now be implemented directly in `IO`, by trying to peek into the buffer, if any. Since IOs in Crystal are buffered in most cases (IO::FileDescriptor is, and IO::Memory can return the whole bytes are in `peek`) it's almost always the case that `gets` will benefit from this optimization. Additionally, `IO::Sized`, which previously overrode `gets`, no longer needs to do it and can now just override `peek`, which has a much simpler implementation. In this particular case it also leads to a faster `gets` performance. This `peek` method can also be defined for other IOs, like ARGF and HTTP::Content, simplifying their code. Another future optimization is to enhance `IO::Delimtied` by trying to peek into an IO's buffer to see if the delimiter is there, instead of working with a byte buffer that has the size of the delimiter. Finally, the current Zlib::Inflate implementation reads more data than it should, which is incorrect and can lead to, for example, some zip files to be incorrectly parsed. A correct solution to this is to either reimplement the DEFLATE algorithm in Crystal, or to feed zlib data byte per byte. The later is simpler, but slower. But with `peek` we can feed more data, know how much was actually used, and only `skip` that much data without effectively reading more than necessary.
Crystal provides access to the Adler32, CRC32, DEFLATE, gzip and zlib algorithms/formats. It currently does so by binding to zlib (also known as libz). However, zlib is just an implementation detail: this shouldn't leak to type and method names because if we eventually decide to change the library used to implement these, or maybe implement stuff in pure Crystal, we'll be stuck with this name. So, here we split the contents of the Zlib module into: - Adler32, for the Adler32 checksum algorithm - CRC32, for the CRC32 checksum algorithm - Flate: for the DEFLATE compression format (RFC 1951), providing Reader and Writer types (Inflate and Deflate could also work, but Reader and Writer are more obvious and consistent.) Flate is also the name used by Go to provide the same functionality, so it will be familiar to some. - Gzip: for the gzip archive format (RFC 1952), which is just a small wrapper (header and checksum) around the DEFLATE format. Reader and Writer are provided, together with access to the first gzip header. - Zlib: for the zlib archive format (RFC 1950), which is just a small wrapper around the DEFLATE format too (here the format is also named the same as the C library, which brings a lot of confusion). Reader and Writer are provided. By doing this we also remove the need to know how to use the zlib C library, which requires users to provide a cryptic `windowBits` argument to choose the desierd format. Finally, we rename HTTP::DeflateHandler to HTTP::CompressHandler because that's what it does: it compress responses in either gzip or DEFLATE, but not always DEFLATE (so the name was misleading). All of this is a big breaking change, but should be easy to upgrade existing code and makes the standard library more consistent and organized.
Sija
force-pushed
the
capitalize-error-messages
branch
from
January 30, 2017 11:33
1527a2d
to
a22c9e6
Compare
I fucked up the rebase :/ I'll open a fresh PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes error messages according to #3944.