Add sync version of std/io readLines

[luvies]

It would be useful to provide a function that can read a line of input from stdin that has a simple interface. It would probably good to support both async and sync versions, something like

const name: string = await Deno.readLine('What is your name? ');
const age: string = Deno.readLineSync('What is your age? ');

I think readLine might be too close to the file functions, so maybe something like getInput or ttyReadLine

[kevinkassimo]

I believe this could be implemented in pure TS with Deno.stdin, so we should probably put this in deno_std

[johnsonjo4531]

I made an async implementation of readLine named input that uses a method lines which is an iterator that yields lines from an input source. Here's an example of it being used. I think I'll make a pull request to add it to Deno's std lib, but I think I want to make a synchronous version first. The only thing that I need first is a version of BufReader (from denoland/deno_std/io/bufio.ts) that is synchronous.

[bartlomieju]

It's now possible to do that using readLines from std/io/bufio.ts

import { readLines } from "https://deno.land/[email protected]/io/bufio.ts";

async function main() {
   for await (const line of readLines(Deno.stdin)) {
      console.log("read line", line);
  }
}

console.log("reading lines from stdin");
main();

$ deno index.ts
Compile file:///Users/biwanczuk/dev/deno/index.ts
reading lines from stdin
asdf
read line asdf
qwer
read line qwer
tryu
read line tryu

[ccheng]

I'd really appreciate having a synchronous version like johnsonjo4531 suggested. I'll just put that out there. :)

Curious if there's any interest in having a synchronous version of BufReader? To enable a synchronous implementation of readLine?

[ry]

Yes having a synchronous version would be useful. It seems to me like it would unfortunately require a lot of duplicated code...

[ccheng]

I'd like to work on this issue (synchronous version of BufReader to enable a synchronous implementation of readLine).

I actually needed this feature, so I've been toying/working on a proof of concept.

It does require a bit of duplicated code. I think there's ways to reduce the duplicate code but I'm not sure it's practical.

Thoughts?

[johnsonjo4531]

I have some thoughts on one way this could be done although I'm not sold on them either at the moment and not sure if they are practical or not as I'm not sure how far reaching changes would be for it. I recently read a tweet about this. Apparently babel/core deduplicates async and sync code with generators (reference). After some searching it appears babel uses a package called gensync. Gensync isn't a perfect fit for this scenario as far as I'm aware though, because it doesn't out of the box wrap async generators only async functions and sync functions, but I believe it might be possible to wrap the promise based api with the asyncIterator protocol.

While it might be worth it to go the generator route to not duplicate code, it does seem like it could come with more complexity and unseen caveats, and at the moment I'm not sure how much added complexity it adds because I have not ever used the library (this is my first time looking into it). I'm definitely going to look more into this library and try to report some findings. We should probably get a deno maintainer's thoughts on this matter though at the same time. Something similar to gensync could be put into deno std to help finish this if the deno team thought it would be good to go the generator route (although maybe it's best deno tries to hide the gensync implementation from the public much like babel does, so std might not be the right spot.).

You can look through some of the examples of gensync used throughout babel/core if your curious on how it would be used. Also some may want to look at the main pull-request that put gensync into babel/core, because it has added information on how it affected babel's code base.

[ccheng]

Thanks for the references! That's really interesting. Definitely would like to know deno maintainer's thoughts on this.

I wonder what their thoughts are on having duplicated code for this one feature for now, and the possibility of gensync which could be useful here and elsewhere in future refactoring and de-duplication of a/sync versions of code.

[johnsonjo4531]

I may have been a bit rash suggesting gensync so soon because I know for sure it does deduplication of sync generators to async functions, but I'm unsure how much work it would take to make gensync do sync generators to async generators deduplication, or whether that's even possible...

I'll definitely have to keep you updated if I figure out more about gensync (whether it applies well in this scenario), but as for now I think duplicating the code is probably the right choice.

[fr3fou]

what was done regarding this?

[ccheng]

Well I actually needed this feature for work, so I've done it and am using it. Went the path of having duplicated code. Don't know what deno maintainer's thoughts are on having duplicated code, but perhaps I should get motivated on making a PR so it can be discussed?

Looked at gensync possibility but don't think it'll work for a/sync functions.

[bartlomieju]

@ccheng feel free to open a PR - it's fine to duplicate code for sync version.

[fr3fou]

Seeing that the stdlib is heavily inspired by go, I'd recommend a similar approach:

scanner := bufio.NewReader(os.Stdin)
scanner.Scan() // advance in input
line := scanner.Text() // get the input

[ry]

@fr3fou Yes we ideally would have code exactly like that but in TS.

[ebebbington]

Could this not be achieved by using the new prompts feature?

[bartlomieju]

I'm still open to this feature, if anyone wants to work on it feel free to open a PR.

[ccheng]

@bartlomieju I'd appreciate some feedback or guidance, especially as it relates to this (now old) PR for this issue: denoland/deno#6171.

That PR initially had lots of duplicated code to create a sync version from the async version, but ry wasn't happy with having a bunch of very technical duplicated code .

So I tried a different approach that supported async and sync with no duplicate code. I think we were waiting for feedback on this but never heard back. The PR eventually got outdated and was closed.

It'd be nice to get some feedback on that last approach which had no duplicate code. If the approach is ok, I might try again with a new PR.

Thanks!

[kt3k]

@ccheng I saw denoland/deno#6171, but the main type ReaderMaybeSync = Reader | ReaderSync looks hard to work with / hard to reason about to my eyes. That requires as casting whenever actually do the operations. I think that is an unusual pattern of coding style.

I prefer the first version of denoland/deno#6171 much better https://github.com/denoland/deno/pull/6171/commits/3aa119c7033be9dd9964c809a705905f6f0c6c5b

What do you think? @bartlomieju @ry

[bartlomieju]

@kt3k I agree with you, in this case duplicated code is better than trying to get it done in a single function.

[iuioiua]

Is this something we'd still like to have? If so, I can give it a crack.

[kt3k]

For I/O, we're moving from Reader/Writer interface to Web Stream based interface. So, in my view, these std/io related issues are half deprecated.

(BTW I think we've never discussed about sync I/O in web stream based world)

@bartlomieju What is your opinion on this?

[iuioiua]

For I/O, we're moving from Reader/Writer interface to Web Stream based interface. So, in my view, these std/io related issues are half deprecated.

I've seen mention of that. Why's that? Just web compatibility?

[kt3k]

I've seen mention of that. Why's that? Just web compatibility?

Mostly yes. Web Stream is already part of basic builtin Web APIs such as fetch (the response body is web stream). So we can't drop Web Stream from Deno. And having two separate I/O system in a single runtime feel confusing and redundant. So we decided to drop Reader/Writer type I/O

[iuioiua]

Is sync I/O even possible using web streams?

[crowlKats]

no, web streams are async

[kt3k]

Transition from Reader/Writer to Web Stream was discussed, for example, in this issue https://github.com/denoland/deno/issues/9795

Maybe we were too less concerned about what happens with sync I/O?

[kt3k]

Is it possible to extend Web Streams API to have synchronous reader/writer?

[crowlKats]

no, as they are promise based on a very deep level; web streams spec adding sync capabilities sounds extremely unlikely

[iuioiua]

Perhaps, instead of being deprecated, std/io can be repurposed to take care of those instances where sync I/O is needed.

[iuioiua]

It's been over a year since any sentiment in support of this functionality has been expressed. Also, Deno's shifting its focus to streams-based IO.

I think it's safe to say that this feature is longer desired, and this issue can be closed.