io.string_reader: fix write(), add test

[tankf33der]

The turn has come to the next function write() which behaves incomprehensibly to me.
Currently on the master branch after calling offset counter doesn't update and some functions break.

...
reader.read_bytes(1)!
reader.write('a'.bytes())!
reader.read_bytes(1)!      <- XXX, byte ignored
...

My idea of PR: The offset counter is the number of bytes in the builder and must be updated after writing additional bytes.

[spytheman]

I do not get it - offset is documented as:
offset int // current offset in the buffer

By setting it here, essentially you make it so the written data will be always ignored when reading (since the checks will show that the buffer needs refilling).

In the test later, you do not test, that what was written can be read too, but just the current content of the buffer.

[spytheman]

Please do some asserts, to show that what was written, can be read later as is by one of the .read methods, preferably .read_string, since that is easier to compare visually with the rest of the data in the tests.

[spytheman]

I am also not sure that a StringReader needs a .write() method at all, especially if it is not tested.
Can it be just deleted/deprecated instead?

[spytheman]

As it is on master, the .write() method seems to allow to inject data into the buffer, which does not seem particularly useful, but that it could be still read later.
On this PR, the .write() method will just inject data, that is later ignored, which is even less useful.

[tankf33der]

@spytheman
Having a write() function is strategically useful. It's easy to imagine cases when you'll be accumulating in the buffer data that isn't in the reader, for example, a separator that you'll use to separate raw data and/or fields from the reader. Moreover, the goal of this PR is to fix something and make it work.

It would be more useful to prohibit adding untested code to the codebase, in which you then easily find bug(s) upon first contact. A lot of energy gets spent on this afterwards.

Returning to the PR, I vote for keeping the write() function in this iteration, I'll add another test with read_string().

[spytheman]

Perhaps it may be clearer, to see this:

import io
import io.string_reader

struct TwoByteReader {
mut:
        data string
        pos  int
}

fn (mut r TwoByteReader) read(mut buf []u8) !int {
        if r.pos >= r.data.len {
                return io.Eof{}
        }
        min := int_min(int_min(r.data.len - r.pos, 2), buf.len)
        for i in 0 .. min {
                buf[i] = r.data[r.pos]
                r.pos++
        }
        return min
}

fn test_write_method() {
        mut two := TwoByteReader{
                data: [u8(48), 49, 50, 51, 52, 53, 54, 55, 56, 57].bytestr()
        }
        mut reader := string_reader.StringReader.new(reader: two)
        dump(reader.write([u8(70), 70, 70])!)
        dump(reader.read_bytes(1)!) // [70] on master
        dump(reader.read_bytes(1)!) // [70] on master
        dump(reader.read_bytes(1)!) // [70] on master
        dump(reader.write([u8(80), 80, 80])!) // 80 is `P`
        dump(reader.read_line()!) // `PPP0123456789` on master
        // for { dump(reader.read_bytes(1) or { break }) }
}

[spytheman]

Producing:

#0 09:46:58 ^ master ~/v>./v xx_test.v
[xx_test.v:27] reader.write([u8(70), 70, 70])!: 3
[xx_test.v:28] reader.read_bytes(1)!: [70]
[xx_test.v:29] reader.read_bytes(1)!: [70]
[xx_test.v:30] reader.read_bytes(1)!: [70]
[xx_test.v:31] reader.write([u8(80), 80, 80])!: 3
[xx_test.v:32] reader.read_line(io.BufferedReadLineConfig{....})!: PPP0123456789
#0 09:47:00 ^ master ~/v>
#0 09:47:01 ^ master ~/v>
#0 09:47:01 ^ master ~/v>git switch -
Switched to branch 'write'
#0 09:47:02 ^ write ~/v>
#0 09:47:03 ^ write ~/v>
#0 09:47:03 ^ write ~/v>./v xx_test.v
[xx_test.v:27] reader.write([u8(70), 70, 70])!: 3
[xx_test.v:28] reader.read_bytes(1)!: [48]
[xx_test.v:29] reader.read_bytes(1)!: [49]
[xx_test.v:30] reader.read_bytes(1)!: [50]
[xx_test.v:31] reader.write([u8(80), 80, 80])!: 3
[xx_test.v:32] reader.read_line(io.BufferedReadLineConfig{....})!: 3456789

[spytheman]

@Casper64 can you please clarify the intent for the module/method, and what is the desired behavior? Unfortunately it is not imported by anything in vlib or the examples, so I can not infer the usecases :-| .