telnet-rs
telnet-rs copied to clipboard
Separate I/O from Telnet Parsing & Formatting
Following on from #15, this PR separates I/O from the parsing and formatting of Telnet data to allow alternate I/O models such as async/await.
TODO:
- [x] Parser & Formatting functions
- [x] Synchronous API with split reads and writes
- [ ] Compression in Synchronous API
- [x] No copy writes for data
- [ ] Tests
- [ ] Docs & README
Following on from the discussion in the issue:
I am thinking maybe it means that we should not implement Write for the telnet writer. Since the telnet protocol is an application layer protocol, it is inevitable to add some other bytes when writing data. I check a few other libraries such as hyper. They also do not implement Write for their high level write APIs. I believe it is because they are treating the user data as 'objects' instead of 'bytes' so they do not need to implement Write to handle 'bytes' for users. However, if you thought it would be better to implement Write for the telnet writer, we could first check if there is any other library encountering the same issue like us.
I also couldn't find any examples of this being done in other libraries and agree that the Write
trait isn't suitable. I have removed it from this PR.
One issue I have with the current proposal is that data is copied into a box as a part of formatting (escaping) it. It is possible and would be faster if data formatting was done entirely through slices. One way this could be done is changing format::data
to pub fn data(data: &[u8]) -> Vec<&[u8]>
. You would then use it like this:
let data = [1, 2, 0xFF, 3, 4];
let buffer = telnet::format::data(&data);
for b in buffer.into_iter() {
stream.write_all(b)?;
}
Thoughts?
Sure, I agree with you. We should use slicing instead of copying for the data. Your proposal for format::data
looks good to me.
I have made the return type for format::data
a new type to allow the inclusion of a to_owned()
method which copies the formatted data slices into a Box.
Any updates?