feat: make test-ui checks the UI files with blueprint-compiler
The interface is hand-written GtkBuilder XML and nothing in the build can see a mistake in it: a
property a class does not have, or a child a container does not accept, is a Gtk-ERROR when the
window is built -- and Gtk-ERROR aborts the process, so a typo in a .ui takes the application down
rather than failing to open a dialog. xml-stripblanks catches malformed XML and nothing else, and
the probes catch the rest only for the windows they open. Three of those aborts happened during
this refactor.
tools/check_ui.py round-trips each file through blueprint-compiler: decompile to blueprint, then
compile that, which type-checks every property and child type against the GIR. The XML stays the
source of truth and stays hand-written -- this is a check, not a compiler, and nothing here is
needed to build the application. Which is why the target is opt-in (no ALL) and the tool is looked
for at configure time rather than required: without it the target is absent, not silently passing.
$ make -C build test-ui
check_ui: 5 files in src/ui
ok help.ui ... source-form.ui
check_ui: 5 ok, 0 failed
Injected the exact mistake that cost a run earlier (an AdwEntryRow subtitle) and the check caught
it, with the caret and the "did you mean title?" hint, exiting non-zero. --lint prints the linter's
advice separately: it is advice, and the linter has opinions this project does not share.
This commit is contained in:
1 parent
8073e7b3fd
commit
7a982da6c4
4 files changed
+190
-17
No files matched your search
@@ -102,6 +102,31 @@ add_custom_command(
|
|||||||
set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/udpscope_ui_resources.c
|
set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/udpscope_ui_resources.c
|
||||||
PROPERTIES COMPILE_OPTIONS "-Wno-overlength-strings")
|
PROPERTIES COMPILE_OPTIONS "-Wno-overlength-strings")
|
||||||
|
|
||||||
|
# --- the UI files: an opt-in check, never part of `cmake --build` ---------------
|
||||||
|
# The interface is hand-written GtkBuilder XML, and nothing in the build sees a mistake in it: a
|
||||||
|
# property a class does not have is a `Gtk-ERROR` when the window is built, and `Gtk-ERROR` *aborts
|
||||||
|
# the process* -- so a typo takes the application down rather than failing to open a dialog.
|
||||||
|
# `xml-stripblanks` catches malformed XML and nothing else, and the probes catch the rest only for the
|
||||||
|
# windows they open.
|
||||||
|
#
|
||||||
|
# blueprint-compiler type-checks every property and child type against the GIR, and it reads XML as
|
||||||
|
# well as blueprint (`decompile` turns one into the other), so `tools/check_ui.py` round-trips each
|
||||||
|
# file through it. The XML stays the source of truth and stays hand-written; nothing here is needed to
|
||||||
|
# build the application, which is the whole point of not converting the files.
|
||||||
|
#
|
||||||
|
# No ALL: this runs only when it is named, so `cmake --build build` is unaffected and
|
||||||
|
# blueprint-compiler is not a build dependency.
|
||||||
|
find_program(UDPSCOPE_PYTHON NAMES python3)
|
||||||
|
if(UDPSCOPE_PYTHON)
|
||||||
|
add_custom_target(test-ui
|
||||||
|
COMMAND ${UDPSCOPE_PYTHON} ${CMAKE_CURRENT_SOURCE_DIR}/tools/check_ui.py ${UDPSCOPE_UI_DIR}
|
||||||
|
DEPENDS ${UDPSCOPE_UI_FILES} ${CMAKE_CURRENT_SOURCE_DIR}/tools/check_ui.py
|
||||||
|
COMMENT "Checking the UI files with blueprint-compiler"
|
||||||
|
VERBATIM)
|
||||||
|
else()
|
||||||
|
message(WARNING "python3 was not found: the test-ui target is not available")
|
||||||
|
endif()
|
||||||
|
|
||||||
# --- build info: version, commit and timestamp compiled into the binary -------
|
# --- build info: version, commit and timestamp compiled into the binary -------
|
||||||
# Written at configure time so add_executable can name the file, and again on every
|
# Written at configure time so add_executable can name the file, and again on every
|
||||||
# build so the commit and the timestamp belong to the binary that is running rather
|
# build so the commit and the timestamp belong to the binary that is running rather
|
||||||
|
|||||||
@@ -34,6 +34,24 @@ deliberate: the entry then always launches exactly the binary that was last buil
|
|||||||
`StartupWMClass` is needed — GTK takes the WM class from the application ID, and the desktop
|
`StartupWMClass` is needed — GTK takes the WM class from the application ID, and the desktop
|
||||||
file's basename is that same string, which is what associates a running window with the entry.
|
file's basename is that same string, which is what associates a running window with the entry.
|
||||||
|
|
||||||
|
The interface files can be checked without building anything or opening a window:
|
||||||
|
|
||||||
|
```sh
|
||||||
|
cmake --build build --target test-ui # or: make -C build test-ui
|
||||||
|
```
|
||||||
|
|
||||||
|
That round-trips every `.ui` through `blueprint-compiler` — `decompile` turns it into blueprint, and
|
||||||
|
compiling *that* type-checks every property and child type against the GIR — printing one line per file.
|
||||||
|
It exists because the alternative is what happens otherwise: a property a class does not have is a
|
||||||
|
`Gtk-ERROR` when the window is built, and `Gtk-ERROR` **aborts the process**, so a typo in a `.ui` takes
|
||||||
|
the application down rather than failing to open a dialog. `preprocess="xml-stripblanks"` catches
|
||||||
|
malformed XML and nothing else, and the probes catch the rest only for the windows they open.
|
||||||
|
|
||||||
|
The XML stays the source of truth and stays hand-written: this is a check, not a compiler, and nothing
|
||||||
|
here is needed to *build* the application. `blueprint-compiler` and `python3` are found at configure
|
||||||
|
time; without them the target is absent rather than silently passing. `tools/check_ui.py --lint` also
|
||||||
|
prints the linter's advice, which is advice and deliberately not a gate.
|
||||||
|
|
||||||
Options: `--server`, `--port`, `--multicast`, `--interface`, `--buffer-mb` (per
|
Options: `--server`, `--port`, `--multicast`, `--interface`, `--buffer-mb` (per
|
||||||
buffer, default 200), `--max-window`, `--holdoff`, `--recv-buffer`, `--plots` (the row
|
buffer, default 200), `--max-window`, `--holdoff`, `--recv-buffer`, `--plots` (the row
|
||||||
count to *start* with, 1..`RowSet::kMaxRows`; the layout editor owns it afterwards),
|
count to *start* with, 1..`RowSet::kMaxRows`; the layout editor owns it afterwards),
|
||||||
|
|||||||
@@ -210,23 +210,30 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear
|
|||||||
- **`find_accel()` masks the modifiers before comparing.** Lock, Num-Lock and the pointer buttons
|
- **`find_accel()` masks the modifiers before comparing.** Lock, Num-Lock and the pointer buttons
|
||||||
all arrive in `GdkModifierType`, so an unmasked compare matches no table row at all once
|
all arrive in `GdkModifierType`, so an unmasked compare matches no table row at all once
|
||||||
Caps Lock is on.
|
Caps Lock is on.
|
||||||
- **A UI file is data, and a mistake in it is a runtime abort, not a build failure.** `.ui` files live in
|
- **A UI file is data, and a mistake in it is a runtime abort, not a build failure.** `.ui` files live
|
||||||
`src/ui/` and are compiled into the binary by the third GResource (`udpscope-ui.gresource.xml`,
|
in `src/ui/` and are compiled into the binary by the third GResource (`udpscope-ui.gresource.xml`,
|
||||||
`prefix=/org/udpscope/ui`), loaded with `gtk_builder_new_from_resource()`. `preprocess="xml-stripblanks"`
|
`prefix=/org/udpscope/ui`), loaded with `gtk_builder_new_from_resource()`.
|
||||||
catches a *malformed* file at build time — and that includes an XML rule easy to break by hand: **a
|
|
||||||
comment may not contain a double hyphen**, so prose written the way every C++ comment in this project
|
`preprocess="xml-stripblanks"` catches a *malformed* file at build time -- which is why it is on
|
||||||
is written (` -- like this -- `) fails the build with a caret under the offending character. Use an em
|
every file in every bundle -- and that includes an XML rule easy to break by hand: **a comment may
|
||||||
dash in a UI file. `preprocess="xml-stripblanks"`
|
not contain a double hyphen**, so prose written the way every C++ comment in this project is written
|
||||||
catches a *malformed* file at build time -- and nothing else does, which is why it is on every file in
|
(` -- like this -- `) fails the build with a caret under the offending character. Use an em dash in a
|
||||||
every bundle. What it cannot catch is a file that is valid XML but wrong for GTK: `GtkBuilder` reports
|
UI file.
|
||||||
that with a **`Gtk-ERROR`, which aborts the process** by default, so a window built by hand from bad
|
|
||||||
markup takes the application down rather than failing to appear. The `[help]` probe opening every
|
What it cannot catch is a file that is valid XML but wrong for GTK, and that is what
|
||||||
window is what catches that. Two specifics already paid for: a `GIcon` property takes an **object**, not
|
`cmake --build build --target test-ui` is for: it round-trips every file through
|
||||||
a name (`<property name="icon">foo</property>` is read as a reference to another object and fails with
|
`blueprint-compiler`, which type-checks properties and child types against the GIR -- for *every*
|
||||||
"Object with ID foo not found" -- the icon is a nested `GThemedIcon`), and a `.ui` big enough to exceed
|
file, rather than for the windows a probe happens to open (see
|
||||||
4095 characters needs `-Wno-overlength-strings` on its generated source, because glib-compile-resources
|
[build-and-run.md](build-and-run.md)). Without that check, `GtkBuilder` reports such a file with a
|
||||||
writes each file as one C string literal and the warning fires on generated *data* (the icon and help
|
**`Gtk-ERROR`, which aborts the process** by default, so a window built from bad markup takes the
|
||||||
bundles are small enough per file never to need it).
|
application down rather than failing to appear.
|
||||||
|
|
||||||
|
Two more paid for: a `GIcon` property takes an **object**, not a name
|
||||||
|
(`<property name="icon">foo</property>` is read as a reference to another object and fails with
|
||||||
|
"Object with ID foo not found" -- the icon is a nested `GThemedIcon`); and a `.ui` big enough to
|
||||||
|
exceed 4095 characters needs `-Wno-overlength-strings` on its generated source, because
|
||||||
|
glib-compile-resources writes each file as one C string literal and the warning fires on generated
|
||||||
|
*data*.
|
||||||
- **Deprecated widgets may be the right widget, as long as no deprecated *function* is called.** The
|
- **Deprecated widgets may be the right widget, as long as no deprecated *function* is called.** The
|
||||||
shortcut overview is a `GtkShortcutsWindow` (`gtk/deprecated/`, deprecated in 4.20) because libadwaita's
|
shortcut overview is a `GtkShortcutsWindow` (`gtk/deprecated/`, deprecated in 4.20) because libadwaita's
|
||||||
successor, `AdwShortcutsDialog`, cannot draw a mouse gesture: `AdwShortcutsItem` has no icon property at
|
successor, `AdwShortcutsDialog`, cannot draw a mouse gesture: `AdwShortcutsItem` has no icon property at
|
||||||
|
|||||||
Executable
+123
@@ -0,0 +1,123 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
# SPDX-License-Identifier: MIT
|
||||||
|
# SPDX-FileCopyrightText: 2026 Martino Ferrari
|
||||||
|
|
||||||
|
"""Checks the interface files: `make test-ui`.
|
||||||
|
|
||||||
|
The interface is hand-written GtkBuilder XML, and nothing in the build can see a mistake in it. A
|
||||||
|
property a class does not have, or a child a container does not accept, is a `Gtk-ERROR` when the
|
||||||
|
window is built — and `Gtk-ERROR` aborts the process, so a typo in a `.ui` takes the application
|
||||||
|
down rather than failing to open a dialog. `preprocess="xml-stripblanks"` catches malformed XML and
|
||||||
|
nothing else, and the probes catch the rest only for the windows they open.
|
||||||
|
|
||||||
|
blueprint-compiler can see it: it type-checks every property and child type against the GIR. It reads
|
||||||
|
blueprint rather than XML — but it reads XML too, because `decompile` turns a `.ui` back into
|
||||||
|
blueprint. So the check is a round trip, and the XML stays the source of truth:
|
||||||
|
|
||||||
|
$ make test-ui
|
||||||
|
check_ui: 5 files in src/ui
|
||||||
|
ok help.ui
|
||||||
|
ok main-window.ui
|
||||||
|
ok preferences.ui
|
||||||
|
ok shortcuts.ui
|
||||||
|
ok source-form.ui
|
||||||
|
check_ui: 5 ok, 0 failed
|
||||||
|
check_ui: 23 pieces of advice — not failures; --lint shows them
|
||||||
|
|
||||||
|
A file that fails prints what the compiler said, with the line, the caret and its suggestion:
|
||||||
|
|
||||||
|
FAIL preferences.ui
|
||||||
|
error: Class Adw.EntryRow does not have a property called subtitle
|
||||||
|
at preferences.blp line 24 column 9:
|
||||||
|
24 | subtitle: "empty = unicast";
|
||||||
|
| ^^^^^^^^
|
||||||
|
hint: Did you mean `title`?
|
||||||
|
|
||||||
|
`lint` runs too and its findings are printed as advice rather than failures: a linter has opinions
|
||||||
|
this project does not have to share (it wants "udpscope" marked translatable, which it is not), and a
|
||||||
|
check that fails on advice is a check people stop running.
|
||||||
|
|
||||||
|
A missing `blueprint-compiler` is a *failure* and not a skip. This target exists to be run, and
|
||||||
|
answering "fine" without having looked is the one answer it must not give.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import argparse
|
||||||
|
import pathlib
|
||||||
|
import shutil
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
|
import tempfile
|
||||||
|
|
||||||
|
TOOL = "blueprint-compiler"
|
||||||
|
|
||||||
|
|
||||||
|
def check(tool: str, ui: pathlib.Path, scratch: pathlib.Path) -> tuple[bool, list[str]]:
|
||||||
|
"""Round-trips one `.ui` through blueprint and returns (ok, advice)."""
|
||||||
|
blp = scratch / (ui.stem + ".blp")
|
||||||
|
|
||||||
|
# `decompile` only transcribes: it is the *compile* of its output that checks the properties, so
|
||||||
|
# both exit codes matter -- a file the decompiler cannot read at all is just as broken.
|
||||||
|
round_trip = subprocess.run(
|
||||||
|
[tool, "decompile", str(ui), "--output", str(blp)], capture_output=True, text=True
|
||||||
|
)
|
||||||
|
if round_trip.returncode != 0 or not blp.exists():
|
||||||
|
return False, [line for line in round_trip.stderr.splitlines() if line]
|
||||||
|
|
||||||
|
compiled = subprocess.run(
|
||||||
|
[tool, "compile", str(blp)], capture_output=True, text=True
|
||||||
|
)
|
||||||
|
if compiled.returncode != 0:
|
||||||
|
return False, [line for line in compiled.stderr.splitlines() if line]
|
||||||
|
|
||||||
|
linted = subprocess.run([tool, "lint", str(blp)], capture_output=True, text=True)
|
||||||
|
advice = [line for line in linted.stderr.splitlines() if line]
|
||||||
|
return True, advice
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> int:
|
||||||
|
parser = argparse.ArgumentParser(description=__doc__)
|
||||||
|
parser.add_argument("directory", nargs="?", default="src/ui",
|
||||||
|
help="where the .ui files are (default: src/ui)")
|
||||||
|
parser.add_argument("--lint", action="store_true",
|
||||||
|
help="print the linter's advice as well as the failures")
|
||||||
|
args = parser.parse_args()
|
||||||
|
|
||||||
|
tool = shutil.which(TOOL)
|
||||||
|
if tool is None:
|
||||||
|
print(f"check_ui: {TOOL} is not installed, so the UI files were NOT checked", file=sys.stderr)
|
||||||
|
print(" install it (Arch: pacman -S blueprint-compiler) and run this again",
|
||||||
|
file=sys.stderr)
|
||||||
|
return 2
|
||||||
|
|
||||||
|
directory = pathlib.Path(args.directory)
|
||||||
|
files = sorted(directory.glob("*.ui"))
|
||||||
|
if not files:
|
||||||
|
print(f"check_ui: no .ui files in {directory}", file=sys.stderr)
|
||||||
|
return 2
|
||||||
|
print(f"check_ui: {len(files)} files in {directory}")
|
||||||
|
|
||||||
|
failed = 0
|
||||||
|
advice_count = 0
|
||||||
|
with tempfile.TemporaryDirectory(prefix="check-ui-") as scratch_name:
|
||||||
|
scratch = pathlib.Path(scratch_name)
|
||||||
|
for ui in files:
|
||||||
|
ok, messages = check(tool, ui, scratch)
|
||||||
|
print(f" {'ok ' if ok else 'FAIL'} {ui.name}")
|
||||||
|
if ok:
|
||||||
|
advice_count += len([m for m in messages if m.startswith("warning:")])
|
||||||
|
else:
|
||||||
|
failed += 1
|
||||||
|
# A failure is always printed; advice only when it was asked for. Twenty lines of opinion
|
||||||
|
# on every run is how a check's output stops being read.
|
||||||
|
if not ok or args.lint:
|
||||||
|
for line in messages:
|
||||||
|
print(f" {line}")
|
||||||
|
|
||||||
|
print(f"check_ui: {len(files) - failed} ok, {failed} failed")
|
||||||
|
if advice_count:
|
||||||
|
print(f"check_ui: {advice_count} pieces of advice — not failures; --lint shows them")
|
||||||
|
return 1 if failed else 0
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
sys.exit(main())
|
||||||
Reference in new issue
Block a user