From c9f4c1d972133bd7a937efd53d3aa80203ab9bc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Mottelet?= Date: Thu, 24 Sep 2026 10:16:16 +0200 Subject: [PATCH] Fix tab completion: quoted output broke matches, plus ruff cleanup 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 #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 --- scilab_kernel/check.py | 13 ++--- scilab_kernel/kernel.py | 82 ++++++++++++++++++------------ scilab_kernel/magics/plot_magic.py | 2 + test_scilab_kernel.py | 6 ++- 4 files changed, 63 insertions(+), 40 deletions(-) diff --git a/scilab_kernel/check.py b/scilab_kernel/check.py index d01362e..798bde5 100644 --- a/scilab_kernel/check.py +++ b/scilab_kernel/check.py @@ -1,18 +1,19 @@ import sys + from metakernel import __version__ as mversion + from . import __version__ from .kernel import ScilabKernel - if __name__ == "__main__": - print('Scilab kernel v%s' % __version__) - print('Metakernel v%s' % mversion) - print('Python v%s' % sys.version) - print('Python path: %s' % sys.executable) + print(f'Scilab kernel v{__version__}') + print(f'Metakernel v{mversion}') + print(f'Python v{sys.version}') + print(f'Python path: {sys.executable}') print('\nConnecting to Scilab...') try: s = ScilabKernel() print('Scilab connection established') print(s.banner) - except Exception as e: + except Exception as e: # noqa: BLE001 -- diagnostic script: report any failure, don't crash with a raw traceback print(e) diff --git a/scilab_kernel/kernel.py b/scilab_kernel/kernel.py index c07315b..4fdcc66 100644 --- a/scilab_kernel/kernel.py +++ b/scilab_kernel/kernel.py @@ -1,25 +1,32 @@ -from __future__ import print_function, absolute_import import codecs +import importlib import json import os +import platform import re import shutil +import subprocess import sys -import platform import tempfile -import importlib -import subprocess +from typing import ClassVar + if importlib.util.find_spec('winreg'): import winreg from xml.dom import minidom +from xml.parsers.expat import ExpatError +from IPython.display import SVG, Image from metakernel import MetaKernel, ProcessMetaKernel, REPLWrapper, pexpect from metakernel.pexpect import which -from IPython.display import Image, SVG from . import __version__ +# Scilab's terminal control codes (mode/cursor resets, backspace) end up +# glued to the front of the *first* line of any console output -- used to +# strip those out of completion results, see get_completions() below. +_ANSI_ESCAPE_RE = re.compile(r'\x1b\[[0-9;]*[A-Za-z]|\x08') + def get_kernel_json(): """Get the kernel json for the kernel. @@ -37,7 +44,7 @@ class ScilabKernel(ProcessMetaKernel): implementation_version = __version__, language = 'scilab' language_version = __version__, - language_info = { + language_info: ClassVar[dict] = { 'name': 'scilab', 'file_extension': '.sci', "mimetype": "text/x-scilab", @@ -106,7 +113,7 @@ def _detect_executable(self): # read the windows registry if os.name == 'nt': try: - with winreg.OpenKey(winreg.HKEY_CLASSES_ROOT, "Scilab5.sce\shell\open\command") as key: + with winreg.OpenKey(winreg.HKEY_CLASSES_ROOT, r"Scilab5.sce\shell\open\command") as key: cmd : str = winreg.EnumValue(key, 0)[1] executable = cmd.split(r'"')[1].replace("wscilex.exe", "wscilex-cli.exe") self.log.warning('Windows registry binary: ' + executable) @@ -116,9 +123,9 @@ def _detect_executable(self): # detect macOS bundle if platform.system() == 'Darwin': - process = subprocess.run(['mdfind', '-onlyin', '/Applications', 'kMDItemCFBundleIdentifier=org.scilab.modules.jvm.Scilab'], - stdout=subprocess.PIPE, - universal_newlines=True) + process = subprocess.run(['mdfind', '-onlyin', '/Applications', 'kMDItemCFBundleIdentifier=org.scilab.modules.jvm.Scilab'], + stdout=subprocess.PIPE, + text=True, check=False) bundles = process.stdout if len(bundles) > 0: executable = bundles.split('\n', 1)[0] + "/Contents/bin/scilab-adv-cli" @@ -145,7 +152,7 @@ def makeWrapper(self): orig_prompt = r'-[0-9]*->' prompt_cmd = None change_prompt = None - continuation_prompt = ' \>' + continuation_prompt = r' \>' self._first = True if os.name == 'nt': prompt_cmd = 'printf("-->")' @@ -166,7 +173,7 @@ def makeWrapper(self): def Write(self, message): clean_msg = message.strip("\n\r\t") - super(ScilabKernel, self).Write(clean_msg) + super().Write(clean_msg) def Print(self, text): text = str(text).strip('\x1b[0m').replace('\u0008', '').strip() @@ -174,7 +181,7 @@ def Print(self, text): if (not line.startswith(chr(27)))] text = '\n'.join(text) if text: - super(ScilabKernel, self).Print(text) + super().Print(text) def do_execute_direct(self, code, silent=False): if self._first: @@ -182,7 +189,7 @@ def do_execute_direct(self, code, silent=False): self.handle_plot_settings() setup = self._setup.strip() self.do_execute_direct(setup, True) - resp = super(ScilabKernel, self).do_execute_direct(code, silent=silent) + resp = super().do_execute_direct(code, silent=silent) if silent: return resp if self.plot_settings.get('backend', None) == 'inline': @@ -198,22 +205,33 @@ def get_kernel_help_on(self, info, level=0, none_on_fail=False): return None else: return "" - self.do_execute_direct('help %s' % obj, True) + self.do_execute_direct(f'help {obj}', True) def do_shutdown(self, restart): self.wrapper.sendline('quit') - super(ScilabKernel, self).do_shutdown(restart) + super().do_shutdown(restart) def get_completions(self, info): """ Get completions from kernel based on info dict. """ - cmd = 'completion("%s")' % info['obj'] + obj = info['obj'] + # completion() displays its result on Scilab's own console, quoted + # (Scilab now shows string arrays as `"a" "b" ...`), which broke + # the parsing below; printf("%s\n", ...) instead prints each match + # bare, one per line. Only printf when there is at least one match: + # it errors out on an empty array. Both calls run in the same + # persistent Scilab session as the user's own code, so nothing here + # is assigned to a variable that could shadow one of theirs. + cmd = ( + f'if ~isempty(completion("{obj}")) then ' + f'printf("%s\\n",completion("{obj}")); end' + ) output = self.do_execute_direct(cmd, True) if not output: return [] - output = output.output.replace('!', '') - return [line.strip() for line in output.splitlines() + text = _ANSI_ESCAPE_RE.sub('', output.output) + return [line.strip() for line in text.splitlines() if info['obj'] in line] def handle_plot_settings(self): @@ -238,17 +256,17 @@ def handle_plot_settings(self): try: width, height = settings['size'].split(',') width, height = int(width), int(height) - except Exception as e: - self.Error('Error setting plot settings: %s' % e) + except (ValueError, AttributeError) as e: + self.Error(f'Error setting plot settings: {e}') - cmds.append('h.figure_size = [%s,%s];' % (width, height)) - cmds.append('h.axes_size = [%s * 0.98, %s * 0.8];' % (width, height)) + cmds.append(f'h.figure_size = [{width},{height}];') + cmds.append(f'h.axes_size = [{width} * 0.98, {height} * 0.8];') if settings['backend'] == 'inline': cmds.append('h.visible = "off";') else: cmds.append('h.visible = "on";') - super(ScilabKernel, self).do_execute_direct('\n'.join(cmds), True) + super().do_execute_direct('\n'.join(cmds), True) def make_figures(self, plot_dir=None): """Create figures for the current figures. @@ -267,7 +285,7 @@ def make_figures(self, plot_dir=None): plot_format = self._plot_fmt.lower() make_figs = '_make_figures("%s", "%s");' make_figs = make_figs % (plot_dir, plot_format) - super(ScilabKernel, self).do_execute_direct(make_figs, True) + super().do_execute_direct(make_figs, True) return plot_dir def extract_figures(self, plot_dir): @@ -291,7 +309,7 @@ def extract_figures(self, plot_dir): if self.error_handler: self.error_handler(e) else: - raise e + raise return images def _handle_svg(self, filename): @@ -305,14 +323,14 @@ def _handle_svg(self, filename): im = SVG(data=data) try: im.data = self._fix_svg_size(im.data) - except Exception: - pass + except (ValueError, ExpatError) as e: + self.log.debug(f'Could not resize SVG (unexpected shape from GnuPlot?): {e}') try: settings = self.plot_settings if settings['antialiasing']: im.data = self._fix_svg_antialiasing(im.data) - except Exception: - pass + except (ValueError, ExpatError) as e: + self.log.debug(f'Could not adjust SVG antialiasing (unexpected shape from GnuPlot?): {e}') return im def _fix_svg_size(self, data): @@ -340,8 +358,8 @@ def _fix_svg_size(self, data): width = width * settings['height'] / height height = settings['height'] - svg.setAttribute('width', '%dpx' % width) - svg.setAttribute('height', '%dpx' % height) + svg.setAttribute('width', f'{int(width)}px') + svg.setAttribute('height', f'{int(height)}px') return svg.toxml() def _fix_svg_antialiasing(self, data): diff --git a/scilab_kernel/magics/plot_magic.py b/scilab_kernel/magics/plot_magic.py index 3019afd..7bedef9 100644 --- a/scilab_kernel/magics/plot_magic.py +++ b/scilab_kernel/magics/plot_magic.py @@ -1,4 +1,6 @@ from metakernel import Magic, option + + class ScilabPlotMagic(Magic): @option( diff --git a/test_scilab_kernel.py b/test_scilab_kernel.py index 7747ad8..26338d7 100644 --- a/test_scilab_kernel.py +++ b/test_scilab_kernel.py @@ -1,6 +1,8 @@ """Example use of jupyter_kernel_test, with tests for IPython.""" import unittest +from typing import ClassVar + import jupyter_kernel_test as jkt @@ -11,12 +13,12 @@ class ScilabKernelTests(jkt.KernelTests): code_hello_world = "disp('hello, world')" - code_display_data = [ + code_display_data: ClassVar[list] = [ {'code': '%plot -f png\nplot([1,2,3])', 'mime': 'image/png'}, {'code': '%plot -f svg\nplot([1,2,3])', 'mime': 'image/svg+xml'} ] - completion_samples = [ + completion_samples: ClassVar[list] = [ { 'text': 'one', 'matches': {'ones'},