Is position used in pipe?

[mariusGundersen]

Based on https://github.com/espruino/Espruino/pull/2350#issuecomment-1512995412 I wanted to see what it would take to implement piping of strings. But looking at the pipe code I don't really follow how it works. There is a position number on the pipe object, but it's not actually used anywhere: https://github.com/espruino/Espruino/blob/9d13de01228be301715f18e66350c3c545d3a167/src/jswrap_pipe.c#L117-L131 read is only given a chunkSize, and write is given the buffer. It seams like the readable and writable streams handle their positioning themselves.

If I understand this correctly then position and all references to it (in the error messages) can be removed.

[gfwilliams]

Yes, it does seem like position isn't needed.

However if we're adding the ability to pipe strings then maybe it would actually be needed for that?

[gfwilliams]

Looking at this, while keep position we don't actually send 3 arguments to readFunc despite saying so in the error message - it's just the one for length.

We could send the extra 3 arguments and could then implement a read function for Strings, but I wonder whether sending the extra arguments might break piping for some other stuff, where read accepts other arguments?

It looks like it might be good to update the error messages, but to keep position and then in handlePipe we could handle the special case where the source was a string, and just call substring in it ourselves, using the position?

[mariusGundersen]

I think we should instead make a wrapper object for string that can handle the reading. That way E.pipe(string, destination) could wrap the string in a Stream object or you could do new Stream(string).pipe(destination) (where Stream is just an example, not sure what would be the best name for this thing). It seems dangerous to suddenly pass position to lots of places that don't use them today and expect them to behave correctly. It would also reduce the code and memory a bit to remove it.

The Stream class doesn't need much, it could be something like this:

class Stream {
  constructor(src){
    this.src = src;
    this.position = 0;
  }
  read(length){
    return this.src.substring(this.position, Math.min(this.position += length, this.src.length));
  }
}

[gfwilliams]

Just done - that ended up being nice and easy. Not sure if there's any other object type it's worth wrapping that way too, like Arrays...