diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 366379320e23..6db70293b865 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,6 +51,9 @@ jobs: - name: Validate Swift file length budget guard run: ./tests/test_ci_swift_file_length_budget.sh + - name: Validate auxiliary window close shortcut lint + run: ./tests/test_ci_auxiliary_window_close_shortcuts.sh + - name: Validate CircleCI auto approval run: python3 tests/test_circleci_auto_approve.py diff --git a/Sources/Feed/FeedTextEditorDebugWindowController.swift b/Sources/Feed/FeedTextEditorDebugWindowController.swift index 2999642ca2e7..5ea8d6e73377 100644 --- a/Sources/Feed/FeedTextEditorDebugWindowController.swift +++ b/Sources/Feed/FeedTextEditorDebugWindowController.swift @@ -16,6 +16,7 @@ final class FeedTextEditorDebugWindowController: NSWindowController, NSWindowDel localized: "feed.textEditorDebug.windowTitle", defaultValue: "Feed Text Editor Lab" ) + window.identifier = NSUserInterfaceItemIdentifier("cmux.feedTextEditorDebug") window.center() window.contentView = NSHostingView(rootView: FeedTextEditorDebugView()) super.init(window: window) diff --git a/Sources/cmuxApp.swift b/Sources/cmuxApp.swift index d40991cd69ed..f765b1ba5dc7 100644 --- a/Sources/cmuxApp.swift +++ b/Sources/cmuxApp.swift @@ -1142,6 +1142,16 @@ private let cmuxAuxiliaryWindowIdentifiers: Set = [ "cmux.about", "cmux.licenses", "cmux.browser-popup", + "cmux.browserProfilePopoverDebug", + "cmux.configEditor", + "cmux.feedButtonStyleDebug", + "cmux.feedPreview", + "cmux.feedTextEditorDebug", + "cmux.fileExplorerStyleDebug", + "cmux.pdfPreviewChromeDebug", + "cmux.splitButtonLayoutDebug", + "cmux.tabBarBackdropLab", + "cmux.taskManager", "cmux.aboutTitlebarDebug", "cmux.debugWindowControls", "cmux.browserImportHintDebug", diff --git a/scripts/lint_auxiliary_window_close_shortcuts.py b/scripts/lint_auxiliary_window_close_shortcuts.py new file mode 100755 index 000000000000..cda59ae88329 --- /dev/null +++ b/scripts/lint_auxiliary_window_close_shortcuts.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python3 +"""Require standalone cmux windows to own the standard close shortcut.""" + +from __future__ import annotations + +import argparse +import pathlib +import re +import sys + + +DEFAULT_ROOTS = ("Sources",) +OWNER_LIST_PATH = pathlib.Path("Sources/cmuxApp.swift") +OWNER_LIST_NAME = "cmuxAuxiliaryWindowIdentifiers" + +# Hidden/internal bootstrap windows should not take Cmd+W away from the active +# main window. Add to this set only when a window is intentionally not user +# closable. +IGNORED_IDENTIFIERS = { + "cmux.bootstrap", +} + +IDENTIFIER_ASSIGNMENT_RE = re.compile( + r"""\b[A-Za-z_][A-Za-z0-9_]*\.identifier\s*=\s*NSUserInterfaceItemIdentifier\("(?Pcmux\.[^"]+)"\)""" +) +STRING_LITERAL_RE = re.compile(r'"(?Pcmux\.[^"]+)"') +BLOCK_COMMENT_RE = re.compile(r"/\*.*?\*/", re.DOTALL) +LINE_COMMENT_RE = re.compile(r"//[^\n]*") + + +def strip_line_comments(text: str) -> str: + text = BLOCK_COMMENT_RE.sub(lambda match: "\n" * match.group(0).count("\n"), text) + return LINE_COMMENT_RE.sub("", text) + + +def load_close_owner_identifiers(repo_root: pathlib.Path) -> set[str]: + path = repo_root / OWNER_LIST_PATH + try: + text = path.read_text(encoding="utf-8") + except FileNotFoundError: + raise ValueError(f"missing {OWNER_LIST_PATH}") from None + + parse_text = strip_line_comments(text) + marker = f"private let {OWNER_LIST_NAME}" + marker_index = parse_text.find(marker) + if marker_index < 0: + raise ValueError(f"missing {OWNER_LIST_NAME} in {OWNER_LIST_PATH}") + + list_start = parse_text.find("[", marker_index) + if list_start < 0: + raise ValueError(f"could not parse {OWNER_LIST_NAME} in {OWNER_LIST_PATH}") + + depth = 0 + list_end = -1 + for index in range(list_start, len(parse_text)): + if parse_text[index] == "[": + depth += 1 + elif parse_text[index] == "]": + depth -= 1 + if depth == 0: + list_end = index + break + if list_end < 0: + raise ValueError(f"could not parse {OWNER_LIST_NAME} in {OWNER_LIST_PATH}") + + list_body = parse_text[list_start:list_end] + return {match.group("identifier") for match in STRING_LITERAL_RE.finditer(list_body)} + + +def collect_window_identifier_assignments( + repo_root: pathlib.Path, + roots: tuple[str, ...], +) -> dict[str, list[str]]: + assignments: dict[str, list[str]] = {} + for root in roots: + root_path = repo_root / root + if not root_path.exists(): + continue + for path in sorted(root_path.rglob("*.swift")): + rel_path = path.relative_to(repo_root).as_posix() + text = strip_line_comments(path.read_text(encoding="utf-8", errors="replace")) + for match in IDENTIFIER_ASSIGNMENT_RE.finditer(text): + identifier = match.group("identifier") + line_number = text.count("\n", 0, match.start()) + 1 + assignments.setdefault(identifier, []).append(f"{rel_path}:{line_number}") + return assignments + + +def main(argv: list[str]) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--repo-root", + default=pathlib.Path.cwd(), + type=pathlib.Path, + help="repository root to scan", + ) + parser.add_argument( + "--roots", + nargs="+", + default=list(DEFAULT_ROOTS), + help="repo-relative Swift roots to scan", + ) + args = parser.parse_args(argv) + + repo_root = args.repo_root.resolve(strict=False) + try: + close_owners = load_close_owner_identifiers(repo_root) + except ValueError as exc: + print(f"Auxiliary window close-shortcut lint could not run: {exc}", file=sys.stderr) + return 2 + + assignments = collect_window_identifier_assignments(repo_root, tuple(args.roots)) + missing = { + identifier: locations + for identifier, locations in assignments.items() + if identifier not in close_owners and identifier not in IGNORED_IDENTIFIERS + } + + if missing: + print("Auxiliary window close-shortcut lint failed.") + print("") + print( + "These cmux window identifiers are assigned to NSWindow/NSPanel " + f"but are missing from {OWNER_LIST_NAME}:" + ) + for identifier in sorted(missing): + print(f"- {identifier}") + for location in missing[identifier]: + print(f" {location}") + print("") + print( + f"Add each user-closable window to {OWNER_LIST_NAME} in {OWNER_LIST_PATH}, " + "or add a documented lint ignore for internal windows that must not own Cmd+W." + ) + return 1 + + print("Auxiliary window close-shortcut lint passed.") + print(f"Checked {len(assignments)} cmux window identifier(s).") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv[1:])) diff --git a/tests/test_ci_auxiliary_window_close_shortcuts.sh b/tests/test_ci_auxiliary_window_close_shortcuts.sh new file mode 100755 index 000000000000..96b99fb73fb7 --- /dev/null +++ b/tests/test_ci_auxiliary_window_close_shortcuts.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$ROOT_DIR" + +python3 scripts/lint_auxiliary_window_close_shortcuts.py + +TMP_DIR="$(mktemp -d)" +trap 'rm -rf "$TMP_DIR"' EXIT + +mkdir -p "$TMP_DIR/Sources" + +cat > "$TMP_DIR/Sources/cmuxApp.swift" <<'SWIFT' +private let cmuxAuxiliaryWindowIdentifiers: Set = [ + "cmux.settings", +] +SWIFT + +cat > "$TMP_DIR/Sources/NewWindow.swift" <<'SWIFT' +import AppKit + +/* +window.identifier = NSUserInterfaceItemIdentifier("cmux.blockCommentOnly") +*/ + +func makeWindow() { + let window = NSWindow() + window.identifier = + NSUserInterfaceItemIdentifier("cmux.newWindow") +} +SWIFT + +if python3 scripts/lint_auxiliary_window_close_shortcuts.py --repo-root "$TMP_DIR" >"$TMP_DIR/missing.out" 2>&1; then + echo "Expected missing auxiliary-window close owner to fail" >&2 + exit 1 +fi +grep -q "cmux.newWindow" "$TMP_DIR/missing.out" +grep -q "Sources/NewWindow.swift:9" "$TMP_DIR/missing.out" + +cat > "$TMP_DIR/Sources/cmuxApp.swift" <<'SWIFT' +private let cmuxAuxiliaryWindowIdentifiers: Set = [ + // "cmux.newWindow", + /* + "cmux.newWindow", + */ + "cmux.settings", +] +SWIFT + +if python3 scripts/lint_auxiliary_window_close_shortcuts.py --repo-root "$TMP_DIR" >"$TMP_DIR/commented-owner.out" 2>&1; then + echo "Expected commented-out auxiliary-window close owner to be ignored" >&2 + exit 1 +fi +grep -q "cmux.newWindow" "$TMP_DIR/commented-owner.out" + +cat > "$TMP_DIR/Sources/cmuxApp.swift" <<'SWIFT' +private let cmuxAuxiliaryWindowIdentifiers: Set = [ + // MARK: - Main Windows [user-closable] + // This comment intentionally contains a lone ] bracket. + "cmux.newWindow", + "cmux.settings", +] +SWIFT + +python3 scripts/lint_auxiliary_window_close_shortcuts.py --repo-root "$TMP_DIR" + +cat > "$TMP_DIR/Sources/NewWindow.swift" <<'SWIFT' +import AppKit + +func makeWindow() { + let window = NSWindow() + /* + window.identifier = NSUserInterfaceItemIdentifier("cmux.blockCommentOnly") + */ + // window.identifier = NSUserInterfaceItemIdentifier("cmux.commentOnly") + _ = window +} +SWIFT + +python3 scripts/lint_auxiliary_window_close_shortcuts.py --repo-root "$TMP_DIR" + +cat > "$TMP_DIR/Sources/cmuxApp.swift" <<'SWIFT' +private let cmuxAuxiliaryWindowIdentifiers: Set = [ + "cmux.settings", +] +SWIFT + +cat > "$TMP_DIR/Sources/NewWindow.swift" <<'SWIFT' +import AppKit + +func makeWindow() { + let window = NSWindow() + window.identifier = NSUserInterfaceItemIdentifier("cmux.bootstrap") +} +SWIFT + +python3 scripts/lint_auxiliary_window_close_shortcuts.py --repo-root "$TMP_DIR"