Skip to content
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
wants to merge 24 commits into from

Conversation

Sija
Copy link
Contributor

@Sija Sija commented Jan 30, 2017

This PR changes error messages according to #3944.

@Sija Sija force-pushed the capitalize-error-messages branch 2 times, most recently from ee6b2ad to 1527a2d Compare January 30, 2017 11:23
TheLonelyGhost and others added 24 commits January 30, 2017 12:27
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
Copy link
Contributor Author

Sija commented Jan 30, 2017

I fucked up the rebase :/ I'll open a fresh PR.

@Sija Sija closed this Jan 30, 2017
@Sija Sija deleted the capitalize-error-messages branch January 30, 2017 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants