From 99a7d7170cb11599ac71a52c6dbf312729527424 Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Wed, 5 Aug 2026 23:27:01 +0000 Subject: [PATCH 1/2] fix: print the display name in compact output, not the config key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same failing check is named two ways depending on the format: $ commit-check -m --subject-imperative=true --no-banner CC003 subject-imperative check failed ==> docs: revamped the profile $ commit-check -m --subject-imperative=true --compact [FAIL] CC003 subject_imperative: docs: revamped the profile Both are text written for a person, so they should agree. The kebab-case form is the one that agrees with the rules reference, whose headings read `subject-imperative (CC003)` — so a name printed to a terminal can be searched for there verbatim, which is why the default output already converts. Every other human-facing surface in the ecosystem does the same: commit-check-action converts the JSON `check` field before showing it in step logs, job summaries and pull request comments. Compact was the only one left printing the raw key. The JSON output is deliberately unchanged. Its `check` field carries the snake_case key so a consumer can map a failure back to the `cchk.toml` option that controls it, which is what the action relies on. Both call sites now go through one helper rather than one of them open-coding the conversion, which is how they drifted apart to begin with. Refs #528 --- commit_check/util.py | 23 +++++++++++++++++++---- tests/main_test.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/commit_check/util.py b/commit_check/util.py index 8053d8c3..edd359e3 100644 --- a/commit_check/util.py +++ b/commit_check/util.py @@ -13,6 +13,22 @@ from commit_check import RED, GREEN, YELLOW, RESET_COLOR +def display_name(check_type: str) -> str: + """Human-readable form of a check name, e.g. ``subject-imperative``. + + Config files and the JSON output carry the snake_case key, because that is + what a reader sets in ``cchk.toml`` and what a consumer maps back to an + option. Text written for a person uses the kebab-case form instead: it is + how the rules reference titles each rule, so a name printed to a terminal + can be searched for there verbatim. + + Every text surface goes through here so the two forms cannot drift apart + again — the compact output once printed the config key while the default + output printed this one. + """ + return check_type.replace("_", "-") + + def _print_failure( check: dict, actual: str, @@ -23,7 +39,8 @@ def _print_failure( rule_id = check.get("rule_id", "") if compact: compact_value = actual.splitlines()[0] if actual else actual - label = f"{rule_id} {check['check']}" if rule_id else check["check"] + name = display_name(check["check"]) + label = f"{rule_id} {name}" if rule_id else name print(f"[FAIL] {label}: {compact_value}") return if not no_banner and not print_error_header.has_been_called: @@ -364,9 +381,7 @@ def print_error_message( :returns: Give error messages to user """ - # The kebab-case form is what the rules reference uses as its headings, so - # the name printed here can be searched for there verbatim. - name = check_type.replace("_", "-") + name = display_name(check_type) label = rule_id if rule_id and docs_url and supports_hyperlinks(): label = hyperlink(rule_id, docs_url) diff --git a/tests/main_test.py b/tests/main_test.py index 9cc43b41..d1b5f59c 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -728,6 +728,35 @@ def test_compact_shows_one_line_per_failure(self, mocker, capsys, monkeypatch): assert all(line.startswith("[FAIL]") for line in lines) assert len(lines) >= 1 + @pytest.mark.benchmark + def test_compact_names_checks_the_way_the_default_output_does( + self, mocker, capsys, monkeypatch + ): + """--compact prints the kebab-case name, not the config key. + + Both are text written for a person, so they have to agree. This + assertion is the one the suite was missing: --compact shipped + printing ``subject_imperative`` while the default output printed + ``subject-imperative``, and nothing here noticed. + """ + mocker.patch("sys.stdin.isatty", return_value=False) + mocker.patch("sys.stdin.read", return_value="docs: revamped the profile\n") + mocker.patch("commit_check.engine.get_commit_info", return_value="test-author") + + # A check whose name contains an underscore, so the two forms differ. + monkeypatch.setattr( + "sys.argv", [CMD, "-m", "--compact", "--subject-imperative=true"] + ) + main() + + out, _ = capsys.readouterr() + assert "CC003 subject-imperative:" in out, ( + f"--compact should print the display name: {out!r}" + ) + assert "subject_imperative" not in out, ( + f"--compact printed the config key: {out!r}" + ) + @pytest.mark.benchmark def test_compact_no_suggestions(self, mocker, capsys, monkeypatch): """--compact output must not include 'Suggest:' lines.""" From edd9cb64b48c4dbcd8d0f1232baf65a1b4653eac Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Wed, 5 Aug 2026 23:35:46 +0000 Subject: [PATCH 2/2] refactor: keep one implementation of the check display name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review pointed out that RuleCatalogEntry.name already did the same snake-to-kebab conversion, so the helper added in the previous commit was a second copy and its docstring claim that every text surface goes through it was false. Two copies of the rule this change exists to enforce is the wrong number. The helper moves to rules_catalog rather than the catalog importing it from util. rules_catalog is a pure data module — dataclasses and nothing else — and commit-check-mcp imports it directly, so it should not gain a dependency on a module that pulls in os, sys and subprocess. Pointing the arrow the other way keeps the catalog light and puts the formatter beside the entry whose name property defines the concept. Verified there is now one implementation, that both call sites reach it, and that importing util, rules_catalog, engine and main together raises no cycle. --- commit_check/rules_catalog.py | 18 +++++++++++++++++- commit_check/util.py | 17 +---------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/commit_check/rules_catalog.py b/commit_check/rules_catalog.py index 29717b2e..93faf529 100644 --- a/commit_check/rules_catalog.py +++ b/commit_check/rules_catalog.py @@ -24,6 +24,22 @@ RULES_DOCS_URL = "https://commit-check.com/rules/" +def display_name(check: str) -> str: + """Human-readable form of a check name, e.g. ``subject-imperative``. + + Config files and the JSON output carry the snake_case key, because that is + what a reader sets in ``cchk.toml`` and what a consumer maps back to an + option. Text written for a person uses the kebab-case form instead: it is + how the rules reference titles each rule, so a name printed to a terminal + can be searched for there verbatim. + + Every text surface goes through here so the two forms cannot drift apart + again — the compact output once printed the config key while the default + output printed this one. + """ + return check.replace("_", "-") + + @dataclass(frozen=True) class RuleCatalogEntry: check: str @@ -35,7 +51,7 @@ class RuleCatalogEntry: @property def name(self) -> str: """Human-readable rule name, e.g. ``subject-imperative``.""" - return self.check.replace("_", "-") + return display_name(self.check) @property def docs_url(self) -> str | None: diff --git a/commit_check/util.py b/commit_check/util.py index edd359e3..6de1af03 100644 --- a/commit_check/util.py +++ b/commit_check/util.py @@ -11,22 +11,7 @@ import sys from subprocess import CalledProcessError from commit_check import RED, GREEN, YELLOW, RESET_COLOR - - -def display_name(check_type: str) -> str: - """Human-readable form of a check name, e.g. ``subject-imperative``. - - Config files and the JSON output carry the snake_case key, because that is - what a reader sets in ``cchk.toml`` and what a consumer maps back to an - option. Text written for a person uses the kebab-case form instead: it is - how the rules reference titles each rule, so a name printed to a terminal - can be searched for there verbatim. - - Every text surface goes through here so the two forms cannot drift apart - again — the compact output once printed the config key while the default - output printed this one. - """ - return check_type.replace("_", "-") +from commit_check.rules_catalog import display_name def _print_failure(