Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: ability to read a line into a passed &mut String #59

Open
Lokathor opened this issue Jun 25, 2019 · 9 comments
Open

Request: ability to read a line into a passed &mut String #59

Lokathor opened this issue Jun 25, 2019 · 9 comments

Comments

@Lokathor
Copy link

I'd like the ability to provide a &mut String value to something similar to Interface::read_line and then have it write the line message into there, instead of allocating a new String each time.

@murarth
Copy link
Owner

murarth commented Jun 25, 2019

I understand that this is the convention applied to line-reading operations on Stdin and BufRead, but I don't think this is a good fit for linefeed.

The primary reason being that the result of Interface::read_line is an enum with three variants, only one of which contains text input. Therefore, a successful call to read_line would not necessarily fill a string buffer with input. The method signature in this case would be misleading.

@Lokathor
Copy link
Author

Right, I'm not suggesting that you alter the existing read_line in any way. Sorry if that was unclear.

Instead, I'd like a separate method that can exist in parallel. Something like this

pub fn read_to_string(&self, &mut String) -> Result<ReadToResult> { ... }

pub enum ReadToResult {
    Eof,
    // the data is already in the String, so we just have an empty variant here
    Success,
    Signal(Signal),
}

@murarth
Copy link
Owner

murarth commented Jun 25, 2019

Whether it's a change to read_line or a new method, the issue remains that it's misleading to users. The buffer is always available after a call to the method, but its contents do not always represent the resulting user input. One could mistakenly ignore the ReadToResult value and treat an empty string buffer as end-of-file. The similarity to Stdin and BufRead methods contributes to this possibility of confusion.

@Lokathor
Copy link
Author

Sure, that's a general danger with methods that allow people to provide a buffer, they might ignore the output. However, since we specifically have a warning against that in Rust ("warning: unused must_use"), I think it's reasonable to allow people to use that style API if they want.

Basically, the library should offer a path where the library user allocates the memory and the library just does stuff with the memory given.

@Evrey
Copy link

Evrey commented Jun 26, 2019

@murarth I'd like to point you at the io::Read trait in Rust, which is able to do what @Lokathor wants: Fill a preëxisting buffer.

  • Read::read fills a buffer slice and returns a result. On success, the buffer is mutated. On failure it may or may not be partially filled with garbage.
  • Read::read_vectored fills a bunch of I/O buffers that are given mutably by the user.
  • Read::read_to_end fills a preëxisting Vec<u8> that is given by the user, analogous to the wish here to fill a given String. Again, a Result is returned and the buffer may contain garbage partial data in the Err case.
  • Read::read_to_string does read_to_end, but as UTF-8 text into a borrowed-from-the-user String. Hmm…
  • Read::read_exact fills a slice again, but this time with an expected amount of data.

That's all reading functions. There is no function in io::Read that takes nothing and returns a secretly allocated buffer. So I'd argue that…

The primary reason being that the result of Interface::read_line is an enum with three variants, only one of which contains text input. Therefore, a successful call to read_line would not necessarily fill a string buffer with input. The method signature in this case would be misleading.

… is either not misleading at all, or that whoever might be mislead by that is also not a user of I/O in Rust and therefore wouldn't use this crate.

@murarth
Copy link
Owner

murarth commented Jun 26, 2019

@Evrey: I am quite familiar with the io::Read trait, thank you. I acknowledged the similarity of this request to methods of standard types in my first comment on this issue.

In my most recent comment, I made the point that this similarity is itself a possible source of confusion. Allow me to demonstrate what I mean by that.

This is a valid and correct usage of Stdin::read_line:

let mut buf = String::new();

loop {
    stdin().read_line(&mut buf)?;

    if buf.is_empty() {
        break;
    }

    process_input(&buf);

    buf.clear();
}

If one were to naively substitute Interface::read_line for Stdin::read_line, they would have a program that appeared to function correctly, but would exit prematurely if the user entered an empty prompt or input a signal-inducing character sequence. Requiring the user to inspect the result of Interface::read_line prevents this kind of mistake.

@Lokathor
Copy link
Author

For the version where you read into a buffer, you can move the Signal and EOF outputs into the Err side of the result, and then when they happen the user won't think that things are 100% fine. They'll inspect the error and then see what's up.

@murarth
Copy link
Owner

murarth commented Jun 27, 2019

Using the Err variant to indicate signal or EOF would be inconsitent with the existing Interface::read_line method.

@Lokathor
Copy link
Author

So, is there any way for there to be a low-allocation-chance version of reading in a line to an existing String?

(I understand that of course if the input is very long it would realloc the String as it pushes in characters).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants