Skip to content

Fix tab completion: quoted output broke matches - #59

Closed
mottelet wants to merge 1 commit into
Calysto:masterfrom
mottelet:fix-tab-completion-quoted-strings
Closed

mottelet wants to merge 1 commit into
Calysto:masterfrom
mottelet:fix-tab-completion-quoted-strings

Conversation

@mottelet

Copy link
Copy Markdown
Contributor

Summary

Tab completion in a notebook cell ran completion(\"obj\") directly and parsed Scilab's own console echo of the result. That echo used to be an unquoted, bracketed format Scilab dropped at least ~2 years ago in favor of displaying string arrays quoted ("a" "b" ...), which the parsing here was never updated for — so every completion candidate comes back wrapped in literal double quotes, and (per the original report) the old fallback also used to open the JavaHelp browser as a side effect on some versions.

Fixed by using printf("%s\n", completion(...)) instead, which prints each match bare, one per line, sidestepping Scilab's own display formatting entirely.

Two things needed care:

  • printf errors out on an empty array, so it's only called when completion() isn't empty — checked without ever assigning the result to a variable, since get_completions() runs in the same persistent Scilab session as the user's own code, and a temp variable could shadow one of theirs.
  • Scilab's terminal control codes end up glued to the front of the first line of any console output. With the old quoted/bracketed format this only ever landed on a line that got filtered out by coincidence (a substring match against info['obj']); with printf's plain output it could leak into a real match — reproduced with a single-match completion (e.g. "getscilabkeyword" returned "\x1b[4l \x08\x1b[0mgetscilabkeywords" instead of "getscilabkeywords"). Now stripped via a small regex before parsing.

Also includes the same pre-existing ruff cleanup as #58 (this branch was cut from master, which still has that debt) — CI was failing on unrelated pre-existing findings before this cleanup, same as described there.

Test plan

  • Verified against a live kernel: multi-match ("plo"), single-match ("getscilabkeyword"), and no-match ("zzzzzz") completions all return clean, unquoted results.
  • Confirmed via who_user() that nothing is left behind in the Scilab session after completion runs.
  • ruff check . clean, all files py_compile, kernel smoke-tested (disp, %plot producing real output).

🤖 Generated with Claude Code

Tab completion ran completion("obj") directly and parsed Scilab's own
console echo of the result. That echo used to be an unquoted, bracketed
format Scilab dropped at least ~2 years ago in favor of displaying
string arrays quoted (`"a"  "b"  ...`), which the parsing here was
never updated for -- so every completion candidate now comes back
wrapped in literal double quotes.

Fixed by using printf("%s\n", completion(...)) instead, which prints
each match bare, one per line, sidestepping Scilab's own display
formatting entirely (and the quoting behind whatever future formatting
changes might come). Two things needed care:

- printf errors out on an empty array, so it's now only called when
  completion() isn't empty (checked without assigning the result to a
  variable -- get_completions() runs in the same persistent Scilab
  session as the user's own code, so nothing here should risk shadowing
  a variable of theirs).
- Scilab's terminal control codes end up glued to the front of the
  *first* line of any console output. With the old quoted/bracketed
  format this only ever landed on a line that got filtered out by
  coincidence; with printf's plain output it could leak into a real
  match (reproduced with a single-match completion, e.g. "getscilabkeyword"
  -> "\x1b[4l \x08\x1b[0mgetscilabkeywords" instead of
  "getscilabkeywords"). Now stripped via a small regex before parsing.

Verified against a live kernel: multi-match, single-match, and
no-match completions all return clean results, and `who_user()`
confirms nothing gets left behind in the session.

Also includes the same pre-existing ruff cleanup as PR Calysto#58 (this
branch was cut from master, which still has it) -- see that PR's
commit message for the itemized list; same fixes, same reasoning.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mottelet

Copy link
Copy Markdown
Contributor Author

Merged into #58 so both fixes land together — closing this one in favor of that. See #58 for the combined changes and updated description.

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

Successfully merging this pull request may close these issues.

1 participant