andre-vm/rev_buf_reader

Impl BufReader which is a combination of BufReader + RevBufReader

Opened this issue ยท 10 comments

Hi! It would be neat if there's a struct where you can read the next and previous parts of a content.
Currently BufReader let's you only read the next things and RevBufReader the previous (reverse) things.
Something in the middle would be awesome!

Also: I saw that the last commit is 2 years ago. May I ask why you're, for example, updating the dependencies (out of curiousity)?

Hi, @TornaxO7! Thank you for reaching out!

That's an interesting idea. But I think that would raise some questions, like:

  • Does it make sense to read a file forwards up to a certain point, then start reading it backwards from that same point? In that case, the user would end up reading the same chunk of data they read before.
  • BufReader always starts reading from the beginning of the file, while RevBufReader starts reading from the end. Where should this combined BufReader start reading from?
  • How would we handle the buffer? BufReader always buffers data ahead of the cursor, while RevBufReader buffers data from behind the cursor. Should the combined BufReader buffer data by placing the cursor in the middle of the buffer, so that we have some room to read in both directions? Or should we always stick to the same direction and invalidate the buffer altogether when the user decides to change?
  • Should this combined BufReader be integrated to this crate, or should a new one be created just for that?

I don't mean to imply this can't be done, it's just not very clear to me how it would work. I don't expect you to answer these questions either, I'm just sharing my doubts.

Also, I don't see what it would be useful for. Do you have a use case in mind?

As for your last question, you're right. The last time I did any changes to this crate was 2 years ago. I haven't done any maintenance since then. Back then, I updated the memchr dependency just as a good practice. I expect newer versions of crates in general to have fewer bugs, better performance, or just work better in some other way.

Also, I don't see what it would be useful for. Do you have a use case in mind?

Yes! I'm (or was) interested in writing another terminal-pager (like less) just for fun which should use a BufReader under the hood to quickly fetch the data which should be displayed.
Since it's normal for a terminal pager (in my opinion) to be able to scroll up and down in a document, BufReader would have to be able to read back and forth of the document/text/file/or whatever.

As for your last question, you're right. The last time I did any changes to this crate was 2 years ago. I haven't done any maintenance since then. Back then, I updated the memchr dependency just as a good practice. I expect newer versions of crates in general to have fewer bugs, better performance, or just work better in some other way.

May I ask if you'd be interested in enabling dependabot for this repository so it will automatically create a PR for you if there's a new version of one of your dependencies :)

Yes! I'm (or was) interested in writing another terminal-pager (like less) just for fun which should use a BufReader under the hood to quickly fetch the data which should be displayed.
Since it's normal for a terminal pager (in my opinion) to be able to scroll up and down in a document, BufReader would have to be able to read back and forth of the document/text/file/or whatever.

Oh, I see. That makes total sense.

Yeah, I think it's possible to create a BufReader that fulfills your use case. One thing I didn't remember is that RevBufReader implements the Seek trait, so that takes care of which point in the file the user would like to start reading from. The buffer could be always filled with the next page if the user is reading forward and with the previous page if the user is reading backward. When the user changes direction, it is possible that the data of interest is already buffered, but if it isn't, it wouldn't be a problem to just read from the file again.

I think it may be suitable for this crate to provide these features using a different struct. I'd keep the API of RevBufReader untouched, although it would be reasonable to change the implementation to use this combined BufReader internally. I'll consider doing that, but this is not a promise. ๐Ÿ˜…

If you're interested in implementing it yourself and making it part of this crate, you're welcome to open a pull request.

May I ask if you'd be interested in enabling dependabot for this repository so it will automatically create a PR for you if there's a new version of one of your dependencies :)

Of course! I just added the dependabot.yml configuration file. Let's see how it goes. ๐Ÿ™‚

I think it may be suitable for this crate to provide these features using a different struct.

Hmm... I'm also thinking to create another crate for this like BiBufReader because it is a bit different then the current crate which includes only a BufReader for reading backwards.
Or a collection like BufReader++ might be more suitable which could include your crate and an implementation of a BiBufReader if you know what I mean.

I'd keep the API of RevBufReader untouched.

of course!

although it would be reasonable to change the implementation to use this combined BufReader internally.

hm... I wouldn't be so sure about this. Having an explicit tool for a use case might be better here and I don't think that there will be that much difference.

Hmm... I'm also thinking to create another crate for this like BiBufReader because it is a bit different then the current crate which includes only a BufReader for reading backwards.
Or a collection like BufReader++ might be more suitable which could include your crate and an implementation of a BiBufReader if you know what I mean.

Either way, sounds great! If you're making a new crate, feel free to include RevBufReader in it. ๐Ÿ‘

hm... I wouldn't be so sure about this. Having an explicit tool for a use case might be better here and I don't think that there will be that much difference.

I mean.. RevBufReader and BiBufReader would have lots of logic in common. If we're having them both in the same crate (either by expanding this crate or creating a separate one), it would make sense to avoid coding the same logic twice.

Alright, I'll think about it but first, I'd like to contribute to rio :) I'll come back.

Okeydokey, I think adding a PR would be fine for this. Should I just add the struct to the whole project? Would you agree to rename the crate (rev_buf_reader may be a bit misleading afterwards in my opinion)? Are you fine if I split up the lib.rs? Or would you prefer that I create an extra crate for this (I wouldn't mind)?

Yes, I agree rev_buf_reader would be misleading after the new struct is added. But I think there's no way to rename a crate on crates.io. And that makes sense, otherwise Cargo dependencies could simply disappear overnight due to renaming.

So I think it would be best to start a new crate and a new repository for it. You can either start it from scratch or by forking this repo, whatever makes it easier for you. Feel free to adopt whatever file structure you think it's best. When the new crate is published, I can deprecate this one.

Okeydokey!

@andre-vm hi! It's main function are basically done. It's just maybe not very peformant for the time being. https://github.com/TornaxO7/ReadCollection
Maybe you'd like to take a look into it :D