From 82c7617ddaae4379916757a1a007f603b1f22af7 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Mon, 21 Sep 2026 00:59:36 +0800 Subject: [PATCH 1/5] Install as a kconfiglib package, not loose modules Installing dropped fifteen unprefixed modules straight into site- packages: kconfiglib.py next to menuconfig.py, rawterm.py, and a defconfig.py and setconfig.py that any other distribution is free to want for itself. A port maintainer packaging this for FreeBSD hit it. The sources stay flat in the checkout, so running python menuconfig.py from a clone keeps working and the README links still point at files that exist. build_py maps them into the package at build time instead. Globbing the repository root would have swept up setup.py and lint.py, so the module list is explicit. Editable installs default to strict mode, because the lenient one maps the package name at the root where there is no __init__.py to find, but an explicit editable_mode is left alone. This breaks importing menuconfig from an installed Kconfiglib, so the version goes to 15.0.0. Importing it from the kconfiglib package replaces that. Nothing changes for code that only imports kconfiglib itself. One wart comes with keeping the layout: the root kconfiglib.py shadows the installed package whenever the checkout is the working directory, which the README now says and the CI check works around by testing from elsewhere. The sdist also grows the whole tests tree. It shipped the test modules but not conftest.py, the helpers or the Kconfig fixtures, so the suite could not run from the tarball at all, which is what a packager building from it needs. --- .github/workflows/package.yml | 112 +++++++++++++++++++++++++++++++ MANIFEST.in | 6 ++ README.md | 9 ++- kconfiglib.py | 2 +- menuconfig.py | 8 ++- pyproject.toml | 4 ++ setup.py | 122 +++++++++++++++++++++++++--------- 7 files changed, 228 insertions(+), 35 deletions(-) diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 0c48716..673d0f3 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -51,6 +51,118 @@ jobs: set -euo pipefail python setup.py bdist_wheel + - name: Check wheel layout + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + from zipfile import ZipFile + + # The wheel ships a kconfiglib package, not fifteen loose modules in + # site-packages. Nothing outside kconfiglib/ but metadata. + # + # That everything the console scripts name is present, importable and + # has the named attribute is checked by resolving them for real in the + # next step, which is strictly stronger than matching paths here. + wheel = next(Path("dist").glob("*.whl")) + with ZipFile(wheel) as archive: + names = set(archive.namelist()) + + stray = sorted( + name + for name in names + if not name.startswith("kconfiglib/") and ".dist-info/" not in name + ) + assert not stray, f"unexpected wheel members: {stray}" + assert "kconfiglib/__init__.py" in names + + print(f"{len(names)} members, nothing outside the package") + PY + + - name: Check wheel install + run: | + set -euo pipefail + python -m venv /tmp/wheel-env + /tmp/wheel-env/bin/pip install dist/*.whl + # The wheel's own list of console scripts, for the check below + python - <<'PY' + from pathlib import Path + from zipfile import ZipFile + + wheel = next(Path("dist").glob("*.whl")) + with ZipFile(wheel) as archive: + name = next( + n for n in archive.namelist() + if n.endswith(".dist-info/entry_points.txt") + ) + Path("/tmp/declared.txt").write_bytes(archive.read(name)) + PY + export DECLARED=/tmp/declared.txt + # Not from the checkout: the root kconfiglib.py shadows the installed + # package whenever it is the working directory. + cd "$(mktemp -d)" + /tmp/wheel-env/bin/python - <<'PY' + import importlib + import os + from importlib.metadata import entry_points + from pathlib import Path + + import kconfiglib + + assert kconfiglib.Kconfig + + # Resolve every console script the way the generated wrapper does, so + # a module that is present but broken, or a renamed main(), fails here + # rather than the first time a user runs it. + # + # Selected by module prefix rather than asserted over every script in + # the environment, which would also cover pip's, and rather than via + # distribution("kconfiglib"), which picks whichever metadata directory + # comes first when a stray egg-info is on the path. + installed = { + e.name: e + for e in entry_points(group="console_scripts") + if e.module.startswith("kconfiglib.") + } + + # Against the wheel's own declaration, so that dropping a script + # still fails here rather than passing because the others survived. + declared = { + line.split("=", 1)[0].strip() + for line in Path(os.environ["DECLARED"]).read_text().splitlines() + if "=" in line and not line.startswith("[") + } + assert declared, "the wheel declares no console scripts" + assert set(installed) == declared, ( + f"declared {sorted(declared)}, installed {sorted(installed)}" + ) + installed = list(installed.values()) + + for entry in installed: + if entry.module == "kconfiglib.guiconfig": + try: + import tkinter # noqa: F401 + except ImportError: + print(f"no tkinter, skipping {entry.name}") + continue + assert callable(entry.load()), entry.name + + # rawterm ships no script of its own + importlib.import_module("kconfiglib.rawterm") + + print(f"{len(installed)} console scripts resolve") + PY + /tmp/wheel-env/bin/genconfig --help > /dev/null + + - name: Check editable install + run: | + set -euo pipefail + python -m pip install --editable . + # The root kconfiglib.py shadows the package whenever the checkout is + # the working directory, so verify the install from somewhere else. + cd "$(mktemp -d)" + python -c "from kconfiglib import Kconfig; import kconfiglib.menuconfig" + - name: List built artifacts run: | set -euo pipefail diff --git a/MANIFEST.in b/MANIFEST.in index 1dad430..a8cb998 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,2 +1,8 @@ # Include the license file in source distributions include LICENSE + +# Ship the whole test suite, not just test_*.py: conftest.py, the helper +# modules and the Kconfig fixtures are what make it runnable for a distro +# packager building from the sdist. +recursive-include tests * +global-exclude __pycache__/* *.pyc diff --git a/README.md b/README.md index c684dca..82d6a58 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,13 @@ pip install git+https://github.com/sysprog21/Kconfiglib Microsoft Windows is supported. +An installed Kconfiglib is a single `kconfiglib` package, so the interfaces are +imported as `from kconfiglib import menuconfig` rather than `import menuconfig` +(the latter worked before 15.0.0, when every module landed loose in +site-packages). The sources stay flat in a checkout, which means a bare +`kconfiglib.py` in the working directory shadows the installed package there: +run `import kconfiglib.menuconfig` from somewhere other than the source tree. + When installed via `pip`, you get both the core library and the following executables. All but three (`genconfig`, `setconfig`, and `lint`) mirror functionality available in the C tools. - [menuconfig](menuconfig.py) @@ -177,7 +184,7 @@ This will work even after installing Kconfiglib with `pip`. Documentation for other modules can be viewed the same way. For executables, a plain `--help` often suffices: ```shell -pydoc menuconfig/guiconfig/... +pydoc kconfiglib.menuconfig kconfiglib.guiconfig ``` A good place to start is the module docstring, located at the beginning of [kconfiglib.py](kconfiglib.py). diff --git a/kconfiglib.py b/kconfiglib.py index 09db084..f49defc 100644 --- a/kconfiglib.py +++ b/kconfiglib.py @@ -634,7 +634,7 @@ def my_other_fn(kconf, name, arg_1, arg_2, ...): from glob import iglob from os.path import dirname, exists, expandvars, isabs, islink, join, realpath -VERSION = (14, 1, 0) +VERSION = (15, 0, 0) # Record types for the location-bearing Symbol/Choice properties. These are # tuple subclasses, so existing positional unpacking and indexing keep working diff --git a/menuconfig.py b/menuconfig.py index 7f6f6ad..af13301 100755 --- a/menuconfig.py +++ b/menuconfig.py @@ -196,8 +196,12 @@ import re import textwrap -import rawterm -from rawterm import Key, Box, Style, Color, NAMED_COLORS +if __package__: + from . import rawterm + from .rawterm import Key, Box, Style, Color, NAMED_COLORS +else: + import rawterm + from rawterm import Key, Box, Style, Color, NAMED_COLORS from kconfiglib import ( Symbol, diff --git a/pyproject.toml b/pyproject.toml index 27f6f85..177bcbf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,7 @@ +[build-system] +requires = ["setuptools>=64"] +build-backend = "setuptools.build_meta" + [tool.ruff] # Minimum supported runtime is Python 3.8 (see setup.py python_requires). target-version = "py38" diff --git a/setup.py b/setup.py index 7fdbe71..98077e0 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,80 @@ import os import setuptools +from setuptools.command.build_py import build_py + +try: + from setuptools.command.editable_wheel import editable_wheel +except ImportError: + # PEP 660 support arrived in setuptools 64. pyproject.toml pins that floor + # for PEP 517 builds; a legacy "python setup.py sdist/bdist_wheel" against + # an older setuptools still works, it just cannot install editable. + editable_wheel = None + +_PACKAGE = "kconfiglib" + +_MODULES = ( + _PACKAGE, + "rawterm", + "menuconfig", + "guiconfig", + "genconfig", + "oldconfig", + "olddefconfig", + "savedefconfig", + "defconfig", + "alldefconfig", + "allnoconfig", + "allmodconfig", + "allyesconfig", + "listnewconfig", + "setconfig", +) + + +class _BuildPy(build_py): + # The sources live flat in the repository root so that "python menuconfig.py" + # keeps working from a checkout. Map them into the package explicitly rather + # than globbing the root, which would sweep in setup.py, lint.py and friends. + def find_package_modules(self, package, package_dir): + if package != _PACKAGE: + return super().find_package_modules(package, package_dir) + return [ + ( + package, + "__init__" if module == _PACKAGE else module, + os.path.join(package_dir, module + ".py"), + ) + for module in _MODULES + ] + + +_CMDCLASS = {"build_py": _BuildPy} + +if editable_wheel is not None: + + class _EditableWheel(editable_wheel): + def finalize_options(self): + super().finalize_options() + # Lenient and compat both bypass the source-to-package output + # mapping above: they point the package name at the project root, + # where kconfiglib.py is a module rather than a package, so + # importing a submodule fails. Strict is the only mode this + # layout can honour, so fill it in as the default and say so + # rather than silently substituting when one of the others was + # asked for. + if self.mode is None: + self.mode = "strict" + elif self.mode.lower() != "strict": + raise SystemExit( + f"editable_mode={self.mode} cannot work while the sources " + "live flat in the project root: it maps kconfiglib at the " + "root, where kconfiglib.py shadows the package. Use " + "editable_mode=strict." + ) + + _CMDCLASS["editable_wheel"] = _EditableWheel + # Make sure that README.md decodes in environments that use the C locale # (which implies ASCII), by explicitly giving the encoding. @@ -11,7 +85,7 @@ setuptools.setup( name="kconfiglib", # MAJOR.MINOR.PATCH, per http://semver.org - version="14.1.1a4", + version="15.0.0", description="A flexible Python Kconfig implementation", long_description=long_description, url="https://github.com/sysprog21/Kconfiglib", @@ -19,38 +93,24 @@ author_email="ci@zephyrproject.org", keywords="kconfig, kbuild, menuconfig, configuration-management", license="ISC", - py_modules=( - "kconfiglib", - "rawterm", - "menuconfig", - "guiconfig", - "genconfig", - "oldconfig", - "olddefconfig", - "savedefconfig", - "defconfig", - "alldefconfig", - "allnoconfig", - "allmodconfig", - "allyesconfig", - "listnewconfig", - "setconfig", - ), + packages=(_PACKAGE,), + package_dir={_PACKAGE: "."}, + cmdclass=_CMDCLASS, entry_points={ "console_scripts": ( - "menuconfig = menuconfig:_main", - "guiconfig = guiconfig:_main", - "genconfig = genconfig:main", - "oldconfig = oldconfig:_main", - "olddefconfig = olddefconfig:main", - "savedefconfig = savedefconfig:main", - "defconfig = defconfig:main", - "alldefconfig = alldefconfig:main", - "allnoconfig = allnoconfig:main", - "allmodconfig = allmodconfig:main", - "allyesconfig = allyesconfig:main", - "listnewconfig = listnewconfig:main", - "setconfig = setconfig:main", + "menuconfig = kconfiglib.menuconfig:_main", + "guiconfig = kconfiglib.guiconfig:_main", + "genconfig = kconfiglib.genconfig:main", + "oldconfig = kconfiglib.oldconfig:_main", + "olddefconfig = kconfiglib.olddefconfig:main", + "savedefconfig = kconfiglib.savedefconfig:main", + "defconfig = kconfiglib.defconfig:main", + "alldefconfig = kconfiglib.alldefconfig:main", + "allnoconfig = kconfiglib.allnoconfig:main", + "allmodconfig = kconfiglib.allmodconfig:main", + "allyesconfig = kconfiglib.allyesconfig:main", + "listnewconfig = kconfiglib.listnewconfig:main", + "setconfig = kconfiglib.setconfig:main", ) }, # No C extensions or third-party dependencies required. From d6a4275b2e33336cde5972169660388460448952 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Mon, 21 Sep 2026 01:00:01 +0800 Subject: [PATCH 2/5] Hoist the helpers both interfaces had copies of menuconfig.py and guiconfig.py share 38 function names, and 17 of those had byte-identical bodies: the whole expression-formatting chain that renders every expression either tool prints, plus the range, include path and choice symbol blurbs from the info dialog. Two copies means a fix applied to one silently leaves the other wrong, and the 21 names that are not identical show that has already been happening. Eleven are pure functions of a MenuNode or an expression, with no terminal and no Tk in them, so they move to uicommon.py unchanged. The remaining six read module globals belonging to one tool or the other and stay put for now. Both tools import the module rather than aliasing each name, so adding a helper is a one-place edit and the call site says where the answer comes from. Together the two shrink from 6653 lines to 6334. Putting them in kconfiglib.py instead would have avoided the import dance that guiconfig.py now needs, but the core library is already the densest file here and these are presentation helpers, not configuration ones. rawterm.py set the precedent for extracting a module the interfaces share. Three tests were parametrized over both tools to check the two agreed, which is a tautology once there is one copy, and the guiconfig half dragged in tkinter to re-run the same function. They call uicommon directly now, as do two of the range tests. The checks that validate an entered value keep their pair, being still a real copy in each tool. --- .github/workflows/package.yml | 5 +- guiconfig.py | 201 ++++----------------------------- menuconfig.py | 203 ++++------------------------------ setup.py | 1 + tests/conftest.py | 13 +++ tests/test_ui_ranges.py | 33 +++--- tests/test_uirender.py | 43 +++---- uicommon.py | 195 ++++++++++++++++++++++++++++++++ 8 files changed, 285 insertions(+), 409 deletions(-) create mode 100644 uicommon.py diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 673d0f3..4f33383 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -147,8 +147,9 @@ jobs: continue assert callable(entry.load()), entry.name - # rawterm ships no script of its own - importlib.import_module("kconfiglib.rawterm") + # These two ship no script of their own + for module in ("kconfiglib.rawterm", "kconfiglib.uicommon"): + importlib.import_module(module) print(f"{len(installed)} console scripts resolve") PY diff --git a/guiconfig.py b/guiconfig.py index a058d33..f0bc20e 100755 --- a/guiconfig.py +++ b/guiconfig.py @@ -66,6 +66,11 @@ from tkinter import font from tkinter import filedialog, messagebox +if __package__: + from . import uicommon +else: + import uicommon + from kconfiglib import ( Symbol, Choice, @@ -79,7 +84,6 @@ HEX, AND, OR, - expr_str, expr_value, split_expr, standard_sc_expr_str, @@ -1119,7 +1123,7 @@ def rec(node): res = [] while node: - if _visible(node) or _show_all: + if uicommon.visible(node) or _show_all: res.append(node) if node.list and isinstance(node.item, Symbol): # Nodes from menu created from dependencies @@ -1157,7 +1161,7 @@ def rec(node): res = [] while node: - if _visible(node) or _show_all: + if uicommon.visible(node) or _show_all: res.append(node) if node.list and not node.is_menuconfig: res += rec(node.list) @@ -1177,17 +1181,6 @@ def rec(node): return rec(menu.list) -def _visible(node): - # Returns True if the node should appear in the menu (outside show-all - # mode) - - return ( - node.prompt - and expr_value(node.prompt[1]) - and not (node.item == MENU and not expr_value(node.visibility)) - ) - - def _add_to_tree(node, top): # Adds 'node' to the tree, at the end of its menu. We rely on going through # the nodes linearly to get the correct order. 'top' holds the menu that @@ -1202,7 +1195,7 @@ def _add_to_tree(node, top): base_tags = _img_tag(node) row_tag = "oddrow" if _tree_row_index % 2 else "evenrow" - if _visible(node) or not _show_all: + if uicommon.visible(node) or not _show_all: tags = base_tags + " " + row_tag else: tags = base_tags + " invisible " + row_tag @@ -1349,7 +1342,7 @@ def _img_tag(node): # BOOL or TRISTATE - if _is_y_mode_choice_sym(item): + if uicommon.is_y_mode_choice_sym(item): # Choice symbol in y-mode choice return "selected" if item.choice.selection is item else "not-selected" @@ -1366,15 +1359,6 @@ def _img_tag(node): return item.str_value + "-tri" -def _is_y_mode_choice_sym(item): - # The choice mode is an upper bound on the visibility of choice symbols, so - # we can check the choice symbols' own visibility to see if the choice is - # in y mode. - # - # 'is not None' so that a non-choice symbol yields False rather than None - return isinstance(item, Symbol) and item.choice is not None and item.visibility == 2 - - def _tree_click(event): # Click on the Kconfig Treeview @@ -1434,7 +1418,7 @@ def _tree_toggle(event): if sel: node = _id_to_node[sel] - if _changeable(node): + if uicommon.changeable(node): _change_node(node, tree.winfo_toplevel()) elif _single_menu_mode_menu(node, tree): _enter_menu_and_select_first(node) @@ -1481,27 +1465,6 @@ def _single_menu_mode_menu(node, tree): ) -def _changeable(node): - # Returns True if 'node' is a Symbol/Choice whose value can be changed - - sc = node.item - - if not isinstance(sc, (Symbol, Choice)): - return False - - # This will hit for invisible symbols, which appear in show-all mode and - # when an invisible symbol has visible children (which can happen e.g. for - # symbols with optional prompts) - if not (node.prompt and expr_value(node.prompt[1])): - return False - - return ( - sc.orig_type in (STRING, INT, HEX) - or len(sc.assignable) > 1 - or _is_y_mode_choice_sym(sc) - ) - - def _tree_toggle_open(item): # Opens/closes the Treeview item 'item' @@ -1603,7 +1566,7 @@ def _change_node(node, parent): # (either the main window or the jump-to dialog), in case we need to pop up # a dialog. - if not _changeable(node): + if not uicommon.changeable(node): return # sc = symbol/choice @@ -1703,7 +1666,7 @@ def cancel(_=None): entry.grid(column=0, row=1, columnspan=2, sticky="ew", padx=".3c") entry.focus_set() - range_info = _range_info(sym) + range_info = uicommon.range_info(sym) if range_info: ttk.Label(dialog, text=range_info).grid( column=0, row=2, columnspan=2, sticky="w", padx=".3c", pady=".2c 0" @@ -1817,18 +1780,6 @@ def _check_valid(dialog, entry, sym, s): return True -def _range_info(sym): - # Returns a string with information about the valid range for the symbol - # 'sym', or None if 'sym' doesn't have a range - - if sym.orig_type in (INT, HEX): - for low, high, cond, _ in sym.ranges: - if expr_value(cond): - return f"Range: {low.str_value}-{high.str_value}" - - return None - - def _save(_=None): # Tries to save the configuration @@ -2085,7 +2036,7 @@ def _leave_menu(): if _cur_menu is not _kconf.top_node: old_menu = _cur_menu - _enter_menu(_parent_menu(old_menu)) + _enter_menu(uicommon.parent_menu(old_menu)) _select(_tree, id(old_menu)) _tree.focus_set() @@ -2306,7 +2257,7 @@ def jump_to_selected(event=None): node = _id_to_node[sel[0]] - if node not in _shown_menu_nodes(_parent_menu(node)): + if node not in _shown_menu_nodes(uicommon.parent_menu(node)): _show_all_var.set(True) if not _single_menu: # See comment in _do_tree_mode() @@ -2483,7 +2434,7 @@ def _update_jump_to_display(): id_ = id node_str = _node_str img_tag = _img_tag - visible = _visible + visible = uicommon.visible for node in _jump_to_matches: item( id_(node), @@ -2498,7 +2449,7 @@ def _jump_to(node): # Jumps directly to 'node' and selects it if _single_menu: - _enter_menu(_parent_menu(node)) + _enter_menu(uicommon.parent_menu(node)) else: _load_parents(node) @@ -2575,17 +2526,6 @@ def _load_parents(node): return -def _parent_menu(node): - # Returns the menu node of the menu that contains 'node'. In addition to - # proper 'menu's, this might also be a 'menuconfig' symbol or a 'choice'. - # "Menu" here means a menu in the interface. - - menu = node.parent - while not menu.is_menuconfig: - menu = menu.parent - return menu - - def _trace_write(var, fn): # Makes fn() be called whenever the Tkinter Variable 'var' changes value @@ -2622,7 +2562,7 @@ def _info_str(node): _name_info(choice) + _help_info(choice) + f"Mode: {choice.str_value}\n\n" - + _choice_syms_info(choice) + + uicommon.choice_syms_info(choice) + _direct_dep_info(choice) + _defaults_info(choice) + _kconfig_def_info(choice) @@ -2662,21 +2602,6 @@ def _value_info(sym): return s -def _choice_syms_info(choice): - # Returns a string listing the choice symbols in 'choice'. Adds - # "(selected)" next to the selected one. - - s = "Choice symbols:\n" - - for sym in choice.syms: - s += " - " + sym.name - if sym is choice.selection: - s += " (selected)" - s += "\n" - - return s + "\n" - - def _help_info(sc): # Returns a string with the help text(s) of 'sc' (Symbol or Choice). # Symbols and choices defined in multiple locations can have multiple help @@ -2700,7 +2625,7 @@ def _direct_dep_info(sc): if sc.direct_dep is _kconf.y: return "" - return f"Direct dependencies (={TRI_TO_STR[expr_value(sc.direct_dep)]}):\n{_split_expr_info(sc.direct_dep, 2)}\n" + return f"Direct dependencies (={TRI_TO_STR[expr_value(sc.direct_dep)]}):\n{uicommon.split_expr_info(sc.direct_dep, 2)}\n" def _defaults_info(sc): @@ -2714,7 +2639,7 @@ def _defaults_info(sc): for val, cond in sc.orig_defaults: s += " - " if isinstance(sc, Symbol): - s += _expr_str(val) + s += uicommon.expr_str_with_values(val) # Skip the tristate value hint if the expression is just a single # symbol. _expr_str() already shows its value as a string. @@ -2730,41 +2655,11 @@ def _defaults_info(sc): s += "\n" if cond is not _kconf.y: - s += f" Condition (={TRI_TO_STR[expr_value(cond)]}):\n{_split_expr_info(cond, 4)}" + s += f" Condition (={TRI_TO_STR[expr_value(cond)]}):\n{uicommon.split_expr_info(cond, 4)}" return s + "\n" -def _split_expr_info(expr, indent): - # Returns a string with 'expr' split into its top-level && or || operands, - # with one operand per line, together with the operand's value. This is - # usually enough to get something readable for long expressions. A fancier - # recursive thingy would be possible too. - # - # indent: - # Number of leading spaces to add before the split expression. - - if len(split_expr(expr, AND)) > 1: - split_op = AND - op_str = "&&" - else: - split_op = OR - op_str = "||" - - s = "" - for i, term in enumerate(split_expr(expr, split_op)): - s += "{}{} {}".format(indent * " ", " " if i == 0 else op_str, _expr_str(term)) - - # Don't bother showing the value hint if the expression is just a - # single symbol. _expr_str() already shows its value. - if isinstance(term, tuple): - s += f" (={TRI_TO_STR[expr_value(term)]})" - - s += "\n" - - return s - - def _select_imply_info(sym): # Returns a string with information about which symbols 'select' or 'imply' # 'sym'. The selecting/implying symbols are grouped according to which @@ -2814,24 +2709,14 @@ def _kconfig_def_info(item): s += ( "\n\n" f"At {node.filename}:{node.linenr}\n" - f"{_include_path_info(node)}" + f"{uicommon.include_path_info(node)}" f"Menu path: {_menu_path_info(node)}\n\n" - f"{node.custom_str(_name_and_val_str)}" + f"{node.custom_str(uicommon.name_and_val_str)}" ) return s -def _include_path_info(node): - if not node.include_path: - # In the top-level Kconfig file - return "" - - return "Included via {}\n".format( - " -> ".join(f"{filename}:{linenr}" for filename, linenr in node.include_path) - ) - - def _menu_path_info(node): # Returns a string describing the menu path leading up to 'node' @@ -2852,47 +2737,5 @@ def _menu_path_info(node): return "(Top)" + path -def _name_and_val_str(sc): - # Custom symbol/choice printer that shows symbol values after symbols - - # Show the values of non-constant (non-quoted) symbols that don't look like - # numbers. Things like 123 are actually symbol references, and only work as - # expected due to undefined symbols getting their name as their value. - # Showing the symbol value for those isn't helpful though. - if isinstance(sc, Symbol) and not sc.is_constant and not _is_num(sc.name): - if not sc.nodes: - # Undefined symbol reference - return f"{sc.name}(undefined/n)" - - return f"{sc.name}(={sc.str_value})" - - # For other items, use the standard format - return standard_sc_expr_str(sc) - - -def _expr_str(expr): - # Custom expression printer that shows symbol values - return expr_str(expr, _name_and_val_str) - - -def _is_num(name): - # Heuristic to see if a symbol name looks like a number, for nicer output - # when printing expressions. Things like 16 are actually symbol names, only - # they get their name as their value when the symbol is undefined. - - try: - int(name) - except ValueError: - if not name.startswith(("0x", "0X")): - return False - - try: - int(name, 16) - except ValueError: - return False - - return True - - if __name__ == "__main__": _main() diff --git a/menuconfig.py b/menuconfig.py index af13301..d00b20b 100755 --- a/menuconfig.py +++ b/menuconfig.py @@ -197,10 +197,11 @@ import textwrap if __package__: - from . import rawterm + from . import rawterm, uicommon from .rawterm import Key, Box, Style, Color, NAMED_COLORS else: import rawterm + import uicommon from rawterm import Key, Box, Style, Color, NAMED_COLORS from kconfiglib import ( @@ -216,7 +217,6 @@ HEX, AND, OR, - expr_str, expr_value, split_expr, standard_sc_expr_str, @@ -1019,7 +1019,7 @@ def _jump_to(node): _cur_menu = node node = node.list else: - _cur_menu = _parent_menu(node) + _cur_menu = uicommon.parent_menu(node) _shown = _shown_nodes(_cur_menu) if node not in _shown: @@ -1057,7 +1057,7 @@ def _leave_menu(): return # Jump to parent menu - parent = _parent_menu(_cur_menu) + parent = uicommon.parent_menu(_cur_menu) _shown = _shown_nodes(parent) try: @@ -1352,7 +1352,7 @@ def _draw_main(): for i in range(_menu_scroll, min(_menu_scroll + menu_height, len(_shown))): node = _shown[i] - if _visible(node) or not _show_all: + if uicommon.visible(node) or not _show_all: style = _style["selection" if i == _sel_node_i else "list"] else: style = _style["inv-selection" if i == _sel_node_i else "inv-list"] @@ -1433,17 +1433,6 @@ def _draw_main_buttons(win, dlg_h, dlg_w): _print_button(win, label, button_y, bx, i == _active_button) -def _parent_menu(node): - # Returns the menu node of the menu that contains 'node'. In addition to - # proper 'menu's, this might also be a 'menuconfig' symbol or a 'choice'. - # "Menu" here means a menu in the interface. - - menu = node.parent - while not menu.is_menuconfig: - menu = menu.parent - return menu - - def _shown_nodes(menu): # Returns the list of menu nodes from 'menu' (see _parent_menu()) that # would be shown when entering it @@ -1452,7 +1441,7 @@ def rec(node): res = [] while node: - if _visible(node) or _show_all: + if uicommon.visible(node) or _show_all: res.append(node) if node.list and not node.is_menuconfig: # Nodes from implicit menu created from dependencies. Will @@ -1516,17 +1505,6 @@ def rec(node): return rec(menu.list) -def _visible(node): - # Returns True if the node should appear in the menu (outside show-all - # mode) - - return ( - node.prompt - and expr_value(node.prompt[1]) - and not (node.item == MENU and not expr_value(node.visibility)) - ) - - def _change_node(node): # Changes the value of the menu node 'node' if it is a symbol. Bools and # tristates are toggled, while other symbol types pop up a text entry @@ -1534,7 +1512,7 @@ def _change_node(node): # # Returns False if the value of 'node' can't be changed. - if not _changeable(node): + if not uicommon.changeable(node): return False # sc = symbol/choice @@ -1547,7 +1525,7 @@ def _change_node(node): s = _input_dialog( f"{node.prompt[0]} ({TYPE_TO_STR[sc.orig_type]})", s, - _range_info(sc), + uicommon.range_info(sc), ) if s is None: @@ -1576,7 +1554,7 @@ def _change_node(node): val_index = sc.assignable.index(sc.tri_value) _set_val(sc, sc.assignable[(val_index + 1) % len(sc.assignable)]) - if _is_y_mode_choice_sym(sc) and not node.list: + if uicommon.is_y_mode_choice_sym(sc) and not node.list: # Immediately jump to the parent menu after making a choice selection, # like 'make menuconfig' does, except if the menu node has children # (which can happen if a symbol 'depends on' a choice symbol that @@ -1586,27 +1564,6 @@ def _change_node(node): return True -def _changeable(node): - # Returns True if the value if 'node' can be changed - - sc = node.item - - if not isinstance(sc, (Symbol, Choice)): - return False - - # This will hit for invisible symbols, which appear in show-all mode and - # when an invisible symbol has visible children (which can happen e.g. for - # symbols with optional prompts) - if not (node.prompt and expr_value(node.prompt[1])): - return False - - return ( - sc.orig_type in (STRING, INT, HEX) - or len(sc.assignable) > 1 - or _is_y_mode_choice_sym(sc) - ) - - def _set_sel_node_tri_val(tri_val): # Sets the value of the currently selected menu entry to 'tri_val', if that # value can be assigned @@ -2697,7 +2654,7 @@ def _draw_jump_to_dialog( node = matches[i] if isinstance(node.item, (Symbol, Choice)): - node_str = _name_and_val_str(node.item) + node_str = uicommon.name_and_val_str(node.item) if node.prompt: node_str += f' "{node.prompt[0]}"' elif node.item == MENU: @@ -2985,7 +2942,7 @@ def _info_str_mconf(node): s += f"Defined at {n.filename}:{n.linenr}\n" s += f" Prompt: {n.prompt[0]}\n" if n.dep is not _kconf.y: - s += f" Depends on: {_expr_str(n.dep)}\n" + s += f" Depends on: {uicommon.expr_str_with_values(n.dep)}\n" # Location hierarchy (matching get_prompt_str in mconf) submenu = [] m = n @@ -3010,11 +2967,11 @@ def _info_str_mconf(node): if not n.prompt: s += f"Defined at {n.filename}:{n.linenr}\n" if n.dep is not _kconf.y: - s += f" Depends on: {_expr_str(n.dep)}\n" + s += f" Depends on: {uicommon.expr_str_with_values(n.dep)}\n" # Selects (symbols only) if isinstance(sc, Symbol) and sc.selects: - sel_strs = [_expr_str(sel_sym) for sel_sym, cond in sc.orig_selects] + sel_strs = [uicommon.expr_str_with_values(sel_sym) for sel_sym, cond in sc.orig_selects] s += "Selects: {}\n".format(" && ".join(sel_strs)) # Selected by @@ -3036,7 +2993,7 @@ def _info_str_mconf(node): # Implies (symbols only) if isinstance(sc, Symbol) and sc.implies: - imp_strs = [_expr_str(imp_sym) for imp_sym, cond in sc.orig_implies] + imp_strs = [uicommon.expr_str_with_values(imp_sym) for imp_sym, cond in sc.orig_implies] s += "Implies: {}\n".format(" && ".join(imp_strs)) # Implied by @@ -3093,7 +3050,7 @@ def _info_str(node): + f"Type: {TYPE_TO_STR[choice.type]}\n" + f"Mode: {choice.str_value}\n" + _help_info(choice) - + _choice_syms_info(choice) + + uicommon.choice_syms_info(choice) + _direct_dep_info(choice) + _defaults_info(choice) + _kconfig_def_info(choice) @@ -3147,21 +3104,6 @@ def _value_info(sym): return s -def _choice_syms_info(choice): - # Returns a string listing the choice symbols in 'choice'. Adds - # "(selected)" next to the selected one. - - s = "Choice symbols:\n" - - for sym in choice.syms: - s += " - " + sym.name - if sym is choice.selection: - s += " (selected)" - s += "\n" - - return s + "\n" - - def _help_info(sc): # Returns a string with the help text(s) of 'sc' (Symbol or Choice). # Symbols and choices defined in multiple locations can have multiple help @@ -3185,7 +3127,7 @@ def _direct_dep_info(sc): return ( "" if sc.direct_dep is _kconf.y - else f"Direct dependencies (={TRI_TO_STR[expr_value(sc.direct_dep)]}):\n{_split_expr_info(sc.direct_dep, 2)}\n" + else f"Direct dependencies (={TRI_TO_STR[expr_value(sc.direct_dep)]}):\n{uicommon.split_expr_info(sc.direct_dep, 2)}\n" ) @@ -3203,7 +3145,7 @@ def _defaults_info(sc): for val, cond in sc.orig_defaults: s += " - " if isinstance(sc, Symbol): - s += _expr_str(val) + s += uicommon.expr_str_with_values(val) # Skip the tristate value hint if the expression is just a single # symbol. _expr_str() already shows its value as a string. @@ -3219,41 +3161,11 @@ def _defaults_info(sc): s += "\n" if cond is not _kconf.y: - s += f" Condition (={TRI_TO_STR[expr_value(cond)]}):\n{_split_expr_info(cond, 4)}" + s += f" Condition (={TRI_TO_STR[expr_value(cond)]}):\n{uicommon.split_expr_info(cond, 4)}" return s + "\n" -def _split_expr_info(expr, indent): - # Returns a string with 'expr' split into its top-level && or || operands, - # with one operand per line, together with the operand's value. This is - # usually enough to get something readable for long expressions. A fancier - # recursive thingy would be possible too. - # - # indent: - # Number of leading spaces to add before the split expression. - - if len(split_expr(expr, AND)) > 1: - split_op = AND - op_str = "&&" - else: - split_op = OR - op_str = "||" - - s = "" - for i, term in enumerate(split_expr(expr, split_op)): - s += "{}{} {}".format(indent * " ", " " if i == 0 else op_str, _expr_str(term)) - - # Don't bother showing the value hint if the expression is just a - # single symbol. _expr_str() already shows its value. - if isinstance(term, tuple): - s += f" (={TRI_TO_STR[expr_value(term)]})" - - s += "\n" - - return s - - def _select_imply_info(sym): # Returns a string with information about which symbols 'select' or 'imply' # 'sym'. The selecting/implying symbols are grouped according to which @@ -3306,24 +3218,14 @@ def _kconfig_def_info(item): s += ( "\n\n" f"At {node.filename}:{node.linenr}\n" - f"{_include_path_info(node)}" + f"{uicommon.include_path_info(node)}" f"Menu path: {_menu_path_info(node)}\n\n" - f"{_indent(node.custom_str(_name_and_val_str), 2)}" + f"{_indent(node.custom_str(uicommon.name_and_val_str), 2)}" ) return s -def _include_path_info(node): - if not node.include_path: - # In the top-level Kconfig file - return "" - - return "Included via {}\n".format( - " -> ".join(f"{filename}:{linenr}" for filename, linenr in node.include_path) - ) - - def _menu_path_info(node): # Returns a string describing the menu path leading up to 'node' @@ -3350,29 +3252,6 @@ def _indent(s, n): return "\n".join(n * " " + line for line in s.split("\n")) -def _name_and_val_str(sc): - # Custom symbol/choice printer that shows symbol values after symbols - - # Show the values of non-constant (non-quoted) symbols that don't look like - # numbers. Things like 123 are actually symbol references, and only work as - # expected due to undefined symbols getting their name as their value. - # Showing the symbol value for those isn't helpful though. - if isinstance(sc, Symbol) and not sc.is_constant and not _is_num(sc.name): - if not sc.nodes: - # Undefined symbol reference - return f"{sc.name}(undefined/n)" - - return f"{sc.name}(={sc.str_value})" - - # For other items, use the standard format - return standard_sc_expr_str(sc) - - -def _expr_str(expr): - # Custom expression printer that shows symbol values - return expr_str(expr, _name_and_val_str) - - def _styled_region(style): # Returns a new rawterm Region with style 'style' and space as the fill # character. The initial dimensions are (1, 1), so the region needs to be @@ -3624,7 +3503,7 @@ def _value_str(node): # BOOL or TRISTATE - if _is_y_mode_choice_sym(item): + if uicommon.is_y_mode_choice_sym(item): return "(X)" if item.choice.selection is item else "( )" tri_val_str = (" ", "M", "*")[item.tri_value] @@ -3642,15 +3521,6 @@ def _value_str(node): return f"<{tri_val_str}>" -def _is_y_mode_choice_sym(item): - # The choice mode is an upper bound on the visibility of choice symbols, so - # we can check the choice symbols' own visibility to see if the choice is - # in y mode. - # - # 'is not None' so that a non-choice symbol yields False rather than None - return isinstance(item, Symbol) and item.choice is not None and item.visibility == 2 - - def _check_valid(sym, s): # Returns True if the string 's' is a well-formed value for 'sym'. # Otherwise, displays an error and returns False. @@ -3679,37 +3549,6 @@ def _check_valid(sym, s): return True -def _range_info(sym): - # Returns a string with information about the valid range for the symbol - # 'sym', or None if 'sym' doesn't have a range - - if sym.orig_type in (INT, HEX): - for low, high, cond, _ in sym.ranges: - if expr_value(cond): - return f"Range: {low.str_value}-{high.str_value}" - - return None - - -def _is_num(name): - # Heuristic to see if a symbol name looks like a number, for nicer output - # when printing expressions. Things like 16 are actually symbol names, only - # they get their name as their value when the symbol is undefined. - - try: - int(name) - except ValueError: - if not name.startswith(("0x", "0X")): - return False - - try: - int(name, 16) - except ValueError: - return False - - return True - - def _warn(*args): # Temporarily exits terminal mode and prints a warning to stderr. # The warning would get lost in terminal mode. diff --git a/setup.py b/setup.py index 98077e0..ad591b4 100644 --- a/setup.py +++ b/setup.py @@ -16,6 +16,7 @@ _MODULES = ( _PACKAGE, "rawterm", + "uicommon", "menuconfig", "guiconfig", "genconfig", diff --git a/tests/conftest.py b/tests/conftest.py index 091cab0..1ac0705 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -86,3 +86,16 @@ def assign_and_verify_user_value(c, sym_name, val, user_val, valid): def verify_str(item, expected): """Verify str(item) matches expected (strip leading/trailing newline).""" assert str(item) == expected[1:-1] + + +def node(kconf, name): + """The first menu node of symbol 'name'.""" + return kconf.syms[name].nodes[0] + + +def named_menu(kconf, prompt): + """The menu whose prompt is 'prompt'.""" + for menu in kconf.menus: + if menu.prompt[0] == prompt: + return menu + raise AssertionError(f"no menu titled {prompt!r}") diff --git a/tests/test_ui_ranges.py b/tests/test_ui_ranges.py index 71438c5..93ca696 100644 --- a/tests/test_ui_ranges.py +++ b/tests/test_ui_ranges.py @@ -3,15 +3,18 @@ # # Regression tests for the INT/HEX range helpers in menuconfig and guiconfig. # -# These call the *real* _range_info()/_check_valid() functions (not a +# These call the *real* range_info()/_check_valid() functions (not a # reimplementation) so that a mis-count of the sym.ranges tuples is caught here -# instead of by a user. sym.ranges yields 4-tuples (low, high, cond, loc); +# instead of by a user. range_info() now lives in uicommon and is shared, so it +# is tested once; _check_valid() is still a separate copy in each tool and is +# tested in both. sym.ranges yields 4-tuples (low, high, cond, loc); # unpacking them as 3-tuples was the root cause of issue #41, which shipped # because nothing exercised these paths. import pytest import menuconfig +import uicommon from kconfiglib import Kconfig @@ -21,22 +24,22 @@ def kconf(): # --------------------------------------------------------------------------- -# menuconfig +# range_info, shared by both tools since it moved to uicommon # --------------------------------------------------------------------------- -def test_menuconfig_range_info_active(kconf): +def test_range_info_active(kconf): # Active range -> the loop body unpacks a 4-tuple. This is the #41 path. - assert menuconfig._range_info(kconf.syms["INT_RANGE_10_20"]) == "Range: 10-20" - assert menuconfig._range_info(kconf.syms["HEX_RANGE_10_20"]) == "Range: 0x10-0x20" + assert uicommon.range_info(kconf.syms["INT_RANGE_10_20"]) == "Range: 10-20" + assert uicommon.range_info(kconf.syms["HEX_RANGE_10_20"]) == "Range: 0x10-0x20" -def test_menuconfig_range_info_none(kconf): +def test_range_info_none(kconf): # No range, and all-ranges-disabled (cond is n): still iterates and unpacks # every tuple, but reports no active range. - assert menuconfig._range_info(kconf.syms["INT_NO_RANGE"]) is None - assert menuconfig._range_info(kconf.syms["INT_ALL_RANGES_DISABLED"]) is None - assert menuconfig._range_info(kconf.syms["HEX_ALL_RANGES_DISABLED"]) is None + assert uicommon.range_info(kconf.syms["INT_NO_RANGE"]) is None + assert uicommon.range_info(kconf.syms["INT_ALL_RANGES_DISABLED"]) is None + assert uicommon.range_info(kconf.syms["HEX_ALL_RANGES_DISABLED"]) is None def test_menuconfig_check_valid(kconf, monkeypatch): @@ -70,16 +73,6 @@ def guiconfig(): return pytest.importorskip("guiconfig") -def test_guiconfig_range_info_active(guiconfig, kconf): - assert guiconfig._range_info(kconf.syms["INT_RANGE_10_20"]) == "Range: 10-20" - assert guiconfig._range_info(kconf.syms["HEX_RANGE_10_20"]) == "Range: 0x10-0x20" - - -def test_guiconfig_range_info_none(guiconfig, kconf): - assert guiconfig._range_info(kconf.syms["INT_NO_RANGE"]) is None - assert guiconfig._range_info(kconf.syms["INT_ALL_RANGES_DISABLED"]) is None - - def test_guiconfig_check_valid(guiconfig, kconf, monkeypatch): errors = [] diff --git a/tests/test_uirender.py b/tests/test_uirender.py index ecb8667..27b0e76 100644 --- a/tests/test_uirender.py +++ b/tests/test_uirender.py @@ -17,6 +17,8 @@ import pytest import menuconfig +import uicommon +from conftest import named_menu, node from kconfiglib import Kconfig @@ -55,17 +57,6 @@ def ui(request): return request.getfixturevalue("mc" if request.param == "menuconfig" else "gc_") -def node(kconf, name): - return kconf.syms[name].nodes[0] - - -def named_menu(kconf, prompt): - for menu in kconf.menus: - if menu.prompt[0] == prompt: - return menu - raise AssertionError(f"no menu titled {prompt!r}") - - # --- menuconfig._value_str: one case per branch ----------------------------- @@ -222,7 +213,11 @@ def test_guiconfig_choice_sym_prompt_falls_back(gc_, kconf): assert gc_._choice_sym_prompt(kconf.syms["PROMPTLESS"], choice_node) is None -# --- the two tools must agree on what is changeable ------------------------- +# --- what counts as changeable ---------------------------------------------- +# +# These used to be parametrized over both tools, because each carried its own +# copy of changeable(). There is one copy now, in uicommon, so parametrizing +# would run the same function twice and drag tkinter in to do it. @pytest.mark.parametrize( @@ -237,18 +232,15 @@ def test_guiconfig_choice_sym_prompt_falls_back(gc_, kconf): ("PROMPTLESS", False), ], ) -@pytest.mark.parametrize("ui", ("menuconfig", "guiconfig"), indirect=True) -def test_changeable_agrees_between_tools(ui, kconf, sym, expected): - # 'is' rather than '==': _changeable() is documented to return True/False, +def test_changeable(kconf, sym, expected): + # 'is' rather than '==': changeable() is documented to return True/False, # and it used to leak the None from the end of an 'and' chain instead - n = node(kconf, sym) - assert ui._changeable(n) is expected + assert uicommon.changeable(node(kconf, sym)) is expected -@pytest.mark.parametrize("ui", ("menuconfig", "guiconfig"), indirect=True) -def test_changeable_rejects_menus_and_comments(ui, kconf): +def test_changeable_rejects_menus_and_comments(kconf): for n in (named_menu(kconf, "A menu"), kconf.comments[0]): - assert ui._changeable(n) is False + assert uicommon.changeable(n) is False # --- menuconfig._shown_nodes: what actually reaches the screen -------------- @@ -281,11 +273,10 @@ def test_shown_nodes_of_an_empty_menu_is_empty(mc, kconf, monkeypatch): assert mc._shown_nodes(named_menu(kconf, "A menu")) != [] -@pytest.mark.parametrize("ui", ("menuconfig", "guiconfig"), indirect=True) -def test_is_y_mode_choice_sym_returns_a_real_bool(ui, kconf): - """The predicate feeding _changeable() must not leak a None.""" - assert ui._is_y_mode_choice_sym(kconf.syms["CHOICE_A"]) is True +def test_is_y_mode_choice_sym_returns_a_real_bool(kconf): + """The predicate feeding changeable() must not leak a None.""" + assert uicommon.is_y_mode_choice_sym(kconf.syms["CHOICE_A"]) is True # Not a choice symbol at all - assert ui._is_y_mode_choice_sym(kconf.syms["BOOL_SYM"]) is False + assert uicommon.is_y_mode_choice_sym(kconf.syms["BOOL_SYM"]) is False # Not a Symbol at all - assert ui._is_y_mode_choice_sym(kconf.choices[0]) is False + assert uicommon.is_y_mode_choice_sym(kconf.choices[0]) is False diff --git a/uicommon.py b/uicommon.py new file mode 100644 index 0000000..03a1175 --- /dev/null +++ b/uicommon.py @@ -0,0 +1,195 @@ +# Copyright (c) 2011-2019 Ulf Magnusson +# SPDX-License-Identifier: ISC + +""" +Presentation helpers shared by the terminal and Tk configuration interfaces. + +menuconfig.py and guiconfig.py show the same information about a symbol; they +differ only in how they paint it. Everything that turns a MenuNode or an +expression into text, with no terminal and no Tk anywhere in it, lives here so +that the two tools cannot answer the same question differently. + +These are pure functions of the objects handed to them. That is what makes +them testable on their own, and it is the reason to keep them out of the two +tools: a helper that reads a module global from menuconfig.py could not be +called from guiconfig.py, and vice versa. +""" + +from kconfiglib import ( + AND, + HEX, + INT, + MENU, + OR, + STRING, + TRI_TO_STR, + Choice, + Symbol, + expr_str, + expr_value, + split_expr, + standard_sc_expr_str, +) + + +def is_num(name): + # Heuristic to see if a symbol name looks like a number, for nicer output + # when printing expressions. Things like 16 are actually symbol names, only + # they get their name as their value when the symbol is undefined. + + try: + int(name) + except ValueError: + if not name.startswith(("0x", "0X")): + return False + + try: + int(name, 16) + except ValueError: + return False + + return True + + +def is_y_mode_choice_sym(item): + # The choice mode is an upper bound on the visibility of choice symbols, so + # we can check the choice symbols' own visibility to see if the choice is + # in y mode. + # + # 'is not None' so that a non-choice symbol yields False rather than None + return isinstance(item, Symbol) and item.choice is not None and item.visibility == 2 + + +def parent_menu(node): + # Returns the menu node of the menu that contains 'node'. In addition to + # proper 'menu's, this might also be a 'menuconfig' symbol or a 'choice'. + # "Menu" here means a menu in the interface. + + menu = node.parent + while not menu.is_menuconfig: + menu = menu.parent + return menu + + +def visible(node): + # Returns True if the node should appear in the menu (outside show-all + # mode) + + return ( + node.prompt + and expr_value(node.prompt[1]) + and not (node.item == MENU and not expr_value(node.visibility)) + ) + + +def changeable(node): + # Returns True if the value if 'node' can be changed + + sc = node.item + + if not isinstance(sc, (Symbol, Choice)): + return False + + # This will hit for invisible symbols, which appear in show-all mode and + # when an invisible symbol has visible children (which can happen e.g. for + # symbols with optional prompts) + if not (node.prompt and expr_value(node.prompt[1])): + return False + + return ( + sc.orig_type in (STRING, INT, HEX) + or len(sc.assignable) > 1 + or is_y_mode_choice_sym(sc) + ) + + +def range_info(sym): + # Returns a string with information about the valid range for the symbol + # 'sym', or None if 'sym' doesn't have a range + + if sym.orig_type in (INT, HEX): + for low, high, cond, _ in sym.ranges: + if expr_value(cond): + return f"Range: {low.str_value}-{high.str_value}" + + return None + + +def include_path_info(node): + if not node.include_path: + # In the top-level Kconfig file + return "" + + return "Included via {}\n".format( + " -> ".join(f"{filename}:{linenr}" for filename, linenr in node.include_path) + ) + + +def choice_syms_info(choice): + # Returns a string listing the choice symbols in 'choice'. Adds + # "(selected)" next to the selected one. + + s = "Choice symbols:\n" + + for sym in choice.syms: + s += " - " + sym.name + if sym is choice.selection: + s += " (selected)" + s += "\n" + + return s + "\n" + + +def name_and_val_str(sc): + # Custom symbol/choice printer that shows symbol values after symbols + + # Show the values of non-constant (non-quoted) symbols that don't look like + # numbers. Things like 123 are actually symbol references, and only work as + # expected due to undefined symbols getting their name as their value. + # Showing the symbol value for those isn't helpful though. + if isinstance(sc, Symbol) and not sc.is_constant and not is_num(sc.name): + if not sc.nodes: + # Undefined symbol reference + return f"{sc.name}(undefined/n)" + + return f"{sc.name}(={sc.str_value})" + + # For other items, use the standard format + return standard_sc_expr_str(sc) + + +def expr_str_with_values(expr): + # Custom expression printer that shows symbol values + return expr_str(expr, name_and_val_str) + + +def split_expr_info(expr, indent): + # Returns a string with 'expr' split into its top-level && or || operands, + # with one operand per line, together with the operand's value. This is + # usually enough to get something readable for long expressions. A fancier + # recursive thingy would be possible too. + # + # indent: + # Number of leading spaces to add before the split expression. + + if len(split_expr(expr, AND)) > 1: + split_op = AND + op_str = "&&" + else: + split_op = OR + op_str = "||" + + s = "" + for i, term in enumerate(split_expr(expr, split_op)): + s += "{}{} {}".format( + indent * " ", " " if i == 0 else op_str, expr_str_with_values(term) + ) + + # Don't bother showing the value hint if the expression is just a + # single symbol. _expr_str() already shows its value. + if isinstance(term, tuple): + s += f" (={TRI_TO_STR[expr_value(term)]})" + + s += "\n" + + return s From a326dec8d00bcb5373a3d3b86d8d4c4a6aadd727 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Mon, 21 Sep 2026 01:00:14 +0800 Subject: [PATCH 3/5] Put the terminal interface globals in one object Twenty module globals, assigned through 64 global statements scattered across the file, with the selected index and the scroll offset written from a dozen places each. Nothing could be called before the main loop had set them up, so the cursor and scrolling logic had no tests at all: reaching it meant starting a terminal. That is most of why menuconfig.py sits at 11 percent coverage while the library it drives is at 75. They become fields on a _State that the entry point builds fresh per run. The rewrite was driven off the syntax tree rather than the text, after checking that no function binds any of the twenty names locally, so every one of the 314 references was unambiguously the global it looked like. Behavior is unchanged: the row renderer, the shown-node walk and the info dialog over five test Kconfigs in four display modes produce output identical to before, byte for byte. Building it fresh also fixes something. The old globals survived between runs, so a second run in one process inherited the first one's scroll position, dialog state and show-all mode. The search caches needed an explicit clear at entry for the same reason, so that goes away. tests/test_uinav.py is what this was for. It builds a _State, hands it a window with a height, and drives entering and leaving menus, the four selection commands and the scroll offset directly. One case pins the invariant the whole offset dance exists for, that the selection never leaves the window; another pins the clamp that catches a terminal shrinking while the user is inside a submenu. Building the fixture's node list has to come after installing the state, not before, because the walk reads show-all off the module rather than off anything passed in. Getting that backwards silently dropped the promptless node the fixture exists to show. --- menuconfig.py | 763 ++++++++++++++++++++--------------------- scripts/benchmark.py | 12 +- tests/test_uinav.py | 192 +++++++++++ tests/test_uirender.py | 20 +- tests/test_uisearch.py | 10 +- 5 files changed, 587 insertions(+), 410 deletions(-) create mode 100644 tests/test_uinav.py diff --git a/menuconfig.py b/menuconfig.py index d00b20b..18e5187 100755 --- a/menuconfig.py +++ b/menuconfig.py @@ -463,32 +463,28 @@ def menuconfig(kconf, headless=False): processing. In headless mode, the function only loads the configuration and returns immediately without user interaction. """ - global _kconf - global _conf_filename - global _conf_changed - global _minconf_filename - global _show_all + global _s - _kconf = kconf - - # Clear cached node lists so they rebuild for the new Kconfig instance - _cached_sc_nodes.clear() - _cached_menu_comment_nodes.clear() + # Start from clean state. A second run in the same process would + # otherwise inherit the first run's scroll position, dialog state and + # show-all mode. + _s = _State() + _s.kconf = kconf # Filename to save configuration to - _conf_filename = standard_config_filename() + _s.conf_filename = standard_config_filename() - # Load existing configuration and set _conf_changed True if it is outdated - _conf_changed = _load_config() + # Load existing configuration and set _s.conf_changed True if it is outdated + _s.conf_changed = _load_config() # Filename to save minimal configuration to - _minconf_filename = "defconfig" + _s.minconf_filename = "defconfig" # Any visible items in the top menu? - _show_all = False + _s.show_all = False if not _shown_nodes(kconf.top_node): # Nothing visible. Start in show-all mode and try again. - _show_all = True + _s.show_all = True if not _shown_nodes(kconf.top_node): # Give up. The implementation relies on always having a selected # node. @@ -531,8 +527,8 @@ def _load_config(): # at all. We prompt for saving the configuration if there are actual changes # or if no .config file exists (so user can save the default configuration). - print(_kconf.load_config()) - if not os.path.exists(_conf_filename): + print(_s.kconf.load_config()) + if not os.path.exists(_s.conf_filename): # No .config exists - treat as changed so user can save defaults return True @@ -540,71 +536,122 @@ def _load_config(): def _needs_save(): - return _kconf_needs_save(_kconf) + return _kconf_needs_save(_s.kconf) -# Global variables used below: -# -# _term: -# rawterm.Terminal instance -# -# _cur_menu: -# Menu node of the menu (or menuconfig symbol, or choice) currently being -# shown -# -# _shown: -# List of items in _cur_menu that are shown (ignoring scrolling). In -# show-all mode, this list contains all items in _cur_menu. Otherwise, it -# contains just the visible items. -# -# _sel_node_i: -# Index in _shown of the currently selected node -# -# _menu_scroll: -# Index in _shown of the top row of the main display -# -# _parent_screen_rows: -# List/stack of the row numbers that the selections in the parent menus -# appeared on. This is used to prevent the scrolling from jumping around -# when going in and out of menus. -# -# _show_help/_show_name/_show_all: -# If True, the corresponding mode is on. See the module docstring. -# -# _conf_filename: -# File to save the configuration to -# -# _minconf_filename: -# File to save minimal configurations to -# -# _conf_changed: -# True if the configuration has been changed. If False, we don't bother -# showing the save-and-quit dialog. -# -# We reset this to False whenever the configuration is saved explicitly -# from the save dialog. +class _State: + """Everything the interface mutates while it is running. + + One object rather than twenty module globals, so that the drawing and + navigation helpers can be driven without a terminal: build a _State, point + the module's _s at it, and call the function under test. That is how the + row-rendering, search and navigation tests drive this module without a + terminal. + + Fields: + + term: + rawterm.Terminal instance + + kconf: + The Kconfig instance being configured + + cur_menu: + Menu node of the menu (or menuconfig symbol, or choice) currently being + shown + + shown: + List of items in cur_menu that are shown (ignoring scrolling). In + show-all mode, this list contains all items in cur_menu. Otherwise, it + contains just the visible items. + + sel_node_i: + Index in shown of the currently selected node + + menu_scroll: + Index in shown of the top row of the main display + + parent_screen_rows: + List/stack of the row numbers that the selections in the parent menus + appeared on. This is used to prevent the scrolling from jumping around + when going in and out of menus. + + show_help/show_name/show_all: + If True, the corresponding mode is on. See the module docstring. + + conf_filename: + File to save the configuration to + + minconf_filename: + File to save minimal configurations to + + conf_changed: + True if the configuration has been changed. If False, we don't bother + showing the save-and-quit dialog. + + We reset this to False whenever the configuration is saved explicitly + from the save dialog. + + screen_win/menu_win/dialog_win/help_win: + rawterm regions making up the display + + dlg_bottom_shadow/dlg_right_shadow: + Regions painting the drop shadow of the active dialog, or None + + active_button: + Index of the selected button in the dialog currently being shown + """ + + def __init__(self): + self.term = None + self.kconf = None + + self.cur_menu = None + self.shown = [] + self.sel_node_i = 0 + self.menu_scroll = 0 + self.parent_screen_rows = [] + + # Search caches, rebuilt on demand by _sorted_sc_nodes() and + # _sorted_menu_comment_nodes() + self.cached_sc_nodes = [] + self.cached_menu_comment_nodes = [] + + self.show_help = False + self.show_name = False + self.show_all = False + + self.conf_filename = None + self.minconf_filename = None + self.conf_changed = False + + self.screen_win = None + self.menu_win = None + self.dialog_win = None + self.help_win = None + self.dlg_bottom_shadow = None + self.dlg_right_shadow = None + + self.active_button = 0 + + +# Rebuilt by menuconfig(), so that a second run in the same process does not +# inherit the first run's scroll position or dialog state. +_s = _State() def _menuconfig(term): # Logic for the main display, with the list of symbols, etc. - global _term - global _conf_filename - global _conf_changed - global _minconf_filename - global _show_help - global _show_name - global _active_button - - _term = term + _s.term = term _init() while True: _draw_main() - _term.update() + _s.term.update() - c = _term.read_key() + c = _s.term.read_key() if c == Key.RESIZE: _resize_main() @@ -638,42 +685,42 @@ def _menuconfig(term): # elif c in ("\t", Key.RIGHT): - _active_button = (_active_button + 1) % len(_MENU_BUTTONS) + _s.active_button = (_s.active_button + 1) % len(_MENU_BUTTONS) elif c == Key.LEFT: - _active_button = (_active_button - 1) % len(_MENU_BUTTONS) + _s.active_button = (_s.active_button - 1) % len(_MENU_BUTTONS) # # Enter activates the currently focused button (matching mconf) # elif c == "\n": - if _active_button == 0: # Select - sel_node = _shown[_sel_node_i] + if _s.active_button == 0: # Select + sel_node = _s.shown[_s.sel_node_i] if not _enter_menu(sel_node): _change_node(sel_node) - elif _active_button == 1: # Exit - if _cur_menu is _kconf.top_node: + elif _s.active_button == 1: # Exit + if _s.cur_menu is _s.kconf.top_node: res = _quit_dialog() if res: return res else: _leave_menu() - elif _active_button == 2: # Help - _info_dialog(_shown[_sel_node_i], False) + elif _s.active_button == 2: # Help + _info_dialog(_s.shown[_s.sel_node_i], False) _resize_main() - elif _active_button == 3: # Save + elif _s.active_button == 3: # Save filename = _save_dialog( - _kconf.write_config, _conf_filename, "configuration" + _s.kconf.write_config, _s.conf_filename, "configuration" ) if filename: - _conf_filename = filename - _conf_changed = False + _s.conf_filename = filename + _s.conf_changed = False - elif _active_button == 4: # Load + elif _s.active_button == 4: # Load _load_dialog() # @@ -682,7 +729,7 @@ def _menuconfig(term): elif c == " ": # Toggle the node if possible - sel_node = _shown[_sel_node_i] + sel_node = _s.shown[_s.sel_node_i] if not _change_node(sel_node): _enter_menu(sel_node) @@ -699,7 +746,7 @@ def _menuconfig(term): elif c in (Key.BACKSPACE, "\x1b", "e", "x", "E", "X"): # Leave menu (ESC/Backspace/E/X). At top level, ESC/E/X show # quit dialog; Backspace always just leaves. - if c != Key.BACKSPACE and _cur_menu is _kconf.top_node: + if c != Key.BACKSPACE and _s.cur_menu is _s.kconf.top_node: res = _quit_dialog() if res: return res @@ -711,15 +758,15 @@ def _menuconfig(term): _resize_main() elif c in ("?", "h", "H"): - _info_dialog(_shown[_sel_node_i], False) + _info_dialog(_s.shown[_s.sel_node_i], False) _resize_main() elif c in ("f", "F"): - _show_help = not _show_help + _s.show_help = not _s.show_help _resize_main() elif c in ("c", "C"): - _show_name = not _show_name + _s.show_name = not _s.show_name elif c in ("a", "A", "z", "Z"): _toggle_show_all() @@ -731,10 +778,10 @@ def _menuconfig(term): def _quit_dialog(): - config_exists = os.path.exists(_conf_filename) + config_exists = os.path.exists(_s.conf_filename) - if not _conf_changed and config_exists: - return f"No changes to save (for '{_conf_filename}')" + if not _s.conf_changed and config_exists: + return f"No changes to save (for '{_s.conf_filename}')" # Match C mconf's handle_exit() -- dialog_yesno with 2 buttons dialog_text = ( @@ -754,7 +801,7 @@ def _quit_dialog(): if result == 0: # Yes # Returns a message to print - msg = _try_save(_kconf.write_config, _conf_filename, "configuration") + msg = _try_save(_s.kconf.write_config, _s.conf_filename, "configuration") if msg: return msg # If save failed, try again @@ -763,7 +810,7 @@ def _quit_dialog(): elif result == 1: # No if not config_exists: return "Configuration was not saved" - return f"Configuration ({_conf_filename}) was not saved" + return f"Configuration ({_s.conf_filename}) was not saved" def _init(): @@ -772,69 +819,52 @@ def _init(): # the terminal. # # The layout matches mconf/lxdialog's dialog_menu(): - # _screen_win - full screen background (cyan/blue) - # _dialog_win - centered dialog body (white, with border/title/ - # instructions/inner menu box/separator/buttons) - # _menu_win - inner menu item area (positioned inside the dialog) - # _help_win - help text (show-help mode only) + # _s.screen_win - full screen background (cyan/blue) + # _s.dialog_win - centered dialog body (white, with border/title/ + # instructions/inner menu box/separator/buttons) + # _s.menu_win - inner menu item area (positioned inside the dialog) + # _s.help_win - help text (show-help mode only) # # Shadow regions for the dialog are created in _resize_main(). - global _screen_win - global _dialog_win - global _menu_win - global _help_win - global _dlg_bottom_shadow - global _dlg_right_shadow - global _active_button - - global _parent_screen_rows - global _cur_menu - global _shown - global _sel_node_i - global _menu_scroll - - global _show_help - global _show_name - _init_styles() # Hide the cursor - _term.hide_cursor() + _s.term.hide_cursor() # Initialize regions -- creation order determines compositing order # (painter's algorithm: later regions paint on top of earlier ones) # Full-screen background (lowest layer) - _screen_win = _styled_region("screen") + _s.screen_win = _styled_region("screen") # The main dialog body (above screen background) - _dialog_win = _styled_region("body") + _s.dialog_win = _styled_region("body") # Inner menu item area (above dialog body) - _menu_win = _styled_region("list") + _s.menu_win = _styled_region("list") # Help text window for show-help mode (above dialog, initially hidden) - _help_win = _styled_region("show-help") + _s.help_win = _styled_region("show-help") # Shadow regions -- created in _resize_main() - _dlg_bottom_shadow = None - _dlg_right_shadow = None + _s.dlg_bottom_shadow = None + _s.dlg_right_shadow = None # Currently focused button (0=Select, 1=Exit, 2=Help, 3=Save, 4=Load) - _active_button = 0 + _s.active_button = 0 # The rows we'd like the nodes in the parent menus to appear on. This # prevents the scroll from jumping around when going in and out of menus. - _parent_screen_rows = [] + _s.parent_screen_rows = [] # Initial state - _cur_menu = _kconf.top_node - _shown = _shown_nodes(_cur_menu) - _sel_node_i = _menu_scroll = 0 + _s.cur_menu = _s.kconf.top_node + _s.shown = _shown_nodes(_s.cur_menu) + _s.sel_node_i = _s.menu_scroll = 0 - _show_help = _show_name = False + _s.show_help = _s.show_name = False # Give regions their initial size _resize_main() @@ -857,14 +887,10 @@ def _resize_main(): # Row dlg_h-3: separator (LTEE + HLINE + RTEE) # Row dlg_h-2: buttons # Row dlg_h-1: bottom border - # Menu items region (_menu_win) overlaid inside the inner box + # Menu items region (_s.menu_win) overlaid inside the inner box - global _menu_scroll - global _dlg_bottom_shadow - global _dlg_right_shadow - - screen_height = _term.height - screen_width = _term.width + screen_height = _s.term.height + screen_width = _s.term.width # Dialog dimensions -- matching mconf/lxdialog/menubox.c dialog_menu() dlg_height = screen_height - 4 @@ -883,7 +909,7 @@ def _resize_main(): # In show-help mode, steal rows from the menu area for help text help_in_dialog = 0 - if _show_help: + if _s.show_help: help_in_dialog = min(_SHOW_HELP_HEIGHT, max(menu_height - 2, 0)) menu_height = max(menu_height - help_in_dialog, 1) @@ -901,38 +927,38 @@ def _resize_main(): # --- Resize and position regions --- # Screen background - _screen_win.resize(screen_height, screen_width) - _screen_win.move(0, 0) - _screen_win.fill(_style["screen"]) + _s.screen_win.resize(screen_height, screen_width) + _s.screen_win.move(0, 0) + _s.screen_win.fill(_style["screen"]) # Dialog body - _dialog_win.resize(dlg_height, dlg_width) - _dialog_win.move(dlg_y, dlg_x) - _dialog_win.fill(_style["body"]) + _s.dialog_win.resize(dlg_height, dlg_width) + _s.dialog_win.move(dlg_y, dlg_x) + _s.dialog_win.fill(_style["body"]) # Menu items (positioned inside the inner menu box of the dialog) - _menu_win.resize(menu_height, menu_width) - _menu_win.move(dlg_y + box_y + 1, dlg_x + box_x + 1) - _menu_win.fill(_style["list"]) + _s.menu_win.resize(menu_height, menu_width) + _s.menu_win.move(dlg_y + box_y + 1, dlg_x + box_x + 1) + _s.menu_win.fill(_style["list"]) # Help window -- positioned below inner menu box in show-help mode, # or moved off-screen when not needed - if _show_help and help_in_dialog > 0: + if _s.show_help and help_in_dialog > 0: help_y = dlg_y + box_y + menu_height + 2 - _help_win.resize(help_in_dialog, menu_width) - _help_win.move(help_y, dlg_x + box_x + 1) - _help_win.fill(_style["show-help"]) + _s.help_win.resize(help_in_dialog, menu_width) + _s.help_win.move(help_y, dlg_x + box_x + 1) + _s.help_win.fill(_style["show-help"]) else: - _help_win.resize(1, 1) - _help_win.move(screen_height, 0) # off-screen + _s.help_win.resize(1, 1) + _s.help_win.move(screen_height, 0) # off-screen # Shadow regions for the dialog - _close_shadow_windows(_dlg_bottom_shadow, _dlg_right_shadow) - _dlg_bottom_shadow, _dlg_right_shadow = _create_shadow_for_win(_dialog_win) + _close_shadow_windows(_s.dlg_bottom_shadow, _s.dlg_right_shadow) + _s.dlg_bottom_shadow, _s.dlg_right_shadow = _create_shadow_for_win(_s.dialog_win) # Adjust the scroll so that the selected node is still within the window - if _sel_node_i - _menu_scroll >= menu_height: - _menu_scroll = _sel_node_i - menu_height + 1 + if _s.sel_node_i - _s.menu_scroll >= menu_height: + _s.menu_scroll = _s.sel_node_i - menu_height + 1 def _height(win): @@ -950,11 +976,6 @@ def _enter_menu(menu): # # Returns False if 'menu' can't be entered. - global _cur_menu - global _shown - global _sel_node_i - global _menu_scroll - if not menu.is_menuconfig: return False # Not a menu @@ -965,12 +986,12 @@ def _enter_menu(menu): # Remember where the current node appears on the screen, so we can try # to get it to appear in the same place when we leave the menu - _parent_screen_rows.append(_sel_node_i - _menu_scroll) + _s.parent_screen_rows.append(_s.sel_node_i - _s.menu_scroll) # Jump into menu - _cur_menu = menu - _shown = shown_sub - _sel_node_i = _menu_scroll = 0 + _s.cur_menu = menu + _s.shown = shown_sub + _s.sel_node_i = _s.menu_scroll = 0 if isinstance(menu.item, Choice): _select_selected_choice_sym() @@ -983,15 +1004,13 @@ def _select_selected_choice_sym(): # any. Does nothing if if the choice has no selection (is not visible/in y # mode). - global _sel_node_i - - choice = _cur_menu.item + choice = _s.cur_menu.item if choice.selection: # Search through all menu nodes to handle choice symbols being defined # in multiple locations for node in choice.selection.nodes: - if node in _shown: - _sel_node_i = _shown.index(node) + if node in _s.shown: + _s.sel_node_i = _s.shown.index(node) _center_vertically() return @@ -999,37 +1018,30 @@ def _select_selected_choice_sym(): def _jump_to(node): # Jumps directly to the menu node 'node' - global _cur_menu - global _shown - global _sel_node_i - global _menu_scroll - global _show_all - global _parent_screen_rows - # Clear remembered menu locations. We might not even have been in the # parent menus before. - _parent_screen_rows = [] + _s.parent_screen_rows = [] - old_show_all = _show_all + old_show_all = _s.show_all jump_into = (isinstance(node.item, Choice) or node.item == MENU) and node.list # If we're jumping to a non-empty choice or menu, jump to the first entry # in it instead of jumping to its menu node if jump_into: - _cur_menu = node + _s.cur_menu = node node = node.list else: - _cur_menu = uicommon.parent_menu(node) + _s.cur_menu = uicommon.parent_menu(node) - _shown = _shown_nodes(_cur_menu) - if node not in _shown: + _s.shown = _shown_nodes(_s.cur_menu) + if node not in _s.shown: # The node wouldn't be shown. Turn on show-all to show it. - _show_all = True - _shown = _shown_nodes(_cur_menu) + _s.show_all = True + _s.shown = _shown_nodes(_s.cur_menu) - _sel_node_i = _shown.index(node) + _s.sel_node_i = _s.shown.index(node) - if jump_into and not old_show_all and _show_all: + if jump_into and not old_show_all and _s.show_all: # If we're jumping into a choice or menu and were forced to turn on # show-all because the first entry wasn't visible, try turning it off. # That will land us at the first visible node if there are visible @@ -1040,7 +1052,7 @@ def _jump_to(node): # If we're jumping to a non-empty choice, jump to the selected symbol, if # any - if jump_into and isinstance(_cur_menu.item, Choice): + if jump_into and isinstance(_s.cur_menu.item, Choice): _select_selected_choice_sym() @@ -1048,34 +1060,29 @@ def _leave_menu(): # Jumps to the parent menu of the current menu. Does nothing if we're in # the top menu. - global _cur_menu - global _shown - global _sel_node_i - global _menu_scroll - - if _cur_menu is _kconf.top_node: + if _s.cur_menu is _s.kconf.top_node: return # Jump to parent menu - parent = uicommon.parent_menu(_cur_menu) - _shown = _shown_nodes(parent) + parent = uicommon.parent_menu(_s.cur_menu) + _s.shown = _shown_nodes(parent) try: - _sel_node_i = _shown.index(_cur_menu) + _s.sel_node_i = _s.shown.index(_s.cur_menu) except ValueError: # The parent actually does not contain the current menu (e.g., symbol # search). So we jump to the first node instead. - _sel_node_i = 0 + _s.sel_node_i = 0 - _cur_menu = parent + _s.cur_menu = parent # Try to make the menu entry appear on the same row on the screen as it did # before we entered the menu. - if _parent_screen_rows: + if _s.parent_screen_rows: # The terminal might have shrunk since we were last in the parent menu - screen_row = min(_parent_screen_rows.pop(), _height(_menu_win) - 1) - _menu_scroll = max(_sel_node_i - screen_row, 0) + screen_row = min(_s.parent_screen_rows.pop(), _height(_s.menu_win) - 1) + _s.menu_scroll = max(_s.sel_node_i - screen_row, 0) else: # No saved parent menu locations, meaning we jumped directly to some # node earlier @@ -1086,113 +1093,95 @@ def _select_next_menu_entry(): # Selects the menu entry after the current one, adjusting the scroll if # necessary. Does nothing if we're already at the last menu entry. - global _sel_node_i - global _menu_scroll - - if _sel_node_i < len(_shown) - 1: + if _s.sel_node_i < len(_s.shown) - 1: # Jump to the next node - _sel_node_i += 1 + _s.sel_node_i += 1 # If the new node is sufficiently close to the edge of the menu window # (as determined by _SCROLL_OFFSET), increase the scroll by one. This # gives nice and non-jumpy behavior even when - # _SCROLL_OFFSET >= _height(_menu_win). - if _sel_node_i >= _menu_scroll + _height( - _menu_win - ) - _SCROLL_OFFSET and _menu_scroll < _max_scroll(_shown, _menu_win): - - _menu_scroll += 1 + # _SCROLL_OFFSET >= _height(_s.menu_win). + last_visible = _s.menu_scroll + _height(_s.menu_win) - _SCROLL_OFFSET + if _s.sel_node_i >= last_visible and _s.menu_scroll < _max_scroll( + _s.shown, _s.menu_win + ): + _s.menu_scroll += 1 def _select_prev_menu_entry(): # Selects the menu entry before the current one, adjusting the scroll if # necessary. Does nothing if we're already at the first menu entry. - global _sel_node_i - global _menu_scroll - - if _sel_node_i > 0: + if _s.sel_node_i > 0: # Jump to the previous node - _sel_node_i -= 1 + _s.sel_node_i -= 1 # See _select_next_menu_entry() - if _sel_node_i < _menu_scroll + _SCROLL_OFFSET: - _menu_scroll = max(_menu_scroll - 1, 0) + if _s.sel_node_i < _s.menu_scroll + _SCROLL_OFFSET: + _s.menu_scroll = max(_s.menu_scroll - 1, 0) def _select_last_menu_entry(): # Selects the last menu entry in the current menu - global _sel_node_i - global _menu_scroll - - _sel_node_i = len(_shown) - 1 - _menu_scroll = _max_scroll(_shown, _menu_win) + _s.sel_node_i = len(_s.shown) - 1 + _s.menu_scroll = _max_scroll(_s.shown, _s.menu_win) def _select_first_menu_entry(): # Selects the first menu entry in the current menu - global _sel_node_i - global _menu_scroll - - _sel_node_i = _menu_scroll = 0 + _s.sel_node_i = _s.menu_scroll = 0 def _toggle_show_all(): # Toggles show-all mode on/off. If turning it off would give no visible # items in the current menu, it is left on. - global _show_all - global _shown - global _sel_node_i - global _menu_scroll - # Row on the screen the cursor is on. Preferably we want the same row to # stay highlighted. - old_row = _sel_node_i - _menu_scroll + old_row = _s.sel_node_i - _s.menu_scroll - _show_all = not _show_all - # List of new nodes to be shown after toggling _show_all - new_shown = _shown_nodes(_cur_menu) + _s.show_all = not _s.show_all + # List of new nodes to be shown after toggling _s.show_all + new_shown = _shown_nodes(_s.cur_menu) # Find a good node to select. The selected node might disappear if show-all # mode is turned off. # Select the previously selected node itself if it is still visible. If # there are visible nodes before it, select the closest one. - for node in _shown[_sel_node_i::-1]: + for node in _s.shown[_s.sel_node_i :: -1]: if node in new_shown: - _sel_node_i = new_shown.index(node) + _s.sel_node_i = new_shown.index(node) break else: # No visible nodes before the previously selected node. Select the # closest visible node after it instead. - for node in _shown[_sel_node_i + 1 :]: + for node in _s.shown[_s.sel_node_i + 1 :]: if node in new_shown: - _sel_node_i = new_shown.index(node) + _s.sel_node_i = new_shown.index(node) break else: # No visible nodes at all, meaning show-all was turned off inside # an invisible menu. Don't allow that, as the implementation relies # on always having a selected node. - _show_all = True + _s.show_all = True return - _shown = new_shown + _s.shown = new_shown # Try to make the cursor stay on the same row in the menu window. This # might be impossible if too many nodes have disappeared above the node. - _menu_scroll = max(_sel_node_i - old_row, 0) + _s.menu_scroll = max(_s.sel_node_i - old_row, 0) def _center_vertically(): # Centers the selected node vertically, if possible - global _menu_scroll - - _menu_scroll = min( - max(_sel_node_i - _height(_menu_win) // 2, 0), _max_scroll(_shown, _menu_win) + _s.menu_scroll = min( + max(_s.sel_node_i - _height(_s.menu_win) // 2, 0), + _max_scroll(_s.shown, _s.menu_win), ) @@ -1201,20 +1190,20 @@ def _draw_main(): # centered dialog with title/instructions/inner menu box/buttons, and # shadow. - screen_width = _term.width - dlg_h = _height(_dialog_win) - dlg_w = _width(_dialog_win) + screen_width = _s.term.width + dlg_h = _height(_s.dialog_win) + dlg_w = _width(_s.dialog_win) - menu_height = _height(_menu_win) - menu_width = _width(_menu_win) + menu_height = _height(_s.menu_win) + menu_width = _width(_s.menu_win) # --- Compute inner box position within the dialog --- # These must match _resize_main() calculations. help_in_dialog = 0 - if _show_help: + if _s.show_help: help_in_dialog = min(_SHOW_HELP_HEIGHT, max(dlg_h - 10 - 2, 0)) # Recalculate menu_height for positioning only (actual size is from - # the _menu_win region). + # the _s.menu_win region). box_y = dlg_h - menu_height - 5 - help_in_dialog box_x = (dlg_w - menu_width) // 2 - 1 box_y = max(box_y, 1) @@ -1228,17 +1217,17 @@ def _draw_main(): # --------------------------------------------------------------- # 1. Screen background # --------------------------------------------------------------- - _screen_win.clear() + _s.screen_win.clear() screen_style = _style["screen"] # Backtitle at row 0 (like mconf's dialog_clear() + backtitle) - _screen_win.write(0, 1, _kconf.mainmenu_text, screen_style) + _s.screen_win.write(0, 1, _s.kconf.mainmenu_text, screen_style) # Subtitle path at row 1 (like mconf's subtitle trail) subtitle_parts = [] - menu = _cur_menu - while menu is not _kconf.top_node: + menu = _s.cur_menu + while menu is not _s.kconf.top_node: subtitle_parts.append( menu.prompt[0] if menu.prompt else standard_sc_expr_str(menu.item) ) @@ -1249,26 +1238,26 @@ def _draw_main(): path_str = "" for part in subtitle_parts: path_str += Box.RARROW + " " + part + " " - _screen_win.write(1, 1, path_str[: screen_width - 2], screen_style) + _s.screen_win.write(1, 1, path_str[: screen_width - 2], screen_style) hline_start = min(1 + len(path_str), screen_width - 1) else: hline_start = 1 # Fill rest of row 1 with horizontal line for j in range(hline_start, screen_width - 1): - _screen_win.write_char(1, j, Box.HLINE, screen_style) + _s.screen_win.write_char(1, j, Box.HLINE, screen_style) # Mode indicators on screen background (show-name/show-all/show-help) enabled_modes = [] - if _show_help: + if _s.show_help: enabled_modes.append("show-help") - if _show_name: + if _s.show_name: enabled_modes.append("show-name") - if _show_all: + if _s.show_all: enabled_modes.append("show-all") if enabled_modes: mode_str = "[" + "+".join(enabled_modes) + "]" - _screen_win.write( + _s.screen_win.write( 0, max(screen_width - len(mode_str) - 1, 0), mode_str, @@ -1278,21 +1267,21 @@ def _draw_main(): # --------------------------------------------------------------- # 2. Dialog body # --------------------------------------------------------------- - _dialog_win.clear() + _s.dialog_win.clear() body_style = _style["body"] border_style = _style.get("border", _style["frame"]) # Outer dialog box: body for interior, frame for border # Matches mconf: draw_box(dialog, 0, 0, h, w, dlg.dialog.atr, dlg.border.atr) - _draw_box(_dialog_win, 0, 0, dlg_h, dlg_w, body_style, border_style) + _draw_box(_s.dialog_win, 0, 0, dlg_h, dlg_w, body_style, border_style) # Separator between menu area and buttons - _draw_separator(_dialog_win, dlg_h - 3, dlg_w) + _draw_separator(_s.dialog_win, dlg_h - 3, dlg_w) # Title centered in top border (like mconf's print_title()) - title = _cur_menu.prompt[0] if _cur_menu.prompt else _kconf.mainmenu_text - _draw_title(_dialog_win, title, dlg_w) + title = _s.cur_menu.prompt[0] if _s.cur_menu.prompt else _s.kconf.mainmenu_text + _draw_title(_s.dialog_win, title, dlg_w) # Instruction text (autowrapped, like mconf's print_autowrap()) # mconf: print_autowrap(dialog, prompt, width - 2, 1, 3) @@ -1305,21 +1294,21 @@ def _draw_main(): # Entire text fits -- center it (like mconf's # print_text_centered) cx = (dlg_w - len(_MENU_INSTRUCTIONS)) // 2 - _dialog_win.write(1, cx, _MENU_INSTRUCTIONS, body_style) + _s.dialog_win.write(1, cx, _MENU_INSTRUCTIONS, body_style) else: inst_lines = textwrap.wrap(_MENU_INSTRUCTIONS, inst_width) for idx, line in enumerate(inst_lines): row = 1 + idx if row >= box_y: break - _dialog_win.write(row, inst_x, line, body_style) + _s.dialog_win.write(row, inst_x, line, body_style) # Inner menu box: box_style=menubox-border, border_style=menubox # (matching mconf's draw_box for the menu area) inner_box_style = _style.get("menubox-border", _style["frame"]) inner_border_style = _style.get("menubox", _style["list"]) _draw_box( - _dialog_win, + _s.dialog_win, box_y, box_x, menu_height + 2, @@ -1330,9 +1319,9 @@ def _draw_main(): # Scroll arrows (like mconf's print_arrows()) _draw_scroll_arrows( - _dialog_win, - len(_shown), - _menu_scroll, + _s.dialog_win, + len(_s.shown), + _s.menu_scroll, box_y, box_x + item_x + 1, menu_height, @@ -1341,47 +1330,47 @@ def _draw_main(): ) # Buttons (like mconf's print_buttons()) - _draw_main_buttons(_dialog_win, dlg_h, dlg_w) + _draw_main_buttons(_s.dialog_win, dlg_h, dlg_w) # --------------------------------------------------------------- - # 3. Menu items (drawn into _menu_win, positioned inside inner box) + # 3. Menu items (drawn into _s.menu_win, positioned inside inner box) # --------------------------------------------------------------- - _menu_win.clear() + _s.menu_win.clear() text_width = menu_width - item_x - for i in range(_menu_scroll, min(_menu_scroll + menu_height, len(_shown))): - node = _shown[i] + for i in range(_s.menu_scroll, min(_s.menu_scroll + menu_height, len(_s.shown))): + node = _s.shown[i] - if uicommon.visible(node) or not _show_all: - style = _style["selection" if i == _sel_node_i else "list"] + if uicommon.visible(node) or not _s.show_all: + style = _style["selection" if i == _s.sel_node_i else "list"] else: - style = _style["inv-selection" if i == _sel_node_i else "inv-list"] + style = _style["inv-selection" if i == _s.sel_node_i else "inv-list"] # Clear entire row with list style, then draw text - _menu_win.write(i - _menu_scroll, 0, " " * menu_width, _style["list"]) + _s.menu_win.write(i - _s.menu_scroll, 0, " " * menu_width, _style["list"]) node_text = _node_str(node) node_text = node_text[:text_width].ljust(text_width) - _menu_win.write(i - _menu_scroll, item_x, node_text, style) + _s.menu_win.write(i - _s.menu_scroll, item_x, node_text, style) # --------------------------------------------------------------- # 4. Help text (show-help mode only) # --------------------------------------------------------------- - if _show_help and _help_win.height > 1: - _help_win.clear() - node = _shown[_sel_node_i] + if _s.show_help and _s.help_win.height > 1: + _s.help_win.clear() + node = _s.shown[_s.sel_node_i] sh_style = _style["show-help"] if isinstance(node.item, (Symbol, Choice)) and node.help: - help_lines = textwrap.wrap(node.help, _width(_help_win)) - for i in range(min(_height(_help_win), len(help_lines))): - _help_win.write(i, 0, help_lines[i], sh_style) + help_lines = textwrap.wrap(node.help, _width(_s.help_win)) + for i in range(min(_height(_s.help_win), len(help_lines))): + _s.help_win.write(i, 0, help_lines[i], sh_style) else: - _help_win.write(0, 0, "(no help)", sh_style) + _s.help_win.write(0, 0, "(no help)", sh_style) # --------------------------------------------------------------- # 5. Shadow # --------------------------------------------------------------- - _refresh_shadow_windows(_dlg_bottom_shadow, _dlg_right_shadow) + _refresh_shadow_windows(_s.dlg_bottom_shadow, _s.dlg_right_shadow) def _draw_scroll_arrows( @@ -1430,7 +1419,7 @@ def _draw_main_buttons(win, dlg_h, dlg_w): bx = start_x + i * 12 if bx + len(label) + 2 > dlg_w: break - _print_button(win, label, button_y, bx, i == _active_button) + _print_button(win, label, button_y, bx, i == _s.active_button) def _shown_nodes(menu): @@ -1441,7 +1430,7 @@ def rec(node): res = [] while node: - if uicommon.visible(node) or _show_all: + if uicommon.visible(node) or _s.show_all: res.append(node) if node.list and not node.is_menuconfig: # Nodes from implicit menu created from dependencies. Will @@ -1568,16 +1557,14 @@ def _set_sel_node_tri_val(tri_val): # Sets the value of the currently selected menu entry to 'tri_val', if that # value can be assigned - sc = _shown[_sel_node_i].item + sc = _s.shown[_s.sel_node_i].item if isinstance(sc, (Symbol, Choice)) and tri_val in sc.assignable: _set_val(sc, tri_val) def _set_val(sc, val): # Wrapper around Symbol/Choice.set_value() for updating the menu state and - # _conf_changed - - global _conf_changed + # _s.conf_changed # Use the string representation of tristate values. This makes the format # consistent for all symbol types. @@ -1586,7 +1573,7 @@ def _set_val(sc, val): if val != sc.str_value: sc.set_value(val) - _conf_changed = True + _s.conf_changed = True # Changing the value of the symbol might have changed what items in the # current menu are visible. Recalculate the state. @@ -1601,24 +1588,20 @@ def _update_menu(): # If possible, preserves the location of the cursor on the screen when # items are added/removed above the selected item. - global _shown - global _sel_node_i - global _menu_scroll - # Row on the screen the cursor was on - old_row = _sel_node_i - _menu_scroll + old_row = _s.sel_node_i - _s.menu_scroll - sel_node = _shown[_sel_node_i] + sel_node = _s.shown[_s.sel_node_i] # New visible nodes - _shown = _shown_nodes(_cur_menu) + _s.shown = _shown_nodes(_s.cur_menu) # New index of selected node - _sel_node_i = _shown.index(sel_node) + _s.sel_node_i = _s.shown.index(sel_node) # Try to make the cursor stay on the same row in the menu window. This # might be impossible if too many nodes have disappeared above the node. - _menu_scroll = max(_sel_node_i - old_row, 0) + _s.menu_scroll = max(_s.sel_node_i - old_row, 0) def _input_dialog(title, initial_text, info_text=None): @@ -1645,7 +1628,7 @@ def _input_dialog(title, initial_text, info_text=None): # Give the input dialog its initial size _resize_input_dialog(win, title, info_lines) - _term.show_cursor(very_visible=True) + _s.term.show_cursor(very_visible=True) # Input field text s = initial_text @@ -1670,9 +1653,9 @@ def edit_width(): _refresh_shadow_windows(bottom_shadow, right_shadow) - _term.update() + _s.term.update() - c = _term.read_key() + c = _s.term.read_key() if c == Key.RESIZE: _resize_main() @@ -1681,11 +1664,11 @@ def edit_width(): bottom_shadow, right_shadow = _create_shadow_for_win(win) elif c == "\n": - _term.hide_cursor() + _s.term.hide_cursor() return s elif c == "\x1b": # ESC - _term.hide_cursor() + _s.term.hide_cursor() return None elif c == "\0": # NUL, ignore @@ -1702,7 +1685,7 @@ def edit_width(): def _resize_input_dialog(win, title, info_lines): # Resizes the input dialog to a size appropriate for the terminal size - screen_height, screen_width = _term.height, _term.width + screen_height, screen_width = _s.term.height, _s.term.width win_height = 5 if info_lines: @@ -1739,17 +1722,13 @@ def _draw_input_dialog(win, title, info_lines, s, i, hscroll): # Truncate then pad to interior width so body_style covers frame bg win.write(4 + linenr, 2, line[:edit_width].ljust(edit_width), _style["body"]) - _term.set_cursor(win, 2, 2 + i - hscroll) + _s.term.set_cursor(win, 2, 2 + i - hscroll) def _load_dialog(): # Dialog for loading a new configuration - global _conf_changed - global _conf_filename - global _show_all - - if _conf_changed: + if _s.conf_changed: c = _key_dialog( "Load", "You have unsaved changes. Load new\n" @@ -1762,7 +1741,7 @@ def _load_dialog(): if c is None or c == "c": return - filename = _conf_filename + filename = _s.conf_filename while True: filename = _input_dialog("File to load", filename, _load_save_info()) if filename is None: @@ -1771,13 +1750,13 @@ def _load_dialog(): filename = os.path.expanduser(filename) if _try_load(filename): - _conf_filename = filename - _conf_changed = _needs_save() + _s.conf_filename = filename + _s.conf_changed = _needs_save() # Turn on show-all mode if the selected node is not visible after - # loading the new configuration. _shown still holds the old state. - if _shown[_sel_node_i] not in _shown_nodes(_cur_menu): - _show_all = True + # loading the new configuration. _s.shown still holds the old state. + if _s.shown[_s.sel_node_i] not in _shown_nodes(_s.cur_menu): + _s.show_all = True _update_menu() @@ -1795,7 +1774,7 @@ def _try_load(filename): # Configuration file to load try: - _kconf.load_config(filename) + _s.kconf.load_config(filename) return True except OSError as e: _error( @@ -1893,9 +1872,9 @@ def _key_dialog(title, text, keys): _refresh_shadow_windows(bottom_shadow, right_shadow) - _term.update() + _s.term.update() - c = _term.read_key() + c = _s.term.read_key() if c == Key.RESIZE: _resize_main() @@ -1919,7 +1898,7 @@ def _key_dialog(title, text, keys): def _resize_key_dialog(win, text): # Resizes the key dialog to a size appropriate for the terminal size - screen_height, screen_width = _term.height, _term.width + screen_height, screen_width = _s.term.height, _s.term.width lines = text.split("\n") @@ -1968,7 +1947,7 @@ def _button_dialog(title, text, buttons, default_button=0): lines = text.split("\n") # Height: border(1) + text lines + blank + separator(1) + buttons + # border(1) = 1 + len(lines) + 1 + 1 + 1 + 1 = len(lines) + 5 - win_height = min(len(lines) + 5, _term.height - 4) + win_height = min(len(lines) + 5, _s.term.height - 4) # Calculate width from longest line and button row # Button row width includes buttons + spacing between them # 2 buttons: spacing 6, 3+ buttons: spacing 4 @@ -1978,11 +1957,11 @@ def _button_dialog(title, text, buttons, default_button=0): ) win_width = min( max(max(len(line) for line in lines) + 4, button_row_width + 4), - _term.width - 4, + _s.term.width - 4, ) win.resize(win_height, win_width) - win.move((_term.height - win_height) // 2, (_term.width - win_width) // 2) + win.move((_s.term.height - win_height) // 2, (_s.term.width - win_width) // 2) bottom_shadow, right_shadow = _create_shadow_for_win(win) @@ -2025,23 +2004,23 @@ def _button_dialog(title, text, buttons, default_button=0): _refresh_shadow_windows(bottom_shadow, right_shadow) - _term.update() + _s.term.update() # Handle input - c = _term.read_key() + c = _s.term.read_key() if c == Key.RESIZE: _resize_main() # Recompute dimensions for new terminal size - win_height = min(len(lines) + 5, _term.height - 4) + win_height = min(len(lines) + 5, _s.term.height - 4) win_width = min( max(max(len(line) for line in lines) + 4, button_row_width + 4), - _term.width - 4, + _s.term.width - 4, ) win.resize(win_height, win_width) win.move( - (_term.height - win_height) // 2, - (_term.width - win_width) // 2, + (_s.term.height - win_height) // 2, + (_s.term.width - win_width) // 2, ) _close_shadow_windows(bottom_shadow, right_shadow) bottom_shadow, right_shadow = _create_shadow_for_win(win) @@ -2217,20 +2196,20 @@ def _create_shadow_windows(y, x, height, width, right_y_offset=1): # Bottom shadow region (1 line high, width wide, offset by 2 on x) bottom_shadow = None - if y + height < _term.height and x + 2 + width <= _term.width: + if y + height < _s.term.height and x + 2 + width <= _s.term.width: try: - bottom_shadow = _term.region(1, width, y + height, x + 2) + bottom_shadow = _s.term.region(1, width, y + height, x + 2) bottom_shadow.fill(shadow_style) except Exception: pass # Right shadow region right_shadow = None - if x + width + 2 <= _term.width and y + height <= _term.height: + if x + width + 2 <= _s.term.width and y + height <= _s.term.height: try: shadow_height = height - right_y_offset + 1 if shadow_height > 0: - right_shadow = _term.region( + right_shadow = _s.term.region( shadow_height, 2, y + right_y_offset, x + width ) right_shadow.fill(shadow_style) @@ -2253,7 +2232,7 @@ def _close_shadow_windows(bottom_shadow, right_shadow): def _refresh_shadow_windows(bottom_shadow, right_shadow): - # Shadow regions are composited automatically by _term.update(). + # Shadow regions are composited automatically by _s.term.update(). # Just clear and refill them to ensure correct content. for shadow in (bottom_shadow, right_shadow): if shadow: @@ -2291,7 +2270,7 @@ def _jump_to_matches(search_string): # with re.search(), which matches anywhere in the string. # # It's not horrible either way. Just a bit smoother. - prefix = _kconf.config_prefix.lower() + prefix = _s.kconf.config_prefix.lower() prefix_len = len(prefix) regex_searches = [ @@ -2383,13 +2362,13 @@ def _jump_to_dialog(): def _jump_to_shadows(): # Shadow windows cover the dialog area (everything except help win) _close_shadow_windows(bottom_shadow, right_shadow) - sh, sw = _term.height, _term.width + sh, sw = _s.term.height, _s.term.width dh = sh - len(_JUMP_TO_HELP_LINES) - 1 return _create_shadow_windows(0, 0, dh, sw, right_y_offset=1) bottom_shadow, right_shadow = _jump_to_shadows() - _term.show_cursor(very_visible=True) + _s.term.show_cursor(very_visible=True) # Functional variants of _select_{next,prev}_menu_entry() that return # new (sel_node_i, scroll) tuples instead of mutating globals, to @@ -2443,18 +2422,18 @@ def select_prev_match(): # Refresh shadow windows after all other windows _refresh_shadow_windows(bottom_shadow, right_shadow) - _term.update() + _s.term.update() - c = _term.read_key() + c = _s.term.read_key() if c == "\n": if matches: _jump_to(matches[sel_node_i]) - _term.hide_cursor() + _s.term.hide_cursor() return True elif c == "\x1b": # ESC - _term.hide_cursor() + _s.term.hide_cursor() return False elif c == Key.RESIZE: @@ -2469,9 +2448,9 @@ def select_prev_match(): elif c == "\x06": # Ctrl-F if matches: - _term.hide_cursor() + _s.term.hide_cursor() _info_dialog(matches[sel_node_i], True) - _term.show_cursor(very_visible=True) + _s.term.show_cursor(very_visible=True) scroll = _resize_jump_to_dialog( edit_box, matches_win, bot_sep_win, help_win, sel_node_i, scroll @@ -2515,49 +2494,43 @@ def select_prev_match(): r.close() -_cached_sc_nodes = [] - - def _sorted_sc_nodes(): # Returns a sorted list of symbol and choice nodes to search. The symbol # nodes appear first, sorted by name, and then the choice nodes, sorted by # prompt and (secondarily) name. - if not _cached_sc_nodes: + if not _s.cached_sc_nodes: # Add symbol nodes - for sym in sorted(_kconf.unique_defined_syms, key=lambda sym: sym.name): - _cached_sc_nodes.extend(sym.nodes) + for sym in sorted(_s.kconf.unique_defined_syms, key=lambda sym: sym.name): + _s.cached_sc_nodes.extend(sym.nodes) # Add choice nodes - choices = sorted(_kconf.unique_choices, key=lambda choice: choice.name or "") + choices = sorted(_s.kconf.unique_choices, key=lambda choice: choice.name or "") - _cached_sc_nodes.extend( + _s.cached_sc_nodes.extend( sorted( [node for choice in choices for node in choice.nodes], key=lambda node: node.prompt[0] if node.prompt else "", ) ) - return _cached_sc_nodes - - -_cached_menu_comment_nodes = [] + return _s.cached_sc_nodes def _sorted_menu_comment_nodes(): # Returns a list of menu and comment nodes to search, sorted by prompt, # with the menus first - if not _cached_menu_comment_nodes: + if not _s.cached_menu_comment_nodes: def prompt_text(mc): return mc.prompt[0] - _cached_menu_comment_nodes.extend(sorted(_kconf.menus, key=prompt_text)) - _cached_menu_comment_nodes.extend(sorted(_kconf.comments, key=prompt_text)) + _s.cached_menu_comment_nodes.extend(sorted(_s.kconf.menus, key=prompt_text)) + _s.cached_menu_comment_nodes.extend(sorted(_s.kconf.comments, key=prompt_text)) - return _cached_menu_comment_nodes + return _s.cached_menu_comment_nodes def _resize_jump_to_dialog( @@ -2568,7 +2541,7 @@ def _resize_jump_to_dialog( # Returns the new scroll index. We adjust the scroll if needed so that the # selected node stays visible. - screen_height, screen_width = _term.height, _term.width + screen_height, screen_width = _s.term.height, _s.term.width bot_sep_win.resize(1, screen_width) @@ -2715,7 +2688,7 @@ def _draw_jump_to_dialog( visible_s = s[hscroll : hscroll + edit_width] edit_box.write(1, 1, visible_s, _style["jump-edit"]) - _term.set_cursor(edit_box, 1, 1 + s_i - hscroll) + _s.term.set_cursor(edit_box, 1, 1 + s_i - hscroll) def _info_dialog(node, from_jump_to_dialog): @@ -2749,9 +2722,9 @@ def _info_dialog(node, from_jump_to_dialog): _refresh_shadow_windows(bottom_shadow, right_shadow) - _term.update() + _s.term.update() - c = _term.read_key() + c = _s.term.read_key() if c == Key.RESIZE: _resize_main() @@ -2832,7 +2805,7 @@ def _resize_info_dialog(win, node, lines): # full-screen: height = screen_height - 4, width = screen_width - 5, # centered on the terminal. - screen_height, screen_width = _term.height, _term.width + screen_height, screen_width = _s.term.height, _s.term.width # Match dialog_textbox() sizing dlg_height = screen_height - 4 @@ -2941,12 +2914,12 @@ def _info_str_mconf(node): if n.prompt: s += f"Defined at {n.filename}:{n.linenr}\n" s += f" Prompt: {n.prompt[0]}\n" - if n.dep is not _kconf.y: + if n.dep is not _s.kconf.y: s += f" Depends on: {uicommon.expr_str_with_values(n.dep)}\n" # Location hierarchy (matching get_prompt_str in mconf) submenu = [] m = n - while m is not _kconf.top_node and len(submenu) < 8: + while m is not _s.kconf.top_node and len(submenu) < 8: submenu.append(m) m = m.parent s += " Location:\n" @@ -2966,16 +2939,19 @@ def _info_str_mconf(node): for n in sc.nodes: if not n.prompt: s += f"Defined at {n.filename}:{n.linenr}\n" - if n.dep is not _kconf.y: + if n.dep is not _s.kconf.y: s += f" Depends on: {uicommon.expr_str_with_values(n.dep)}\n" # Selects (symbols only) if isinstance(sc, Symbol) and sc.selects: - sel_strs = [uicommon.expr_str_with_values(sel_sym) for sel_sym, cond in sc.orig_selects] + sel_strs = [ + uicommon.expr_str_with_values(sel_sym) + for sel_sym, cond in sc.orig_selects + ] s += "Selects: {}\n".format(" && ".join(sel_strs)) # Selected by - if isinstance(sc, Symbol) and sc.rev_dep is not _kconf.n: + if isinstance(sc, Symbol) and sc.rev_dep is not _s.kconf.n: for val, label in ( (2, "Selected by [y]:"), (1, "Selected by [m]:"), @@ -2993,11 +2969,14 @@ def _info_str_mconf(node): # Implies (symbols only) if isinstance(sc, Symbol) and sc.implies: - imp_strs = [uicommon.expr_str_with_values(imp_sym) for imp_sym, cond in sc.orig_implies] + imp_strs = [ + uicommon.expr_str_with_values(imp_sym) + for imp_sym, cond in sc.orig_implies + ] s += "Implies: {}\n".format(" && ".join(imp_strs)) # Implied by - if isinstance(sc, Symbol) and sc.weak_rev_dep is not _kconf.n: + if isinstance(sc, Symbol) and sc.weak_rev_dep is not _s.kconf.n: for val, label in ( (2, "Implied by [y]:"), (1, "Implied by [m]:"), @@ -3126,7 +3105,7 @@ def _direct_dep_info(sc): return ( "" - if sc.direct_dep is _kconf.y + if sc.direct_dep is _s.kconf.y else f"Direct dependencies (={TRI_TO_STR[expr_value(sc.direct_dep)]}):\n{uicommon.split_expr_info(sc.direct_dep, 2)}\n" ) @@ -3160,7 +3139,7 @@ def _defaults_info(sc): s += val.name s += "\n" - if cond is not _kconf.y: + if cond is not _s.kconf.y: s += f" Condition (={TRI_TO_STR[expr_value(cond)]}):\n{uicommon.split_expr_info(cond, 4)}" return s + "\n" @@ -3184,14 +3163,14 @@ def sis(expr, val, title): s = "" - if sym.rev_dep is not _kconf.n: + if sym.rev_dep is not _s.kconf.n: s += sis(sym.rev_dep, 2, "Symbols currently y-selecting this symbol:\n") s += sis(sym.rev_dep, 1, "Symbols currently m-selecting this symbol:\n") s += sis( sym.rev_dep, 0, "Symbols currently n-selecting this symbol (no effect):\n" ) - if sym.weak_rev_dep is not _kconf.n: + if sym.weak_rev_dep is not _s.kconf.n: s += sis(sym.weak_rev_dep, 2, "Symbols currently y-implying this symbol:\n") s += sis(sym.weak_rev_dep, 1, "Symbols currently m-implying this symbol:\n") s += sis( @@ -3231,7 +3210,7 @@ def _menu_path_info(node): path = "" - while node.parent is not _kconf.top_node: + while node.parent is not _s.kconf.top_node: node = node.parent # Promptless choices might appear among the parents. Use @@ -3257,7 +3236,7 @@ def _styled_region(style): # character. The initial dimensions are (1, 1), so the region needs to be # sized and positioned separately. - win = _term.region(1, 1) + win = _s.term.region(1, 1) win.fill(_style[style]) return win @@ -3483,7 +3462,7 @@ def _should_show_name(node): # The 'not node.prompt' case only hits in show-all mode, for promptless # symbols and choices - return not node.prompt or (_show_name and isinstance(node.item, (Symbol, Choice))) + return not node.prompt or (_s.show_name and isinstance(node.item, (Symbol, Choice))) def _value_str(node): @@ -3553,13 +3532,13 @@ def _warn(*args): # Temporarily exits terminal mode and prints a warning to stderr. # The warning would get lost in terminal mode. try: - _term.suspend() + _s.term.suspend() except Exception: pass print("menuconfig warning: ", end="", file=sys.stderr) print(*args, file=sys.stderr) try: - _term.resume() + _s.term.resume() except Exception: pass diff --git a/scripts/benchmark.py b/scripts/benchmark.py index 07a59be..a539976 100755 --- a/scripts/benchmark.py +++ b/scripts/benchmark.py @@ -105,8 +105,9 @@ def bench_shown_nodes(kconf, repeat): """menuconfig._shown_nodes() over every menu -- the TUI's per-redraw walk.""" import menuconfig - menuconfig._kconf = kconf - menuconfig._show_all = False + menuconfig._s = menuconfig._State() + menuconfig._s.kconf = kconf + menuconfig._s.show_all = False return _bench_menu_walk(kconf, repeat, menuconfig._shown_nodes) @@ -114,9 +115,10 @@ def bench_node_str(kconf, repeat): """menuconfig._node_str() for every node -- one call per visible row.""" import menuconfig - menuconfig._kconf = kconf - menuconfig._show_all = True - menuconfig._show_name = False + menuconfig._s = menuconfig._State() + menuconfig._s.kconf = kconf + menuconfig._s.show_all = True + menuconfig._s.show_name = False nodes = list(kconf.node_iter()) diff --git a/tests/test_uinav.py b/tests/test_uinav.py new file mode 100644 index 0000000..d67a310 --- /dev/null +++ b/tests/test_uinav.py @@ -0,0 +1,192 @@ +# Copyright (c) 2011-2019 Ulf Magnusson +# SPDX-License-Identifier: ISC +# +# Menu navigation and scrolling tests for menuconfig. +# +# _enter_menu()/_leave_menu() and the four _select_*_menu_entry() functions are +# the whole cursor state machine: between them they write _sel_node_i and +# _menu_scroll from a dozen places, and getting the pair out of step is what +# makes a TUI scroll to the wrong row or index past the end of the list. +# +# None of this was reachable before the interface state moved into +# menuconfig._State. The functions read and wrote module globals that only +# _menuconfig() ever set up, so driving one meant starting a terminal. Now a +# test builds a _State, gives it a window with a height, and calls them. + +import locale + +import pytest + +import menuconfig +from conftest import named_menu +from kconfiglib import Kconfig + + +class FakeWin: + """Stands in for a rawterm region. _height() only reads .height.""" + + def __init__(self, height): + self.height = height + + +@pytest.fixture(scope="module") +def kconf(): + return Kconfig("tests/Kuirender", warn=False) + + +@pytest.fixture +def mc(kconf, monkeypatch): + """menuconfig positioned at the top menu, in a 5-row window.""" + state = menuconfig._State() + state.kconf = kconf + state.show_all = True + state.menu_win = FakeWin(5) + state.cur_menu = kconf.top_node + monkeypatch.setattr(menuconfig, "_s", state) + # After the install, not before: _shown_nodes() reads show_all off the + # module's _s rather than off anything passed in, so building the list + # first silently used the previous state's value and dropped the + # promptless nodes this fixture exists to show. + state.shown = menuconfig._shown_nodes(kconf.top_node) + return menuconfig + + +def test_the_top_menu_has_enough_rows_to_scroll(mc): + """Guards the fixture: the cases below are meaningless on a short list.""" + assert len(mc._s.shown) > mc._s.menu_win.height + + +def test_next_and_prev_walk_the_list(mc): + mc._select_next_menu_entry() + assert mc._s.sel_node_i == 1 + mc._select_prev_menu_entry() + assert mc._s.sel_node_i == 0 + + +def test_selection_stops_at_both_ends(mc): + mc._select_prev_menu_entry() + assert mc._s.sel_node_i == 0 + + mc._select_last_menu_entry() + last = len(mc._s.shown) - 1 + assert mc._s.sel_node_i == last + + mc._select_next_menu_entry() + assert mc._s.sel_node_i == last + + +def test_scroll_never_leaves_the_selection_off_screen(mc): + """The invariant the whole scroll offset dance exists to keep.""" + height = mc._s.menu_win.height + for _ in range(len(mc._s.shown) + 5): + mc._select_next_menu_entry() + assert mc._s.menu_scroll <= mc._s.sel_node_i < mc._s.menu_scroll + height + for _ in range(len(mc._s.shown) + 5): + mc._select_prev_menu_entry() + assert mc._s.menu_scroll <= mc._s.sel_node_i < mc._s.menu_scroll + height + + +def test_scroll_stops_at_max_scroll(mc): + """Asserting this right after _select_last_menu_entry() proves nothing: + that function assigns _max_scroll() to menu_scroll, so the comparison is + against the value just written. Walk there one entry at a time instead, + which is the path that has to respect the limit.""" + limit = mc._max_scroll(mc._s.shown, mc._s.menu_win) + assert limit > 0 + + for _ in range(len(mc._s.shown) + 10): + mc._select_next_menu_entry() + assert mc._s.menu_scroll <= limit + + assert mc._s.menu_scroll == limit + assert mc._s.sel_node_i == len(mc._s.shown) - 1 + + +def test_first_entry_resets_the_scroll(mc): + mc._select_last_menu_entry() + assert mc._s.menu_scroll > 0 + mc._select_first_menu_entry() + assert (mc._s.sel_node_i, mc._s.menu_scroll) == (0, 0) + + +def shown_node(mc, name): + for node in mc._s.shown: + if node.item is not None and getattr(node.item, "name", None) == name: + return node + raise AssertionError(name + " not shown in the top menu") + + +def test_entering_and_leaving_a_menu_restores_the_screen_row(mc, kconf): + """The reason _enter_menu() pushes onto parent_screen_rows at all.""" + menu = shown_node(mc, "MENUCONFIG_SYM") + mc._s.sel_node_i = mc._s.shown.index(menu) + # Two rows down from the top of the window, so the row survives the + # clamp in _leave_menu() + mc._s.menu_scroll = mc._s.sel_node_i - 2 + row_before = 2 + + assert mc._enter_menu(menu) is True + assert mc._s.cur_menu is menu + assert (mc._s.sel_node_i, mc._s.menu_scroll) == (0, 0) + + mc._leave_menu() + assert mc._s.cur_menu is kconf.top_node + assert mc._s.shown[mc._s.sel_node_i] is menu + assert mc._s.sel_node_i - mc._s.menu_scroll == row_before + assert mc._s.parent_screen_rows == [] + + +def test_a_shrunk_window_clamps_the_restored_row(mc): + """The terminal can shrink while we are inside the submenu.""" + menu = shown_node(mc, "MENUCONFIG_SYM") + mc._s.sel_node_i = mc._s.shown.index(menu) + mc._s.menu_scroll = 0 + assert mc._enter_menu(menu) is True + + mc._s.menu_win = FakeWin(3) + mc._leave_menu() + + # Restored to the last row of the smaller window rather than off-screen + assert mc._s.sel_node_i - mc._s.menu_scroll == 2 + assert mc._s.menu_scroll >= 0 + + +def test_an_empty_menu_is_never_entered(mc, kconf): + """_enter_menu() refuses, because the display needs a selected node.""" + empty = named_menu(kconf, "Empty menu") + before = (mc._s.cur_menu, mc._s.sel_node_i, mc._s.menu_scroll) + assert mc._enter_menu(empty) is False + assert (mc._s.cur_menu, mc._s.sel_node_i, mc._s.menu_scroll) == before + assert mc._s.parent_screen_rows == [] + + +def test_leaving_the_top_menu_does_nothing(mc, kconf): + mc._leave_menu() + assert mc._s.cur_menu is kconf.top_node + + +def test_a_second_run_does_not_inherit_the_first_runs_position( + mc, kconf, tmp_path, monkeypatch, capsys +): + """What the _State() reset in menuconfig() is for. + + This one drives the real entry point, which is the only way to cover the + reset, and that entry point has two process-wide side effects: it sets + the locale from the environment, and it loads a .config from the working + directory, printing as it goes. Left alone, the locale would stay changed + for every test that runs afterwards, including the byte-for-byte render + comparisons. So run it in an empty directory and put the locale back. + """ + monkeypatch.chdir(tmp_path) + saved = locale.setlocale(locale.LC_ALL) + try: + mc._select_last_menu_entry() + assert mc._s.sel_node_i > 0 + + menuconfig.menuconfig(kconf, headless=True) + finally: + locale.setlocale(locale.LC_ALL, saved) + capsys.readouterr() + assert menuconfig._s.sel_node_i == 0 + assert menuconfig._s.menu_scroll == 0 + assert menuconfig._s.parent_screen_rows == [] diff --git a/tests/test_uirender.py b/tests/test_uirender.py index 27b0e76..2063ff6 100644 --- a/tests/test_uirender.py +++ b/tests/test_uirender.py @@ -29,10 +29,12 @@ def kconf(): @pytest.fixture def mc(kconf, monkeypatch): - """menuconfig with its module globals set up for rendering.""" - monkeypatch.setattr(menuconfig, "_kconf", kconf, raising=False) - monkeypatch.setattr(menuconfig, "_show_all", True, raising=False) - monkeypatch.setattr(menuconfig, "_show_name", False, raising=False) + """menuconfig with its interface state set up for rendering.""" + state = menuconfig._State() + state.kconf = kconf + state.show_all = True + state.show_name = False + monkeypatch.setattr(menuconfig, "_s", state) return menuconfig @@ -145,7 +147,7 @@ def test_menuconfig_node_str_y_mode_choice_shows_selection(mc, kconf): def test_menuconfig_show_name_appends_symbol_name(mc, kconf, monkeypatch): - monkeypatch.setattr(mc, "_show_name", True) + monkeypatch.setattr(mc._s, "show_name", True) assert mc._node_str(node(kconf, "BOOL_SYM")) == "[ ] A bool (NEW)" @@ -248,9 +250,9 @@ def test_changeable_rejects_menus_and_comments(kconf): def test_shown_nodes_hides_promptless_symbols_until_show_all(mc, kconf, monkeypatch): """PROMPTLESS has no prompt, so it appears only in show-all mode.""" - monkeypatch.setattr(mc, "_show_all", False) + monkeypatch.setattr(mc._s, "show_all", False) hidden = mc._shown_nodes(kconf.top_node) - monkeypatch.setattr(mc, "_show_all", True) + monkeypatch.setattr(mc._s, "show_all", True) shown = mc._shown_nodes(kconf.top_node) promptless = node(kconf, "PROMPTLESS") @@ -261,14 +263,14 @@ def test_shown_nodes_hides_promptless_symbols_until_show_all(mc, kconf, monkeypa def test_shown_nodes_descends_into_a_menuconfig(mc, kconf, monkeypatch): - monkeypatch.setattr(mc, "_show_all", False) + monkeypatch.setattr(mc._s, "show_all", False) children = mc._shown_nodes(node(kconf, "MENUCONFIG_SYM")) assert children == [node(kconf, "UNDER_MENUCONFIG")] def test_shown_nodes_of_an_empty_menu_is_empty(mc, kconf, monkeypatch): """This is what makes _node_str() draw "----" instead of "--->".""" - monkeypatch.setattr(mc, "_show_all", False) + monkeypatch.setattr(mc._s, "show_all", False) assert mc._shown_nodes(named_menu(kconf, "Empty menu")) == [] assert mc._shown_nodes(named_menu(kconf, "A menu")) != [] diff --git a/tests/test_uisearch.py b/tests/test_uisearch.py index 94b882e..f1f64c2 100644 --- a/tests/test_uisearch.py +++ b/tests/test_uisearch.py @@ -227,10 +227,12 @@ def test_matches_are_pushed_to_the_tree(search): def mc_search(monkeypatch): """Returns a run() driving the real menuconfig._jump_to_matches().""" kconf = Kconfig("tests/Kuirender", warn=False) - monkeypatch.setattr(menuconfig, "_kconf", kconf, raising=False) - monkeypatch.setattr(menuconfig, "_show_all", True, raising=False) - monkeypatch.setattr(menuconfig, "_cached_sc_nodes", [], raising=False) - monkeypatch.setattr(menuconfig, "_cached_menu_comment_nodes", [], raising=False) + # A fresh _State() also brings empty search caches, so one test's sort + # order cannot leak into the next + state = menuconfig._State() + state.kconf = kconf + state.show_all = True + monkeypatch.setattr(menuconfig, "_s", state) def run(text): matches, bad_re = menuconfig._jump_to_matches(text) From ce86838689689a54ec32d2c50d9f2a023d23086d Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Mon, 21 Sep 2026 01:13:18 +0800 Subject: [PATCH 4/5] Reject properties on nodes that cannot carry them A help, prompt, type, def_bool, transitional, option or visible if written under a menu or a comment escaped as an AttributeError traceback instead of a parse error. Menu and comment nodes carry a plain constant as their item rather than a symbol or a choice, and they leave the help and visibility slots unset, so the handlers reached for attributes that were not there. A typo in a Kconfig file produced a Python stack trace with no file or line in it. Alongside those, the modules property raised TypeError on any symbol that is not MODULES, because its warning passed a filename and a line number to a helper that takes a single location. Which node kinds may carry which property is a table now, checked once at the top of the property loop, rather than nine hand-written kind tests scattered through the dispatch. The table is complete because it is a table: the chain only ever covered the cases somebody remembered. It also reuses the symbol-or-choice frozenset that already existed. Visibility stays a separate check, since menu and comment items are both plain constants and telling them apart is an identity test, not a class one. Nothing that parsed before parses differently. Every input newly rejected raised AttributeError or TypeError before, which is not acceptance. tests/test_fuzz.py found all of them. Generating token soup was tried first and is useless here, because every case dies on line one and the block and property parsers are never reached. So it builds a structurally valid Kconfig tree and then corrupts it, which puts about half of each run past the end of the parser. The contract it asserts is the only one worth asserting on random input: the parser either accepts the file or raises its own error, never anything else. A blown Python stack is not acceptance either, so it is only forgiven for the one shape known to cause it, where a config listed by two separate choice blocks sends visibility evaluation around a cycle. That defect reproduces on the base branch and is left for its own change. Matching the shape rather than a seed number means changing the generator cannot quietly widen the exemption. --- kconfiglib.py | 67 ++++++- tests/test_fuzz.py | 487 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 543 insertions(+), 11 deletions(-) create mode 100644 tests/test_fuzz.py diff --git a/kconfiglib.py b/kconfiglib.py index f49defc..c6229ea 100644 --- a/kconfiglib.py +++ b/kconfiglib.py @@ -3451,6 +3451,14 @@ def _parse_props(self, node): while self._next_line(): t0 = self._tokens[0] + owner = _PROP_OWNER.get(t0) + if owner is not None and node.item.__class__ not in owner: + # _parse_error() quotes the offending line, so naming the kind + # that may carry it is the whole message. + self._parse_error( + "only " + _PROP_OWNER_STR[owner] + " can have this property" + ) + if t0 in _TYPE_TOKENS: # Relies on '_T_BOOL is BOOL', etc., to save a conversion self._set_type(node.item, t0) @@ -3467,9 +3475,6 @@ def _parse_props(self, node): self._parse_help(node) elif t0 is _T_SELECT: - if node.item.__class__ is not Symbol: - self._parse_error("only symbols can select") - node.selects.append( Select(self._expect_nonconst_sym(), self._parse_cond(), self.loc) ) @@ -3503,14 +3508,17 @@ def _parse_props(self, node): ) elif t0 is _T_IMPLY: - if node.item.__class__ is not Symbol: - self._parse_error("only symbols can imply") - node.implies.append( Imply(self._expect_nonconst_sym(), self._parse_cond(), self.loc) ) elif t0 is _T_VISIBLE: + # Not table-driven: menu and comment items are both plain + # constants, so this is an identity test, not a class one. + # 'visibility' is only initialized on menu nodes. + if node.item is not MENU: + self._parse_error("only menus can have 'visible if'") + if not self._check_token(_T_IF): self._parse_error("expected 'if' after 'visible'") @@ -3606,14 +3614,10 @@ def _parse_props(self, node): "assumes the symbol name MODULES, like older " "versions of the C implementation did when " "'modules' wasn't used.", - self.filename, - self.linenr, + self.loc, ) elif t0 is _T_OPTIONAL: - if node.item.__class__ is not Choice: - self._parse_error('"optional" is only valid for choices') - node.item.is_optional = True elif t0 is _T_TRANSITIONAL: @@ -8178,6 +8182,47 @@ def _probe_fingerprint(encoding): } ) +_SYMBOL_ONLY = frozenset({Symbol}) +_CHOICE_ONLY = frozenset({Choice}) + +# Which node kinds may carry which property. +# +# 'menu' and 'comment' nodes hold a plain constant as their item rather than a +# Symbol or a Choice, and they leave the 'help' and 'visibility' slots unset. +# A handler that reaches for item.orig_type, item.name_and_loc, item.env_var +# or node.help on one of those finds nothing, and the user gets an +# AttributeError traceback instead of a line number. Checking the pairing once, +# from a table, is what keeps the set complete: guarding handler by handler +# only ever covers the cases somebody thought to guard. +# +# A token absent from the table is allowed on any node kind, which is how +# 'depends on', 'default' and 'range' behave today. +_PROP_OWNER = dict.fromkeys(_TYPE_TOKENS, _SYMBOL_CHOICE) +_PROP_OWNER.update(dict.fromkeys(_DEF_TOKEN_TO_TYPE, _SYMBOL_CHOICE)) +_PROP_OWNER.update( + { + _T_HELP: _SYMBOL_CHOICE, + _T_PROMPT: _SYMBOL_CHOICE, + _T_SELECT: _SYMBOL_ONLY, + _T_IMPLY: _SYMBOL_ONLY, + _T_MODULES: _SYMBOL_ONLY, + _T_OPTION: _SYMBOL_ONLY, + _T_TRANSITIONAL: _SYMBOL_ONLY, + _T_OPTIONAL: _CHOICE_ONLY, + } +) + +# 'visible if' is not in the table because it is the one property keyed on the +# item's identity rather than its class: menu and comment nodes are both plain +# constants, so a class test cannot tell them apart. _parse_props() checks it +# against MENU directly. + +_PROP_OWNER_STR = { + _SYMBOL_CHOICE: "symbols and choices", + _SYMBOL_ONLY: "symbols", + _CHOICE_ONLY: "choices", +} + _MENU_COMMENT = frozenset( { MENU, diff --git a/tests/test_fuzz.py b/tests/test_fuzz.py new file mode 100644 index 0000000..f9ea89f --- /dev/null +++ b/tests/test_fuzz.py @@ -0,0 +1,487 @@ +# Copyright (c) 2011-2019 Ulf Magnusson +# SPDX-License-Identifier: ISC +# +# Randomized parser tests. +# +# The parse path holds nearly all of the library's branching: _tokenize(), +# _parse_block() and _parse_props() each run to roughly complexity 32-34, and +# everything downstream trusts whatever they produce. The hand-written tests +# cover the Kconfig files a person would write; these cover the ones nobody +# would, which is where a parser actually breaks. +# +# The contract is narrow, and it is the only one worth asserting on random +# input: for any input at all, Kconfig() either parses it or raises +# KconfigError. It must never escape with an AttributeError from an item that +# was assumed to be a Symbol, or from a MenuNode slot that was never +# initialized. That is the shape a parser crash actually takes here, and it is +# what five of these found: 'help', 'prompt', a type, a 'def_bool' or a +# 'visible if' on a 'menu' or 'comment' node each produced a traceback rather +# than a file:line error. See test_regressions below. +# +# Generating pure token soup does not work: every case dies on line one and +# the block and property parsers are never reached. So the generator builds a +# structurally valid Kconfig tree first and then corrupts it. The mix that +# comes out, measured over 2400 cases: +# +# ~30% parse cleanly +# ~47% fail to parse, at whatever point the corruption reached +# ~23% parse completely and are then rejected for a dependency loop, +# because the conditions draw from a small shared pool of symbol +# names. These still exercise the whole parser; the loop is found +# afterwards +# +# so a little over half of every run reaches the end of the parser. +# +# Deterministic on purpose: the seeds are fixed, so a failure reproduces on +# every machine and in CI rather than showing up once and vanishing. To widen +# the search locally, raise SEEDS or CASES_PER_SEED. +# +# What is deliberately never generated: $(shell,...) and $(python,...), which +# run arbitrary commands by design, and 'source' lines. A fuzzer emitting +# either would be running random code on the machine rather than testing the +# parser. + +import os +import random + +import pytest + +from kconfiglib import Kconfig, KconfigError + +# Exceptions that mean "the input was rejected", which is a correct outcome. +# +# RecursionError is not one of them. A blown Python stack is the interpreter +# falling over, not the parser saying no, and that is the one distinction a +# robustness sweep exists to make: tolerating it here would let any new +# unbounded recursion land in the same bucket and keep the sweep green. +# +# One generated file in 960 does hit it today, through a defect that predates +# these tests and reproduces on the base branch: a config listed by two +# separate choice blocks sends visibility evaluation around a cycle. That one +# shape is what _check() forgives, and nothing else, so the tolerance is +# exactly as wide as the known bug. test_the_known_recursion_shape_still_bites +# below fails once somebody fixes the cycle, so the exemption cannot quietly +# outlive the defect. +EXPECTED = (KconfigError,) + + +def has_known_recursion_shape(text): + """True if 'text' contains the one shape known to blow the stack. + + A config listed by two separate choice blocks makes visibility evaluation + cycle. Detected by shape rather than by seed number, so that changing the + generator cannot silently turn the exemption into a blanket one, and so + that the day the parser is fixed this predicate can just be deleted. + """ + seen = set() + current = set() + in_choice = False + repeated = False + for line in text.splitlines(): + stripped = line.strip() + if stripped.startswith("choice"): + in_choice, current = True, set() + elif stripped.startswith("endchoice"): + # A block's own names join 'seen' only once it closes. Folding + # them in per line made a name repeated inside a single block + # look like a name shared between two, which is the shape that + # actually recurses. + # + # 'current' starts empty rather than being bound in the branch + # above, because the generator emits a stray endchoice with no + # choice open and this predicate runs from an exception handler, + # where a NameError would be reported in place of the crash it + # was called to classify. + seen |= current + current = set() + in_choice = False + elif in_choice and stripped.startswith("config "): + name = stripped.split(None, 1)[1].strip() + if name in seen: + repeated = True + current.add(name) + return repeated + + +SEEDS = range(120) +CASES_PER_SEED = 8 + + +TYPES = ("bool", "tristate", "string", "hex", "int") +NAMES = [f"SYM_{i}" for i in range(12)] + +# 'select' and 'imply' targets come from their own pool of symbols that are +# defined with no dependencies of their own (see valid_source). Drawing them +# from NAMES instead put a dependency loop in a quarter of the generated +# files, and Kconfig reports those only after parsing the whole thing, so they +# added nothing but noise to the mix the mutations are supposed to produce. +LEAVES = [f"LEAF_{i}" for i in range(6)] + + +def _cond(rng): + a, b = rng.choice(NAMES), rng.choice(NAMES) + return rng.choice( + ( + a, + f"{a} && {b}", + f"{a} || {b}", + f"!{a}", + f"{a} = {b}", + f"{a} != y", + f"({a} && !{b})", + ) + ) + + +# A valid value for each type, for the 'default' properties +_VALUES = {"bool": "y", "tristate": "m", "string": '"s"', "hex": "0x10", "int": "7"} +_RANGES = {"hex": "0x0 0xff", "int": "0 99"} + +_BOOLISH = ("bool", "tristate") +_NUMERIC = ("hex", "int") + +# Property generators, each paired with the types it is valid for. A table +# rather than a chain of cumulative probability thresholds: the thresholds hid +# which properties a given type could actually get, because a type that failed +# one branch's condition fell through to the next threshold rather than being +# excluded. Here a property is drawn only from the ones that apply, so the +# generated file is always type-correct and the distribution is even. +_PROPS = ( + (TYPES, lambda rng, name, typ: [f"\tdefault {_VALUES[typ]}"]), + (TYPES, lambda rng, name, typ: [f"\tdefault {_VALUES[typ]} if {_cond(rng)}"]), + (TYPES, lambda rng, name, typ: [f"\tdepends on {_cond(rng)}"]), + (TYPES, lambda rng, name, typ: [f'\tprompt "alt" if {_cond(rng)}']), + ( + TYPES, + lambda rng, name, typ: [ + "\thelp", + f"\t Some help text for {name}.", + "\t Second line.", + ], + ), + (_BOOLISH, lambda rng, name, typ: [f"\tselect {rng.choice(LEAVES)}"]), + ( + _BOOLISH, + lambda rng, name, typ: [f"\timply {rng.choice(LEAVES)} if {_cond(rng)}"], + ), + (_NUMERIC, lambda rng, name, typ: [f"\trange {_RANGES[typ]}"]), +) + + +def _props(rng, name, typ): + out = [f'\t{typ} "{name} prompt"'] + applicable = [build for types, build in _PROPS if typ in types] + for _ in range(rng.randint(0, 4)): + out += rng.choice(applicable)(rng, name, typ) + return out + + +def _block(rng, depth=0): + out = [] + for _ in range(rng.randint(1, 4)): + r = rng.random() + if r < 0.45 or depth > 2: + name = rng.choice(NAMES) + typ = rng.choice(TYPES) + out.append(f"config {name}") + out += _props(rng, name, typ) + elif r < 0.6: + out.append(f'menu "A menu {rng.randint(0, 9)}"') + if rng.random() < 0.3: + out.append(f"\tvisible if {_cond(rng)}") + out += _block(rng, depth + 1) + out.append("endmenu") + elif r < 0.75: + out.append(f"if {_cond(rng)}") + out += _block(rng, depth + 1) + out.append("endif") + elif r < 0.9: + out.append("choice") + out.append('\tprompt "A choice"') + if rng.random() < 0.4: + out.append("\toptional") + for _ in range(rng.randint(1, 3)): + name = rng.choice(NAMES) + out.append(f"config {name}") + out.append(f'\tbool "{name}"') + out.append("endchoice") + else: + out.append('comment "A comment"') + if rng.random() < 0.3: + out.append(f"\tdepends on {_cond(rng)}") + return out + + +def _leaf_defs(): + # Selectable symbols with no dependencies, so that a 'select' can never + # close a dependency cycle + out = [] + for leaf in LEAVES: + out.append(f"config {leaf}") + out.append(f'\tbool "{leaf}"') + return out + + +def valid_source(rng): + return "\n".join(_block(rng) + _leaf_defs()) + "\n" + + +MUTATIONS = ( + "drop_line", + "truncate_line", + "drop_token", + "swap_keyword", + "insert_garbage", + "dedent", + "indent", + "drop_char", + "dup_line", +) + +GARBAGE = ( + '"', + "(", + ")", + "&&", + "$", + "$(", + "!", + ",", + "=", + "\\", + "endmenu", + "endif", + "endchoice", + "help", + "config", + "\x00", +) + + +def mutate(text, rng, n): + lines = text.split("\n") + for _ in range(n): + if not lines: + break + i = rng.randrange(len(lines)) + how = rng.choice(MUTATIONS) + ln = lines[i] + if how == "drop_line": + del lines[i] + elif how == "truncate_line" and ln: + lines[i] = ln[: rng.randrange(len(ln))] + elif how == "drop_token": + toks = ln.split() + if toks: + del toks[rng.randrange(len(toks))] + lines[i] = ( + "\t" + " ".join(toks) if ln.startswith("\t") else " ".join(toks) + ) + elif how == "swap_keyword": + toks = ln.split() + if toks: + toks[rng.randrange(len(toks))] = rng.choice(GARBAGE) + lines[i] = " ".join(toks) + elif how == "insert_garbage": + lines.insert(i, rng.choice(GARBAGE)) + elif how == "dedent": + lines[i] = ln.lstrip() + elif how == "indent": + lines[i] = "\t" + ln + elif how == "drop_char" and ln: + j = rng.randrange(len(ln)) + lines[i] = ln[:j] + ln[j + 1 :] + elif how == "dup_line": + lines.insert(i, ln) + return "\n".join(lines) + + +def _check(path): + """Parses 'path' and walks the result, returning True if it parsed. + + Raises AssertionError if anything but a clean parse or a clean rejection + comes out. A RecursionError counts as a crash unless the file carries the + one shape known to cause it, which keeps the exemption exactly as wide as + the known defect while still feeding every generated file to the parser. + """ + + def crashed(e, where): + if isinstance(e, RecursionError) and has_known_recursion_shape( + path.read_text(encoding="utf-8") + ): + return False + raise AssertionError(f"{type(e).__name__} escaped {where}") from e + + try: + kconf = Kconfig(str(path), warn=False) + except EXPECTED: + return False + except Exception as e: # noqa: BLE001 -- turning a crash into a report + return crashed(e, "the parser") + + # Walk the result too: a parser can accept garbage and leave behind a tree + # that only blows up when something reads it. + try: + for sym in kconf.unique_defined_syms: + sym.str_value # noqa: B018 -- evaluating it is the point + str(sym) + for node in kconf.node_iter(): + str(node) + kconf.write_config(os.devnull) + except EXPECTED: + return False + except Exception as e: # noqa: BLE001 -- turning a crash into a report + return crashed(e, "while walking the parsed tree") + return True + + +@pytest.mark.parametrize("seed", SEEDS) +def test_corrupted_kconfig_is_parsed_or_rejected(seed, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + rng = random.Random(seed) + path = tmp_path / "Kfuzz" + for _ in range(CASES_PER_SEED): + text = mutate(valid_source(rng), rng, rng.randint(0, 3)) + path.write_text(text, encoding="utf-8") + try: + _check(path) + except AssertionError as e: + raise AssertionError(f"{e}\n--- input ---\n{text}--- end ---") from e + + +def test_the_generator_still_reaches_the_parser(tmp_path, monkeypatch): + """Guards the fuzzer itself. + + A generator that only ever produces line-one syntax errors passes every + assertion above while testing almost nothing. If this ratio collapses, + the mutation rate or the grammar has drifted and the sweep above has + quietly stopped being a test. + + Counts clean parses only, so it reads low (~30%): the files rejected for + a dependency loop got all the way through the parser but are not counted + here. The bounds are wide because this is a smoke alarm, not a target. + """ + monkeypatch.chdir(tmp_path) + rng = random.Random(0) + path = tmp_path / "Kratio" + parsed = 0 + total = 200 + for _ in range(total): + text = mutate(valid_source(rng), rng, rng.randint(0, 3)) + path.write_text(text, encoding="utf-8") + parsed += _check(path) + assert 0.1 * total < parsed < 0.9 * total, ( + f"{parsed}/{total} of the generated files parsed; the generator is " + f"no longer producing a mix of valid and invalid input" + ) + + +# Every one of these crashed with an AttributeError before the guards in +# _parse_props(). They are pinned here so the fixes cannot regress without the +# fuzzer happening to rediscover them. +@pytest.mark.parametrize( + "text", + ( + 'comment "c"\nhelp\n', + 'menu "m"\nhelp\nendmenu\n', + 'comment "c"\nbool\n', + 'menu "m"\ntristate\nendmenu\n', + 'comment "c"\ndef_bool y\n', + 'menu "m"\nprompt "p"\nendmenu\n', + 'comment "c"\nprompt "p"\n', + 'config A\n\tbool "a"\n\tvisible if B\n', + 'comment "c"\nvisible if B\n', + ), +) +def test_regressions(text, tmp_path, monkeypatch): + """A property on a node that cannot carry it is a KconfigError, not a + traceback. The user typed something wrong; they get the line number.""" + monkeypatch.chdir(tmp_path) + path = tmp_path / "Kbad" + path.write_text(text, encoding="utf-8") + with pytest.raises(KconfigError): + Kconfig(str(path), warn=False) + + +@pytest.mark.parametrize( + "text", + ( + "config\n", + "config A\n\tbool\n\tdefault\n", + "menu\n", + 'menu "m"\n', + "if\n", + "choice\n", + "endmenu\n", + "endif\n", + "endchoice\n", + "source\n", + "config A\n\tint\n\trange 1\n", + "config A\n\tdepends on\n", + "config A\n\tselect\n", + 'config A\n\tbool "unterminated\n', + "config A\n\tdefault y if\n", + "config A\n\tdefault y if (\n", + "mainmenu\n", + "\tbool\n", + "config " + "A" * 10000 + "\n", + "\x00\n", + 'config A\n\tbool "a"\n\tdepends on ' + "!" * 500 + "A\n", + ), +) +def test_malformed_input_never_crashes(text, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + path = tmp_path / "Kbad" + path.write_text(text, encoding="utf-8") + _check(path) + + +def test_the_known_recursion_shape_still_bites(tmp_path, monkeypatch): + """The sentinel for the exemption _check() grants. + + A config listed by two separate choice blocks sends visibility evaluation + around a cycle until the stack runs out. That is a real defect, it + reproduces on the base branch, and it is left for its own change. This + test fails the day it is fixed, which is the signal to delete + has_known_recursion_shape() and the branch in _check() that uses it, + rather than leaving dead tolerance behind for a bug that no longer + exists. + """ + monkeypatch.chdir(tmp_path) + text = ( + 'menu "m"\nconfig A\nconfig B\n\tbool "b"\n\tprompt "alt" if C\n' + "endmenu\n" + "choice\nconfig A\nconfig D\nendchoice\n" + 'choice\nconfig D\nconfig C\n\tbool "c"\nendchoice\n' + 'config D\n\thex "d"\n\tdepends on B || E\n' + ) + assert has_known_recursion_shape(text) + + path = tmp_path / "Kcycle" + path.write_text(text, encoding="utf-8") + with pytest.raises(RecursionError): + kconf = Kconfig(str(path), warn=False) + for sym in kconf.unique_defined_syms: + sym.str_value # noqa: B018 -- evaluating it is the point + + +@pytest.mark.parametrize("depth", (50, 300)) +def test_deep_nesting_parses(depth, tmp_path, monkeypatch): + """Recursion depth is the classic parser cliff, so assert the strong + thing: nesting this deep parses, rather than merely failing tidily. + + The parser descends once per enclosing 'if', so it does run out of Python + stack eventually; 1000 nested conditions reach CPython's default limit. + Nothing near that is a Kconfig anybody writes, and making the parser + iterative is a different change from this one, so the cliff is recorded + here rather than fixed. What matters is that ordinary depths stay well + clear of it, and that a RecursionError is never mistaken for the parser + rejecting a file, which _check() is careful about. + """ + monkeypatch.chdir(tmp_path) + text = "".join(f"if Y{i}\n" for i in range(depth)) + text += 'config DEEP\n\tbool "deep"\n' + text += "endif\n" * depth + path = tmp_path / "Kdeep" + path.write_text(text, encoding="utf-8") + assert _check(path), f"nesting {depth} deep should parse" From f0fcf8c16f8e723fdb330a8455f1cc142e86d0b6 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Mon, 21 Sep 2026 01:13:18 +0800 Subject: [PATCH 5/5] Run the benchmark and raise the coverage floor scripts/benchmark.py has been sitting in the tree unused. The collector and probe cache work that went in recently came with specific numbers attached, twenty small parses dropping from 0.81s to 0.01s among them, and nothing was watching whether they held. The selftest job now runs it and keeps the JSON as an artifact, so a later regression in the parse or redraw paths can be attributed instead of merely noticed. Once per job, not twice: asking for JSON returns before the table is printed, so a second run would re-measure everything and the table in the log would not be the numbers in the artifact. Still no pass/fail threshold, and the numbers want reading with care. Phases on this fixture move by around 7 percent run to run on an idle machine and around 30 on a busy one, and the spread is cross-process fixed cost rather than the timing loop, so a shared runner is the busy case. That attributes a large regression and nothing subtler. Pointing it at a tree big enough for the phases to run in milliseconds would tighten it. The coverage floor goes from 35 to 38 against a total that is now 41, and uicommon.py joins the measured set. The comment explaining how to reproduce the runner figure locally stays, because it is easy to take a number from a plain local run and set a floor that fails every job here. --- .github/workflows/test.yml | 54 +++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index eb028fa..93795b7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -87,15 +87,17 @@ jobs: # backstop against a large drop, not a ratchet; raise it as # coverage improves. # - # 35 is set against what these runners report, which is 40. Do not + # 38 is set against what these runners report, which is 41. Do not # calibrate it from a local run on Python 3.14 or newer: coverage # switches to the sys.monitoring backend there and scores the same - # suite at 51, and a floor taken from that number fails every job - # here. COVERAGE_CORE=ctrace reproduces the runner figure locally. + # suite differently, and a floor taken from that number fails every + # job here. COVERAGE_CORE=ctrace reproduces the runner figure + # locally. python -m pytest tests/ --ignore=tests/test_conformance.py \ --cov=kconfiglib --cov=menuconfig --cov=guiconfig --cov=rawterm \ + --cov=uicommon \ --cov-report=term --cov-report=xml:coverage.xml \ - --cov-fail-under=35 + --cov-fail-under=38 fi - name: Store coverage report @@ -126,6 +128,50 @@ jobs: python .ci/validate-rawterm.py if errorlevel 1 exit /b %errorlevel% + - name: Benchmark the load and UI hot paths + # No pass/fail threshold, and read the numbers with care. Measured on + # this fixture, phase timings move by around 7% run to run on an idle + # machine and around 30% on a busy one; the spread is cross-process + # fixed cost, not the timing loop, so raising -n does not close it. A + # shared runner is the busy case. So this attributes a large regression + # and nothing subtler, and the value is the trend across runs rather + # than any single number. Pointing it at a tree big enough for the + # phases to run in milliseconds would tighten it. + if: ${{ matrix.target.headless-only != true }} + shell: bash + run: | + set -euo pipefail + # Measured once: --json returns before printing the table, so a + # second run would re-measure everything and the table shown here + # would not be the numbers in the artifact. Render the table from + # the captured JSON instead, so the log and the artifact agree. + python scripts/benchmark.py --tree tests --kconfig Kuirender \ + --json > benchmark.json + python - <<'PY' + import json + + with open("benchmark.json") as f: + phases = json.load(f) + + print(f"{'phase':<28} {'best':>10} detail") + print("-" * 72) + for name, data in sorted(phases.items()): + seconds = data.get("seconds") + best = "-" if seconds is None else f"{seconds * 1000:.2f}ms" + detail = ", ".join( + f"{k}={v}" for k, v in sorted(data.items()) if k != "seconds" + ) + print(f"{name:<28} {best:>10} {detail}") + PY + + - name: Store benchmark timings + if: ${{ !cancelled() && matrix.target.headless-only != true }} + uses: actions/upload-artifact@v6 + with: + name: benchmark-${{ matrix.target.os }}-py${{ matrix.target.python }} + path: benchmark.json + if-no-files-found: ignore + - name: Diagnostic dump on failure if: failure() shell: bash