Conversation
Running a cell that leaves a block unclosed -- function x=f(y) with no
endfunction, an if/for/while/select/try with no matching end -- left
Scilab sitting at its continuation prompt waiting for the rest of the
block, and the kernel just hung. metakernel's REPLWrapper does detect
this (fast, well under a second) and has a built-in recovery: send
Ctrl-C, wait for a normal prompt, then raise a clean, catchable
ValueError. But scilab-adv-cli does not respond to SIGINT while
waiting for more input, so that recovery wait always ran out its full
budget (pexpect's default of 30s) before giving up -- so every
incomplete cell cost a good 30 seconds before showing a generic
"Timed out" error, though the session did eventually recover on its
own (confirmed: the next cell executes normally afterward).
Fixed with a small REPLWrapper subclass (_ScilabREPLWrapper) that
overrides just the continuation-prompt recovery step. A bare "end"
reliably closes any Scilab block type (function/if/for/while/...) and
returns to the top-level prompt in well under a second -- verified
directly against a live Scilab process for each block type. Nested
unclosed blocks (e.g. an unclosed if containing an unclosed for) need
one "end" per level, so it's sent repeatedly (capped at 50 attempts)
until a normal prompt reappears, matching what a user closing the
blocks by hand would have to type anyway.
Everything else about metakernel's flow is unchanged -- the "soft
continuation" empty-line retry, the eventual ValueError, and its
message ("Continuation prompt found - input was incomplete:\n<code>")
are all metakernel's own, already well-suited to surfacing a clear
error to the user; this only fixes the part that was actually broken
for Scilab specifically.
Verified against a live kernel: unclosed function/if/for now each
report the error in ~0.2-0.3s (down from 30s+), a nested unclosed
if+for needs and gets two "end"s and also recovers in ~0.3s, normal
complete multi-line blocks (if/end, function/endfunction, for/end)
are unaffected and execute correctly, and the session remains fully
functional for subsequent cells in every case.
Also includes the same pre-existing ruff cleanup as Calysto#58/Calysto#59 (this
branch was cut from master, which still has that debt).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6 tasks done
Contributor
Author
This file contains hidden or 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
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.
Summary
Running a cell that leaves a block unclosed —
function x=f(y)with noendfunction, anif/for/while/select/trywith no matchingend— left Scilab sitting at its continuation prompt waiting for the rest of the block, and the kernel just hung for a long time.metakernel'sREPLWrapperactually detects this already (fast, well under a second) and has a built-in recovery path: send Ctrl-C, wait for a normal prompt, then raise a clean, catchableValueError. Butscilab-adv-clidoes not respond to SIGINT while waiting for more input, so that recovery wait always ran out its full budget (pexpect's default of 30s) before giving up — so every incomplete cell cost a good ~30 seconds before showing a generic "Timed out" error, even though the session did eventually recover on its own (confirmed: the next cell executes normally afterward).Fixed with a small
REPLWrappersubclass (_ScilabREPLWrapper) that overrides just the continuation-prompt recovery step. A bareendreliably closes any Scilab block type (function/if/for/while/...) and returns to the top-level prompt in well under a second — verified directly against a live Scilab process for each block type. Nested unclosed blocks (e.g. an unclosedifcontaining an unclosedfor) need oneendper level, so it's sent repeatedly (capped at 50 attempts) until a normal prompt reappears — matching what a user closing the blocks by hand would have to type anyway.Everything else about
metakernel's flow is unchanged — the "soft continuation" empty-line retry, the eventualValueError, and its message ("Continuation prompt found - input was incomplete:\n<code>") are allmetakernel's own, already well-suited to surfacing a clear error to the user; this only fixes the part that was actually broken for Scilab specifically.Also includes the same pre-existing ruff cleanup as #58/#59 (this branch was cut from
master, which still has that debt).Test plan
function/if/fornow each report the error in ~0.2-0.3s (down from 30s+).if+forneeds and gets twoends, also recovering in ~0.3s.if/end,function/endfunction,for/end) are unaffected and execute correctly.disp(...)and%plotboth working right after).ruff check .andpy_compileclean on all changed files.🤖 Generated with Claude Code