Factor out handle_active_request() to correct multiline input request issue
#568
Conversation
| // Upon entering read-console, finalize any debug call text that we were capturing. | ||
| // At this point, the user can either advance the debugger, causing us to capture | ||
| // a new expression, or execute arbitrary code, where we will reuse a finalized | ||
| // debug call text to maintain the debug state. | ||
| self.dap.finalize_call_text(); |
There was a problem hiding this comment.
I'm not entirely sure if it matters that this now runs before pending lines are handled 🤔
There was a problem hiding this comment.
hmm, I think it's alright but we should only finalize capture if in a browser prompt? The only edge case I can think of is with adversarial readline prompts so this does not matter much I guess.
There was a problem hiding this comment.
LGTM! It now makes sense to me to do that since input requests are nested in execute-requests (posit-dev/positron#4901 (comment)).
| // Upon entering read-console, finalize any debug call text that we were capturing. | ||
| // At this point, the user can either advance the debugger, causing us to capture | ||
| // a new expression, or execute arbitrary code, where we will reuse a finalized | ||
| // debug call text to maintain the debug state. | ||
| self.dap.finalize_call_text(); |
There was a problem hiding this comment.
hmm, I think it's alright but we should only finalize capture if in a browser prompt? The only edge case I can think of is with adversarial readline prompts so this does not matter much I guess.
…st issue The ordering is now: - Handle input requests - Then pending lines - Then close out active requests
DavisVaughan
force-pushed
the
fix/input-request-in-multiline
branch
from
25fc2b1
to
ad0fb43
Compare
Addresses posit-dev/positron#4901
The problem was that in something like:
if you send that whole selection, then we'd process the
val <- readline("prompt>")line, but then R callsread_console()back and we'd just immediately shovepaste0(val, "-suffix")through as the reply to the readline request! We need to use theprompt_info()to recognize that an intermediate expression has put us into aninput_requeststate, and handle that before we handlepending_lines.The ordering of our state machine is now:
I've accomplished this by factoring out
handle_active_request(). This takes that big if/else branch related to the active request and gives it its own function. The logic in that if/else was actually pretty tricky, and I think it is much cleaner now. It also allows us to sneak inhandle_pending_lines()between the input request check and closing out the active request.