Make IO a class #4901
Make IO a class #4901
Conversation
|
I was about to PR creating a |
|
Making |
|
Yes, I think we can remove |
|
(I'll leave it in this PR, though) |
There was a problem hiding this comment.
I can't think of an example where a class IO won't work and a module would be better. Yet I always liked that acting as an IO didn't require a base class.
It's hard to search "include IO" in all crystal repositories in github, from a wider search there are many occurrences that can be found. There are some usages in db drivers.
Would it be to much of a burden to:
-
in vNext
- declare IO::Base abstract class
- make IO print a deprecation warning
-
in vNext+1
- remove module IO
- change IO::Base to IO
- make IO::Base < IO with a deprecation warning
-
in vNext + 2
- remove IO::Base
Probably yes. The error will be point easily. But without that there is a hard dependency requirement for shards and compiler version IMO.
| @active_delimiter_buffer : Bytes | ||
|
|
||
| # Creates a new `IO::Delimited` which wraps *io*, and can read until the | ||
| # byte sequence *read_delimiter* (interpreted as UTF-8) is found. If |
There was a problem hiding this comment.
shouldn't the delimiter be interpreted in the encoding of the underling io?
There was a problem hiding this comment.
Maybe, but we can discuss that in a separate PR :-)
| @@ -1,12 +1,10 @@ | |||
| # The `IO::Buffered` mixin enhances the `IO` module with input/output buffering. | |||
| # The `IO::Buffered` mixin enhances an `IO` with input/output buffering. | |||
There was a problem hiding this comment.
WhyIO::Buffered should be a module and IO::Delimited a class?
There was a problem hiding this comment.
We could make IO::Buffered a class too, but then you'd have to do like in Java:
io = File.open("some_file")
io = IO::Buffered.new(io)
io.getsotherwise things will be very inefficient. That's why in Ruby buffering is embedded in these kind of IOs because it's almost always slow otherwise.
We could have IO::Buffering as a module and IO::Buffered as a class, though, or something like that.
Still, maybe a discussion for another issue :-)
|
@bcardiff I don't know about those steps. Maybe it's better to just break things, given that the fix is really simple. This will make shards that depend on other shards break and people filling PRs to fix this. |
|
Bump! |
|
Oh, this is actually approved. I'll use the "If a crystal-lang org member creates a PR, at least one other member has to approve it" card here |
This changes
IOto be aclassinstead of amodule. The reasons are:IOhas an@encodingvariable that can be set and changed. This makesIOas a struct very inconvenient because its mutable.IOs in the standard library.Because
Zip::Filenow works withIOinstead of a union of two specific IOs (and this can't be represented in the language because the types get merged to the closest ancestor) I added raising methods likeseekandpos, which also make sense because all IOs should provide that interface, even if they can't effectively implement it (otherwise, let's break IO into many small interfaces like in Go, that's acceptable too but a much bigger and boring change).