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

Ark: Break up long inputs #4745

Closed
lionel- opened this issue Sep 19, 2024 · 1 comment
Closed

Ark: Break up long inputs #4745

lionel- opened this issue Sep 19, 2024 · 1 comment
Assignees
Labels
area: console Issues related to Console category. area: kernels Issues related to Jupyter kernels and LSP servers lang: r

Comments

@lionel-
Copy link
Contributor

lionel- commented Sep 19, 2024

Joint work with @DavisVaughan.
Parent issue: #1326

When a code execution request is sent to Ark, it sends it to R via the ReadConsole() hook. This hook takes input via a buffer that has fixed size of 4096 bytes: https://github.com/r-devel/r-svn/blob/08656ceb6a8c0b6fd31f436a16cea03fb614327a/src/include/Defn.h#L1896

When the input exceeds that size there is no straightforward way to break it up from our read_console() handler. Our previous solutions to this problem all had issues:

So instead we are going to change read_console() to break up input into multiple lines that will be sent one by one to R. With multi-line expressions, the input will be incomplete until the last line is reached and R calls us back for the next input. This turns read_console() into a simple state machine: If a line of input is pending, send it right away. Otherwise, proceed as normal.

For simplicity we would prefer to not use the input boundaries routine implemented in posit-dev/ark#522. Instead we'll send the lines one by one. We can depend on the fact that if we get a prompt of type "incomplete", it means the previous line completed an expression that the R evaluator could interpret.

Things that require care:

  • When an error occurs, discard the remaining lines of input. This is similar to what RStudio does. When the frontend breaks up multiple expressions (Frontend should break up multiline selections that get sent to Console (by expression or by \n) #1326), it will have to do the same for consistency.

  • When we get into the debug browser, just continue sending inputs? Again this is similar to RStudio behaviour. This is different to R's own behaviour when it gets multiple expressions as once because it runs nested REPLS. This means there is a stack of input buffers for each nested console. Achieving the same in Positron would be challenging for little benefit.

    Alternatively we could discard the pending inputs like we do in case of error. It's unlikely the pending expressions will make sense in the debug context. However the user might wonder where did the pending inputs go.

  • There might be multiple expressions in a single input. For instance if a Jupyter chunk has multiple expressions, or when a selection is executed from Positron since we don't break up expressions yet (see Frontend should break up multiline selections that get sent to Console (by expression or by \n) #1326).

    When that is the case, the results of intermediate expressions should be emitted immediately to IOPub as Stdout stream to preserve the correct order of output lines. Only the result of the final expression should be emitted as an ExecuteResult on Shell.

  • We need to trim trailing whitespace so that the last complete expression is not treated as an intermediate input.

  • If we rely on an incomplete prompt type to determine our state, we should make sure the prompt type is correct. I think we could improve https://github.com/posit-dev/ark/blob/a0c4890e2e52a670f629ba12c17a638c258c3a52/crates/ark/src/interface.rs#L781 with:

    let matches_continuation = prompt == continuation_prompt;
    let incomplete = matches_continuation && (n_frame == 0 || browser);

    In conjunction with a fix for Unreliable readline prompt detection with active browser sessions #4742.

@testlabauto
Copy link
Contributor

Verified Fixed

Positron Version(s) : 2024.11.0-13
OS Version          : OSX

Test scenario(s)

Verified with very large block of:

This is a long comment that takes a lot of space. It will be duplicated many times.

Link(s) to TestRail test cases run or created:

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: console Issues related to Console category. area: kernels Issues related to Jupyter kernels and LSP servers lang: r
Projects
None yet
Development

No branches or pull requests

3 participants