Avoid crashing when given input longer than R's buffer #202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lionel-
approved these changes
Jan 11, 2024
|
|
||
| if input.len() > buflen { | ||
| let dropped = &input[buflen..input.len()]; | ||
| log::error!("Console input too large for buffer. Input has been trimmed, dropping: '{dropped}'."); |
There was a problem hiding this comment.
Is this a case where we also want to log in the user console? Not important though, just a thought about an additional error log level that we might want to implement someday.
There was a problem hiding this comment.
Or maybe a toast message? Would require some kind of ark -> frontend notification system, maybe we already have that in place for R_ShowMessage(), but idk if it works properly.
Regardless, I'm very optimistic that this will be a non issue when the frontend splits large inputs up.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses posit-dev/positron#1767
Which is this TODO left for ark posit-dev/positron#1326 (comment)
Ideally the frontend will break long inputs up for us, making this redundant, but until then we need to at least avoid crashing.
I've taken the same approach as RStudio, which is to just trim the input if it exceeds the buffer size. This isn't ever a practical issue in RStudio because it seems to split large chunks of input by newlines, and sends one line at a time to R's buffer.
Our previous behavior of just returning without copying anything into the buffer wasn't working, and would result in a crash, presumably because R was expecting something to get copied into the buffer after a read-console iteration.