From 2685c0e8cadd5e1c1cba688f53517db371268fd8 Mon Sep 17 00:00:00 2001 From: Marco Heinemann Date: Sun, 7 Jun 2026 23:50:23 +0200 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=90=9B=20Route=20analyse/=20logging?= =?UTF-8?q?=20by=20environment=20instead=20of=20a=20stderr=20handler=20at?= =?UTF-8?q?=20import?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The analyse/ modules installed a logging.StreamHandler on their own loggers at import time, sending routine INFO progress (`Source files loaded: N`, ...) to stderr unconditionally. Wrappers that colour stderr (tox, CI log viewers) rendered these harmless lines red, and importing the package took the handler/level/stream choice away from consumers — against the stdlib logging guidance for libraries. Add an environment-aware logging facade in logger.py (`get_logger` + a swappable backend) and route the analyse/ layer through it: - default (plain library): stdlib logging with no handler installed, so INFO is dropped and WARNING+ reaches stderr via logging.lastResort. - CLI: the existing rich logger — INFO to stdout, WARNING to stderr, honouring --verbose/--quiet (now added to the `analyse` command too). - Sphinx: sphinx.util.logging — progress at VERBOSE, warnings carrying type="codelinks" + a subtype so they are suppressible and rendered on the Sphinx warning stream (mirrors sphinx-needs' logging helper). utils.py's git warnings previously used logging.warning() on the root logger (triggering basicConfig); they now go through the facade with subtypes. The dead logging block in oneline_parser.py (no log calls) is removed. Closes #72 --- src/sphinx_codelinks/analyse/analyse.py | 10 +- .../analyse/oneline_parser.py | 9 -- src/sphinx_codelinks/analyse/projects.py | 10 +- src/sphinx_codelinks/analyse/utils.py | 51 ++++-- src/sphinx_codelinks/cmd.py | 7 +- src/sphinx_codelinks/logger.py | 151 ++++++++++++++++++ .../sphinx_extension/source_tracing.py | 4 + tests/test_cmd.py | 19 +++ tests/test_logger.py | 125 +++++++++++++++ 9 files changed, 344 insertions(+), 42 deletions(-) create mode 100644 tests/test_logger.py diff --git a/src/sphinx_codelinks/analyse/analyse.py b/src/sphinx_codelinks/analyse/analyse.py index 842c51b8..4a27cda4 100644 --- a/src/sphinx_codelinks/analyse/analyse.py +++ b/src/sphinx_codelinks/analyse/analyse.py @@ -1,7 +1,6 @@ from collections.abc import Generator from dataclasses import dataclass import json -import logging from pathlib import Path from typing import Any, TypedDict @@ -26,14 +25,9 @@ OneLineCommentStyle, SourceAnalyseConfig, ) +from sphinx_codelinks.logger import get_logger -# initialize logger -logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) -# log to the console -console = logging.StreamHandler() -console.setLevel(logging.INFO) -logger.addHandler(console) +logger = get_logger(__name__) class AnalyseWarningType(TypedDict): diff --git a/src/sphinx_codelinks/analyse/oneline_parser.py b/src/sphinx_codelinks/analyse/oneline_parser.py index 3395a1e0..91aabb52 100644 --- a/src/sphinx_codelinks/analyse/oneline_parser.py +++ b/src/sphinx_codelinks/analyse/oneline_parser.py @@ -1,17 +1,8 @@ from dataclasses import dataclass from enum import Enum -import logging from sphinx_codelinks.config import ESCAPE, UNIX_NEWLINE, OneLineCommentStyle -# initialize logger -logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) -# log to the console -console = logging.StreamHandler() -console.setLevel(logging.INFO) -logger.addHandler(console) - class WarningSubTypeEnum(str, Enum): """Enum for warning sub types.""" diff --git a/src/sphinx_codelinks/analyse/projects.py b/src/sphinx_codelinks/analyse/projects.py index 4a1d873c..0b2e428f 100644 --- a/src/sphinx_codelinks/analyse/projects.py +++ b/src/sphinx_codelinks/analyse/projects.py @@ -1,5 +1,4 @@ import json -import logging from pathlib import Path from typing import cast @@ -9,14 +8,9 @@ SourceAnalyse, ) from sphinx_codelinks.config import CodeLinksConfig, CodeLinksProjectConfigType +from sphinx_codelinks.logger import get_logger -# initialize logger -logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) -# log to the console -console = logging.StreamHandler() -console.setLevel(logging.INFO) -logger.addHandler(console) +logger = get_logger(__name__) class AnalyseProjects: diff --git a/src/sphinx_codelinks/analyse/utils.py b/src/sphinx_codelinks/analyse/utils.py index 5a11fddb..df37d63b 100644 --- a/src/sphinx_codelinks/analyse/utils.py +++ b/src/sphinx_codelinks/analyse/utils.py @@ -1,6 +1,5 @@ from collections.abc import ByteString, Callable import configparser -import logging from pathlib import Path from typing import TypedDict from urllib.request import pathname2url @@ -10,6 +9,7 @@ from tree_sitter import Node as TreeSitterNode from sphinx_codelinks.config import UNIX_NEWLINE, CommentCategory +from sphinx_codelinks.logger import get_logger from sphinx_codelinks.source_discover.config import CommentType # Language-specific node types for scope detection @@ -31,13 +31,7 @@ }, } -# initialize logger -logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) -# log to the console -console = logging.StreamHandler() -console.setLevel(logging.INFO) -logger.addHandler(console) +logger = get_logger(__name__) GIT_HOST_URL_TEMPLATE = { "github": "https://github.com/{owner}/{repo}/blob/{rev}/{path}#L{lineno}", @@ -270,7 +264,11 @@ def locate_git_root(src_dir: Path) -> Path | None: for parent in parents: if (parent / ".git").exists() and (parent / ".git").is_dir(): return parent - logger.warning(f"git root is not found in the parent of {src_dir}") + logger.warning( + f"git root is not found in the parent of {src_dir}", + subtype="git_root", + location=str(src_dir), + ) return None @@ -278,7 +276,11 @@ def get_remote_url(git_root: Path, remote_name: str = "origin") -> str | None: """Get remote url from .git/config.""" config_path = git_root / ".git" / "config" if not config_path.exists(): - logging.warning(f"{config_path} does not exist") + logger.warning( + f"{config_path} does not exist", + subtype="git_config", + location=str(config_path), + ) return None config = configparser.ConfigParser(allow_no_value=True, strict=False) @@ -287,7 +289,11 @@ def get_remote_url(git_root: Path, remote_name: str = "origin") -> str | None: if section in config and "url" in config[section]: url: str = config[section]["url"] return url - logger.warning(f"remote-url is not found in {config_path}") + logger.warning( + f"remote-url is not found in {config_path}", + subtype="git_remote", + location=str(config_path), + ) return None @@ -295,16 +301,28 @@ def get_current_rev(git_root: Path) -> str | None: """Get current commit rev from .git/HEAD.""" head_path = git_root / ".git" / "HEAD" if not head_path.exists(): - logging.warning(f"{head_path} does not exist") + logger.warning( + f"{head_path} does not exist", + subtype="git_head", + location=str(head_path), + ) return None head_content = head_path.read_text().strip() if not head_content.startswith("ref: "): - logging.warning(f"Expect starting with 'ref: ' in {head_path}") + logger.warning( + f"Expect starting with 'ref: ' in {head_path}", + subtype="git_head", + location=str(head_path), + ) return None ref_path = git_root / ".git" / head_content.split(":", 1)[1].strip() if not ref_path.exists(): - logging.warning(f"{ref_path} does not exist") + logger.warning( + f"{ref_path} does not exist", + subtype="git_ref", + location=str(ref_path), + ) return None return ref_path.read_text().strip() @@ -315,7 +333,10 @@ def form_https_url( parsed_url = parse(git_url) template = GIT_HOST_URL_TEMPLATE.get(parsed_url.platform) if not template: - logging.warning(f"Unsupported Git host: {parsed_url.platform}") + logger.warning( + f"Unsupported Git host: {parsed_url.platform}", + subtype="git_host", + ) return git_url https_url = template.format( owner=parsed_url.owner, diff --git a/src/sphinx_codelinks/cmd.py b/src/sphinx_codelinks/cmd.py index 29611163..f32a30d1 100644 --- a/src/sphinx_codelinks/cmd.py +++ b/src/sphinx_codelinks/cmd.py @@ -14,7 +14,7 @@ CodeLinksProjectConfigType, generate_project_configs, ) -from sphinx_codelinks.logger import logger +from sphinx_codelinks.logger import configure_cli, logger from sphinx_codelinks.needextend_write import MarkedObjType, convert_marked_content from sphinx_codelinks.source_discover.config import ( CommentType, @@ -88,9 +88,12 @@ def analyse( # noqa: PLR0912 # for CLI, so it needs the branches exists=True, ), ] = None, + verbose: OptVerbose = False, + quiet: OptQuiet = False, ) -> None: """Analyse marked content in source code.""" # @CLI command to analyse source code and extract traceability markers, IMPL_CLI_ANALYZE, impl, [FE_CLI_ANALYZE] + configure_cli(verbose, quiet) data: CodeLinksConfigType = load_config_from_toml(config) @@ -291,7 +294,7 @@ def write_rst( # noqa: PLR0913 # for CLI, so it takes as many as it requires quiet: OptQuiet = False, ) -> None: """Generate needextend.rst from the extracted obj in JSON.""" - logger.configure(verbose, quiet) + configure_cli(verbose, quiet) try: with jsonpath.open("r") as f: marked_content = json.load(f) diff --git a/src/sphinx_codelinks/logger.py b/src/sphinx_codelinks/logger.py index 05b3f352..d96411d1 100644 --- a/src/sphinx_codelinks/logger.py +++ b/src/sphinx_codelinks/logger.py @@ -1,5 +1,10 @@ +import logging +from typing import Protocol + from rich.console import Console from rich.text import Text +from sphinx import version_info as _sphinx_version_info +from sphinx.util import logging as sphinx_logging import typer @@ -92,3 +97,149 @@ def error( logger = Logger() + + +# --------------------------------------------------------------------------- # +# Environment-aware logging facade +# +# The ``analyse`` layer is shared between the standalone CLI and the Sphinx +# extension. Instead of installing handlers on its own loggers (which leaks +# routine INFO progress onto stderr; see issue #72), the layer logs through +# :func:`get_logger`, and the active *frontend* selects where records go: +# +# * default (plain library) -> stdlib logging with no handler installed, so +# INFO is dropped silently and WARNING+ falls back to stderr via +# ``logging.lastResort``. +# * CLI -> the rich :data:`logger` above (INFO to stdout, WARNING to stderr, +# honouring ``--verbose``/``--quiet``). +# * Sphinx -> ``sphinx.util.logging`` (respects verbosity, colour, +# ``suppress_warnings`` and the Sphinx warning stream). +# --------------------------------------------------------------------------- # + + +class _Backend(Protocol): + """Where the ``analyse`` layer's log records are routed. + + Arguments are positional-only so each backend may ignore (and underscore) + the ones it does not need. + """ + + def info(self, name: str, msg: str, location: str | None, /) -> None: ... + + def warning( + self, name: str, msg: str, subtype: str, location: str | None, / + ) -> None: ... + + +class _StdlibBackend: + """Default backend: emit through stdlib logging, install nothing. + + A library must not configure handlers on its own loggers. With no handler, + INFO records are dropped and WARNING+ reaches stderr via ``lastResort``. + """ + + def info(self, name: str, msg: str, _location: str | None, /) -> None: + logging.getLogger(name).info(msg) + + def warning( + self, name: str, msg: str, _subtype: str, _location: str | None, / + ) -> None: + logging.getLogger(name).warning(msg) + + +class _CliBackend: + """CLI backend: route through the rich :data:`logger`. + + Progress is INFO (stdout, hidden by ``--quiet``); warnings go to stderr. + """ + + def info(self, _name: str, msg: str, _location: str | None, /) -> None: + logger.info(msg) + + def warning( + self, _name: str, msg: str, _subtype: str, _location: str | None, / + ) -> None: + # reserve stderr for warnings/errors (the rich logger prints to stdout + # by default; route this to the error console explicitly) + logger.warning(msg, console=logger.err_console) + + +class _SphinxBackend: + """Sphinx backend: route through ``sphinx.util.logging``. + + Progress is emitted at VERBOSE (shown only with ``sphinx-build -v``); + warnings carry ``type="codelinks"`` plus a subtype so they are suppressible + via ``suppress_warnings`` and rendered on the Sphinx warning stream. + """ + + # Sphinx >= 8 renders the warning type itself; older versions need it + # appended to the message (mirrors sphinx-needs' logging helper). + _show_warning_types = _sphinx_version_info >= (8,) + + def info(self, name: str, msg: str, _location: str | None, /) -> None: + sphinx_logging.getLogger(name).verbose(msg) + + def warning( + self, name: str, msg: str, subtype: str, location: str | None, / + ) -> None: + message = msg + if not self._show_warning_types: + message += f" [codelinks.{subtype}]" if subtype else " [codelinks]" + sphinx_logging.getLogger(name).warning( + message, + type="codelinks", + subtype=subtype, + location=location, + ) + + +class _Dispatch: + """Holds the active backend, swapped by the ``configure_*`` entry points.""" + + def __init__(self) -> None: + self.backend: _Backend = _StdlibBackend() + + +_dispatch = _Dispatch() + + +class CodelinksLogger: + """Thin proxy used by the ``analyse`` layer. + + The active backend is resolved at *call* time so that an entry point may + select the frontend after the ``analyse`` modules have been imported. + """ + + __slots__ = ("_name",) + + def __init__(self, name: str) -> None: + self._name = name + + def info(self, msg: str, *, location: str | None = None) -> None: + _dispatch.backend.info(self._name, msg, location) + + def warning( + self, msg: str, *, subtype: str = "", location: str | None = None + ) -> None: + _dispatch.backend.warning(self._name, msg, subtype, location) + + +def get_logger(name: str) -> CodelinksLogger: + """Return a logger that routes to whichever frontend is configured.""" + return CodelinksLogger(name) + + +def configure_cli(verbose: bool = False, quiet: bool = False) -> None: + """Select the CLI frontend and configure the rich logger's verbosity.""" + logger.configure(verbose=verbose, quiet=quiet) + _dispatch.backend = _CliBackend() + + +def configure_sphinx() -> None: + """Select the Sphinx frontend (``sphinx.util.logging``).""" + _dispatch.backend = _SphinxBackend() + + +def reset() -> None: + """Restore the default (plain library) backend. Mainly for tests.""" + _dispatch.backend = _StdlibBackend() diff --git a/src/sphinx_codelinks/sphinx_extension/source_tracing.py b/src/sphinx_codelinks/sphinx_extension/source_tracing.py index 37504da3..80106d46 100644 --- a/src/sphinx_codelinks/sphinx_extension/source_tracing.py +++ b/src/sphinx_codelinks/sphinx_extension/source_tracing.py @@ -25,6 +25,7 @@ file_lineno_href, generate_project_configs, ) +from sphinx_codelinks.logger import configure_sphinx from sphinx_codelinks.sphinx_extension import debug from sphinx_codelinks.sphinx_extension.directives.src_trace import ( SourceTracing, @@ -52,6 +53,9 @@ def _check_sphinx_needs_dependency(app: Sphinx) -> bool: def setup(app: Sphinx) -> dict[str, Any]: # type: ignore[explicit-any] + # Route the shared analyse layer's logging through Sphinx (verbosity, + # colour, suppress_warnings, warning stream) instead of stderr. + configure_sphinx() # Check if sphinx-needs is available and properly configured if not _check_sphinx_needs_dependency(app): logger.error( diff --git a/tests/test_cmd.py b/tests/test_cmd.py index edc62a32..bb948119 100644 --- a/tests/test_cmd.py +++ b/tests/test_cmd.py @@ -116,6 +116,25 @@ def test_analyse_outputs_warnings(tmp_path: Path) -> None: assert "too_many_fields" in result.output +def test_analyse_progress_shown_by_default_and_hidden_when_quiet( + tmp_path: Path, +) -> None: + """Routine progress is shown on the CLI by default and silenced by --quiet.""" + config_path = DATA_DIR / "configs" / "minimum_config.toml" + + result = runner.invoke( + app, ["analyse", str(config_path), "--outdir", str(tmp_path)] + ) + assert result.exit_code == 0 + assert "Source files loaded" in result.output + + quiet_result = runner.invoke( + app, ["analyse", str(config_path), "--outdir", str(tmp_path), "--quiet"] + ) + assert quiet_result.exit_code == 0 + assert "Source files loaded" not in quiet_result.output + + @pytest.mark.parametrize( ("options", "stdout"), [ diff --git a/tests/test_logger.py b/tests/test_logger.py new file mode 100644 index 00000000..750f60c2 --- /dev/null +++ b/tests/test_logger.py @@ -0,0 +1,125 @@ +# @Test suite for the environment-aware logging facade, TEST_LOG_1, test, [IMPL_LOG_ROUTING] +import importlib +import logging + +import pytest +from sphinx.util.logging import VERBOSE + +from sphinx_codelinks import logger as logmod + + +@pytest.fixture(autouse=True) +def _reset_backend(): + """Each test starts and ends with the default (library) backend.""" + logmod.reset() + logmod.logger.configure(verbose=False, quiet=False) + yield + logmod.reset() + logmod.logger.configure(verbose=False, quiet=False) + + +def test_default_backend_drops_info_and_emits_warning(caplog): + """As a plain library (no frontend configured), routine INFO is silently + dropped while genuine warnings still propagate.""" + log = logmod.get_logger("sphinx_codelinks.analyse.sample") + + log.info("routine progress") + log.warning("real problem", subtype="git_root") + + messages = [record.getMessage() for record in caplog.records] + assert "routine progress" not in messages + assert "real problem" in messages + + +def test_cli_backend_routes_info_to_stdout_and_warning_to_stderr(capsys): + """CLI frontend: routine progress goes to stdout, warnings to stderr.""" + logmod.configure_cli(verbose=False, quiet=False) + log = logmod.get_logger("sphinx_codelinks.analyse.sample") + + log.info("files loaded: 3") + log.warning("git root not found", subtype="git_root") + + captured = capsys.readouterr() + assert "files loaded: 3" in captured.out + assert "files loaded: 3" not in captured.err + assert "git root not found" in captured.err + assert "git root not found" not in captured.out + + +def test_cli_backend_quiet_suppresses_info_but_keeps_warning(capsys): + """--quiet hides routine progress but never hides warnings.""" + logmod.configure_cli(verbose=False, quiet=True) + log = logmod.get_logger("sphinx_codelinks.analyse.sample") + + log.info("files loaded: 3") + log.warning("git root not found", subtype="git_root") + + captured = capsys.readouterr() + assert "files loaded: 3" not in captured.out + assert "git root not found" in captured.err + + +class _ListHandler(logging.Handler): + def __init__(self) -> None: + super().__init__() + self.records: list[logging.LogRecord] = [] + + def emit(self, record: logging.LogRecord) -> None: + self.records.append(record) + + +def test_sphinx_backend_routes_through_sphinx_logging(): + """Sphinx frontend: records flow through the ``sphinx.`` namespace; + progress is VERBOSE and warnings carry the codelinks type/subtype.""" + logmod.configure_sphinx() + + handler = _ListHandler() + sphinx_logger = logging.getLogger("sphinx") + sphinx_logger.addHandler(handler) + old_level = sphinx_logger.level + sphinx_logger.setLevel(VERBOSE) + try: + log = logmod.get_logger("sphinx_codelinks.analyse.sample") + log.info("files loaded: 3") + log.warning("git root not found", subtype="git_root", location="x.cpp") + finally: + sphinx_logger.removeHandler(handler) + sphinx_logger.setLevel(old_level) + + # routed under the sphinx-prefixed namespace Sphinx actually captures + assert all( + rec.name == "sphinx.sphinx_codelinks.analyse.sample" for rec in handler.records + ) + + info_records = [r for r in handler.records if "files loaded" in r.getMessage()] + assert info_records and info_records[0].levelno == VERBOSE + + warn_records = [ + r for r in handler.records if "git root not found" in r.getMessage() + ] + assert warn_records + assert warn_records[0].levelno == logging.WARNING + assert getattr(warn_records[0], "type", None) == "codelinks" + assert getattr(warn_records[0], "subtype", None) == "git_root" + + +ANALYSE_MODULE_LOGGERS = ( + "sphinx_codelinks.analyse.analyse", + "sphinx_codelinks.analyse.oneline_parser", + "sphinx_codelinks.analyse.projects", + "sphinx_codelinks.analyse.utils", +) + + +def test_analyse_modules_install_no_handlers_at_import(): + """Regression guard for #72: importing the analyse layer must not install + handlers or pin levels on its own loggers.""" + for name in ANALYSE_MODULE_LOGGERS: + importlib.import_module(name) + module_logger = logging.getLogger(name) + assert module_logger.handlers == [], ( + f"{name} installed handlers at import: {module_logger.handlers}" + ) + assert module_logger.level == logging.NOTSET, ( + f"{name} pinned its level at import to {module_logger.level}" + ) From 78a15a872b25e4bd06a0e143fc29698c9cc2bbb3 Mon Sep 17 00:00:00 2001 From: Marco Heinemann Date: Mon, 8 Jun 2026 00:10:37 +0200 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=90=9B=20Assert=20on=20result.output?= =?UTF-8?q?=20in=20CLI=20error=20tests=20(typer=200.24+=20dropped=20CliRun?= =?UTF-8?q?ner=20mix=5Fstderr)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit typer >= 0.24 (CI resolved 0.26.7) removed `mix_stderr` from `CliRunner`, so error output from `typer.BadParameter` now lands only on stderr and `result.stdout` is empty for error cases. The negative CLI tests asserted on `result.stdout`, so they began failing on every matrix cell once CI picked up the newer typer (independent of the logging change — they fail on main too). `result.output` carries the error text under both old and new typer, so use it for the error-path assertions. Success-output assertions (test_discover) are unaffected and left as-is. --- tests/test_cmd.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_cmd.py b/tests/test_cmd.py index bb948119..7887d799 100644 --- a/tests/test_cmd.py +++ b/tests/test_cmd.py @@ -246,7 +246,7 @@ def test_analyse_config_negative( ] result = runner.invoke(app, options) assert result.exit_code != 0 - normalized = _normalize_output(result.stdout) + normalized = _normalize_output(result.output) for line in output_lines: assert line in normalized @@ -280,7 +280,7 @@ def test_analyse_project_negative(projects, output_lines, tmp_path: Path) -> Non options.extend(projects_config) result = runner.invoke(app, options) assert result.exit_code != 0 - normalized = _normalize_output(result.stdout) + normalized = _normalize_output(result.output) for line in output_lines: assert line in normalized @@ -306,7 +306,7 @@ def test_write_rst_invalid_json(tmp_path: Path) -> None: result = runner.invoke(app, options) assert result.exit_code != 0 - assert "Expecting" in result.stdout + assert "Expecting" in result.output @pytest.mark.parametrize( @@ -351,7 +351,7 @@ def test_write_rst_negative(json_objs: list[dict], output_lines, tmp_path) -> No result = runner.invoke(app, options) assert result.exit_code != 0 - normalized = _normalize_output(result.stdout) + normalized = _normalize_output(result.output) for line in output_lines: assert line in normalized From d6f83d1e977c270efdf669607dcf0434ecf3c784 Mon Sep 17 00:00:00 2001 From: Marco Heinemann Date: Mon, 8 Jun 2026 00:10:48 +0200 Subject: [PATCH 3/6] =?UTF-8?q?=F0=9F=90=9B=20Return=20the=20SHA=20for=20a?= =?UTF-8?q?=20detached=20HEAD=20in=20get=5Fcurrent=5Frev;=20drop=20danglin?= =?UTF-8?q?g=20test=20need=20link?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routing the analyse layer's warnings through Sphinx (this PR) means they now reach the docs build, which runs `sphinx-build -nW` (warnings as errors). Two problems surfaced: - In a detached HEAD (CI checkouts), `.git/HEAD` holds the commit SHA directly. `get_current_rev` treated that as an error, warned (`codelinks.git_head`) and returned None — tripping `-W`. The SHA *is* the rev, so return it. This also yields correct remote source URLs in detached-HEAD builds. Adds a regression test for the detached-HEAD case. - The codelinks comment atop tests/test_logger.py declared need TEST_LOG_1 with a link to a non-existent IMPL_LOG_ROUTING, producing a sphinx-needs unknown-link warning. Drop the traceability comment (no matching impl need exists to link to). --- src/sphinx_codelinks/analyse/utils.py | 9 +++------ tests/test_analyse_utils.py | 11 +++++++++++ tests/test_logger.py | 2 +- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/sphinx_codelinks/analyse/utils.py b/src/sphinx_codelinks/analyse/utils.py index df37d63b..7bfd5bb1 100644 --- a/src/sphinx_codelinks/analyse/utils.py +++ b/src/sphinx_codelinks/analyse/utils.py @@ -309,12 +309,9 @@ def get_current_rev(git_root: Path) -> str | None: return None head_content = head_path.read_text().strip() if not head_content.startswith("ref: "): - logger.warning( - f"Expect starting with 'ref: ' in {head_path}", - subtype="git_head", - location=str(head_path), - ) - return None + # Detached HEAD (e.g. CI checkouts): .git/HEAD holds the commit SHA + # directly, which is exactly the rev we want. + return head_content ref_path = git_root / ".git" / head_content.split(":", 1)[1].strip() if not ref_path.exists(): diff --git a/tests/test_analyse_utils.py b/tests/test_analyse_utils.py index 207b1f9a..23ad162f 100644 --- a/tests/test_analyse_utils.py +++ b/tests/test_analyse_utils.py @@ -929,6 +929,17 @@ def test_get_current_rev(git_repo: tuple[Path, str]) -> None: assert current_rev == utils.get_current_rev(repo_path) +def test_get_current_rev_detached_head(tmp_path: Path) -> None: + """In a detached HEAD (e.g. CI checkouts) .git/HEAD holds the commit SHA + directly; get_current_rev returns it rather than warning and giving up.""" + git_root = tmp_path / "repo" + (git_root / ".git").mkdir(parents=True) + sha = "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678" + (git_root / ".git" / "HEAD").write_text(f"{sha}\n") + + assert utils.get_current_rev(git_root) == sha + + @pytest.mark.parametrize( ("text", "leading_sequences", "result"), [ diff --git a/tests/test_logger.py b/tests/test_logger.py index 750f60c2..52eb0cee 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -1,4 +1,4 @@ -# @Test suite for the environment-aware logging facade, TEST_LOG_1, test, [IMPL_LOG_ROUTING] +# Test suite for the environment-aware logging facade (issue #72). import importlib import logging From 607f785e17d2b6d28d319bba3ff67b87a25e32c9 Mon Sep 17 00:00:00 2001 From: Marco Heinemann Date: Mon, 8 Jun 2026 08:11:26 +0200 Subject: [PATCH 4/6] =?UTF-8?q?=E2=9C=A8=20Log=20a=20default-visible=20per?= =?UTF-8?q?-project=20codelinks=20summary;=20gate=20the=20breakdown=20behi?= =?UTF-8?q?nd=20-v?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By default a Sphinx build showed nothing from codelinks (progress was emitted at VERBOSE, i.e. only with `sphinx-build -v`), and even then the lines lacked project context. Emit a per-project marker from `SourceAnalyse.run()` — used by both the CLI (`AnalyseProjects`) and the Sphinx `src-trace` directive — so it appears in both frontends: - INFO (default-visible): `codelinks []: files, markers` (Sphinx status stream / CLI stdout, silenced by `--quiet`). - DEBUG (only with `-v` / `--verbose`): per-type breakdown — comments, oneline-needs, id-refs, marked-rst. `SourceAnalyse` gains a `name` kwarg (the project) which both callers pass. Adds a `debug` level to the logging facade (Sphinx `verbose()`, CLI rich `debug()`), and switches the Sphinx `info` mapping from `verbose()` to `info()` so the summary is visible in a normal build. The context-less per-step logging in `analyse.py` is removed in favour of the labelled summary. --- src/sphinx_codelinks/analyse/analyse.py | 29 ++++++++++------- src/sphinx_codelinks/analyse/projects.py | 4 +-- src/sphinx_codelinks/logger.py | 26 ++++++++++++--- .../sphinx_extension/directives/src_trace.py | 2 +- tests/test_cmd.py | 32 +++++++++++-------- tests/test_logger.py | 25 ++++++++++++--- 6 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/sphinx_codelinks/analyse/analyse.py b/src/sphinx_codelinks/analyse/analyse.py index 4a27cda4..91d4a149 100644 --- a/src/sphinx_codelinks/analyse/analyse.py +++ b/src/sphinx_codelinks/analyse/analyse.py @@ -51,7 +51,10 @@ class SourceAnalyse: def __init__( self, analyse_config: SourceAnalyseConfig, + *, + name: str = "", ) -> None: + self.name = name self.analyse_config = analyse_config self.src_files: list[SourceFile] = [] self.src_comments: list[SourceComment] = [] @@ -104,9 +107,6 @@ def create_src_objects(self) -> None: self.src_files.append(src_file) self.src_comments.extend(src_comments) - logger.info(f"Source files loaded: {len(self.src_files)}") - logger.info(f"Source comments extracted: {len(self.src_comments)}") - def extract_marker( self, text: str, @@ -347,13 +347,6 @@ def extract_marked_content(self) -> None: if marked_rst: self.marked_rst.append(marked_rst) - if self.analyse_config.get_need_id_refs: - logger.info(f"Need-id-refs extracted: {len(self.need_id_refs)}") - if self.analyse_config.get_oneline_needs: - logger.info(f"Oneline needs extracted: {len(self.oneline_needs)}") - if self.analyse_config.get_rst: - logger.info(f"Marked rst extracted: {len(self.marked_rst)}") - def merge_marked_content(self) -> None: self.all_marked_content.extend(self.need_id_refs) self.oneline_needs.sort(key=lambda x: x.source_map["start"]["row"]) @@ -372,9 +365,23 @@ def dump_marked_content(self, outdir: Path) -> None: ] with output_path.open("w") as f: json.dump(to_dump, f) - logger.info(f"Marked content dumped to {output_path}") def run(self) -> None: self.create_src_objects() self.extract_marked_content() self.merge_marked_content() + self._log_summary() + + def _log_summary(self) -> None: + """Emit a per-project marker (default-visible) plus a -v breakdown.""" + label = f"codelinks [{self.name}]" if self.name else "codelinks" + logger.info( + f"{label}: {len(self.src_files)} files, " + f"{len(self.all_marked_content)} markers" + ) + logger.debug( + f"{label}: {len(self.src_comments)} comments, " + f"{len(self.oneline_needs)} oneline-needs, " + f"{len(self.need_id_refs)} id-refs, " + f"{len(self.marked_rst)} marked-rst" + ) diff --git a/src/sphinx_codelinks/analyse/projects.py b/src/sphinx_codelinks/analyse/projects.py index 0b2e428f..b3fdb0f2 100644 --- a/src/sphinx_codelinks/analyse/projects.py +++ b/src/sphinx_codelinks/analyse/projects.py @@ -26,7 +26,7 @@ def __init__(self, codelink_config: CodeLinksConfig) -> None: def run(self) -> None: for project, config in self.projects_configs.items(): - src_analyse = SourceAnalyse(config["analyse_config"]) + src_analyse = SourceAnalyse(config["analyse_config"], name=project) src_analyse.run() self.projects_analyse[project] = src_analyse @@ -40,7 +40,7 @@ def dump_markers(self) -> None: } with output_path.open("w") as f: json.dump(to_dump, f) - logger.info(f"Marked content dumped to {output_path}") + logger.debug(f"codelinks: marked content dumped to {output_path}") @classmethod def load_warnings(cls, warnings_dir: Path) -> list[AnalyseWarning] | None: diff --git a/src/sphinx_codelinks/logger.py b/src/sphinx_codelinks/logger.py index d96411d1..f1004554 100644 --- a/src/sphinx_codelinks/logger.py +++ b/src/sphinx_codelinks/logger.py @@ -124,6 +124,8 @@ class _Backend(Protocol): the ones it does not need. """ + def debug(self, name: str, msg: str, location: str | None, /) -> None: ... + def info(self, name: str, msg: str, location: str | None, /) -> None: ... def warning( @@ -138,6 +140,9 @@ class _StdlibBackend: INFO records are dropped and WARNING+ reaches stderr via ``lastResort``. """ + def debug(self, name: str, msg: str, _location: str | None, /) -> None: + logging.getLogger(name).debug(msg) + def info(self, name: str, msg: str, _location: str | None, /) -> None: logging.getLogger(name).info(msg) @@ -150,9 +155,13 @@ def warning( class _CliBackend: """CLI backend: route through the rich :data:`logger`. - Progress is INFO (stdout, hidden by ``--quiet``); warnings go to stderr. + The summary is INFO (stdout, hidden by ``--quiet``); the breakdown is DEBUG + (stdout, shown only with ``--verbose``); warnings go to stderr. """ + def debug(self, _name: str, msg: str, _location: str | None, /) -> None: + logger.debug(msg) + def info(self, _name: str, msg: str, _location: str | None, /) -> None: logger.info(msg) @@ -167,18 +176,22 @@ def warning( class _SphinxBackend: """Sphinx backend: route through ``sphinx.util.logging``. - Progress is emitted at VERBOSE (shown only with ``sphinx-build -v``); - warnings carry ``type="codelinks"`` plus a subtype so they are suppressible - via ``suppress_warnings`` and rendered on the Sphinx warning stream. + The summary is INFO (shown in a normal build's status stream); the breakdown + is VERBOSE (shown only with ``sphinx-build -v``); warnings carry + ``type="codelinks"`` plus a subtype so they are suppressible via + ``suppress_warnings`` and rendered on the Sphinx warning stream. """ # Sphinx >= 8 renders the warning type itself; older versions need it # appended to the message (mirrors sphinx-needs' logging helper). _show_warning_types = _sphinx_version_info >= (8,) - def info(self, name: str, msg: str, _location: str | None, /) -> None: + def debug(self, name: str, msg: str, _location: str | None, /) -> None: sphinx_logging.getLogger(name).verbose(msg) + def info(self, name: str, msg: str, _location: str | None, /) -> None: + sphinx_logging.getLogger(name).info(msg) + def warning( self, name: str, msg: str, subtype: str, location: str | None, / ) -> None: @@ -215,6 +228,9 @@ class CodelinksLogger: def __init__(self, name: str) -> None: self._name = name + def debug(self, msg: str, *, location: str | None = None) -> None: + _dispatch.backend.debug(self._name, msg, location) + def info(self, msg: str, *, location: str | None = None) -> None: _dispatch.backend.info(self._name, msg, location) diff --git a/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py b/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py index c04f1332..0b522f77 100644 --- a/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py +++ b/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py @@ -123,7 +123,7 @@ def run(self) -> list[nodes.Node]: src_trace_toml_path = Path(src_trace_sphinx_config.config_from_toml) conf_dir = conf_dir / src_trace_toml_path.parent analyse_config.git_root = (conf_dir / analyse_config.git_root).resolve() - src_analyse = SourceAnalyse(analyse_config) + src_analyse = SourceAnalyse(analyse_config, name=project) src_analyse.run() dirs = { diff --git a/tests/test_cmd.py b/tests/test_cmd.py index 7887d799..3d3c67d2 100644 --- a/tests/test_cmd.py +++ b/tests/test_cmd.py @@ -116,23 +116,29 @@ def test_analyse_outputs_warnings(tmp_path: Path) -> None: assert "too_many_fields" in result.output -def test_analyse_progress_shown_by_default_and_hidden_when_quiet( - tmp_path: Path, -) -> None: - """Routine progress is shown on the CLI by default and silenced by --quiet.""" +def test_analyse_logs_per_project_summary_and_gates_detail(tmp_path: Path) -> None: + """Each project gets a default-visible ``codelinks []`` summary with + counts; the per-type breakdown is gated behind --verbose; --quiet silences it.""" config_path = DATA_DIR / "configs" / "minimum_config.toml" + base = ["analyse", str(config_path), "--outdir", str(tmp_path)] - result = runner.invoke( - app, ["analyse", str(config_path), "--outdir", str(tmp_path)] - ) + # default: per-project summary shown, breakdown hidden + result = runner.invoke(app, base) assert result.exit_code == 0 - assert "Source files loaded" in result.output - - quiet_result = runner.invoke( - app, ["analyse", str(config_path), "--outdir", str(tmp_path), "--quiet"] - ) + assert "codelinks [" in result.output + assert "markers" in result.output + assert "oneline-needs" not in result.output + + # --verbose: per-type breakdown also shown + verbose_result = runner.invoke(app, [*base, "--verbose"]) + assert verbose_result.exit_code == 0 + assert "codelinks [" in verbose_result.output + assert "oneline-needs" in verbose_result.output + + # --quiet: summary silenced + quiet_result = runner.invoke(app, [*base, "--quiet"]) assert quiet_result.exit_code == 0 - assert "Source files loaded" not in quiet_result.output + assert "codelinks [" not in quiet_result.output @pytest.mark.parametrize( diff --git a/tests/test_logger.py b/tests/test_logger.py index 52eb0cee..dd43cfb4 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -59,6 +59,17 @@ def test_cli_backend_quiet_suppresses_info_but_keeps_warning(capsys): assert "git root not found" in captured.err +def test_cli_backend_debug_is_gated_by_verbose(capsys): + """CLI frontend: detailed debug output is hidden by default, shown with -v.""" + logmod.configure_cli(verbose=False, quiet=False) + logmod.get_logger("sphinx_codelinks.analyse.sample").debug("breakdown detail") + assert "breakdown detail" not in capsys.readouterr().out + + logmod.configure_cli(verbose=True, quiet=False) + logmod.get_logger("sphinx_codelinks.analyse.sample").debug("breakdown detail") + assert "breakdown detail" in capsys.readouterr().out + + class _ListHandler(logging.Handler): def __init__(self) -> None: super().__init__() @@ -69,8 +80,8 @@ def emit(self, record: logging.LogRecord) -> None: def test_sphinx_backend_routes_through_sphinx_logging(): - """Sphinx frontend: records flow through the ``sphinx.`` namespace; - progress is VERBOSE and warnings carry the codelinks type/subtype.""" + """Sphinx frontend: info is default-visible (INFO), debug is -v only + (VERBOSE), warnings carry the codelinks type/subtype; all under sphinx.*""" logmod.configure_sphinx() handler = _ListHandler() @@ -80,7 +91,8 @@ def test_sphinx_backend_routes_through_sphinx_logging(): sphinx_logger.setLevel(VERBOSE) try: log = logmod.get_logger("sphinx_codelinks.analyse.sample") - log.info("files loaded: 3") + log.info("project summary") + log.debug("breakdown detail") log.warning("git root not found", subtype="git_root", location="x.cpp") finally: sphinx_logger.removeHandler(handler) @@ -91,8 +103,11 @@ def test_sphinx_backend_routes_through_sphinx_logging(): rec.name == "sphinx.sphinx_codelinks.analyse.sample" for rec in handler.records ) - info_records = [r for r in handler.records if "files loaded" in r.getMessage()] - assert info_records and info_records[0].levelno == VERBOSE + info_records = [r for r in handler.records if "project summary" in r.getMessage()] + assert info_records and info_records[0].levelno == logging.INFO + + debug_records = [r for r in handler.records if "breakdown detail" in r.getMessage()] + assert debug_records and debug_records[0].levelno == VERBOSE warn_records = [ r for r in handler.records if "git root not found" in r.getMessage() From 8b0794d4e1b18abdf947c291f6c12297115e138b Mon Sep 17 00:00:00 2001 From: Marco Heinemann Date: Mon, 8 Jun 2026 08:21:08 +0200 Subject: [PATCH 5/6] =?UTF-8?q?=F0=9F=91=8C=20Pluralize=20counts=20in=20th?= =?UTF-8?q?e=20codelinks=20project=20summary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "1 file, 1 marker" instead of "1 files, 1 markers"; applies to the default summary and the -v breakdown (comments / oneline needs / id-refs / marked-rst blocks) via a small `_count` helper. --- src/sphinx_codelinks/analyse/analyse.py | 17 +++++++++++------ tests/test_analyse.py | 9 +++++++++ tests/test_cmd.py | 4 ++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/sphinx_codelinks/analyse/analyse.py b/src/sphinx_codelinks/analyse/analyse.py index 91d4a149..cfd10177 100644 --- a/src/sphinx_codelinks/analyse/analyse.py +++ b/src/sphinx_codelinks/analyse/analyse.py @@ -30,6 +30,11 @@ logger = get_logger(__name__) +def _count(n: int, noun: str) -> str: + """Format ``n noun`` with a naive (append-s) plural for progress summaries.""" + return f"{n} {noun}" if n == 1 else f"{n} {noun}s" + + class AnalyseWarningType(TypedDict): file_path: str lineno: int @@ -376,12 +381,12 @@ def _log_summary(self) -> None: """Emit a per-project marker (default-visible) plus a -v breakdown.""" label = f"codelinks [{self.name}]" if self.name else "codelinks" logger.info( - f"{label}: {len(self.src_files)} files, " - f"{len(self.all_marked_content)} markers" + f"{label}: {_count(len(self.src_files), 'file')}, " + f"{_count(len(self.all_marked_content), 'marker')}" ) logger.debug( - f"{label}: {len(self.src_comments)} comments, " - f"{len(self.oneline_needs)} oneline-needs, " - f"{len(self.need_id_refs)} id-refs, " - f"{len(self.marked_rst)} marked-rst" + f"{label}: {_count(len(self.src_comments), 'comment')}, " + f"{_count(len(self.oneline_needs), 'oneline need')}, " + f"{_count(len(self.need_id_refs), 'id-ref')}, " + f"{_count(len(self.marked_rst), 'marked-rst block')}" ) diff --git a/tests/test_analyse.py b/tests/test_analyse.py index 6e6c2a7f..64474919 100644 --- a/tests/test_analyse.py +++ b/tests/test_analyse.py @@ -232,3 +232,12 @@ def test_oneline_parser_warnings_are_collected(tmp_path): warning = src_analyse.oneline_warnings[0] assert "too_many_fields" in warning.sub_type assert warning.lineno == 17 + + +def test_count_pluralizes_nouns() -> None: + from sphinx_codelinks.analyse.analyse import _count + + assert _count(0, "file") == "0 files" + assert _count(1, "file") == "1 file" + assert _count(2, "marker") == "2 markers" + assert _count(1, "marked-rst block") == "1 marked-rst block" diff --git a/tests/test_cmd.py b/tests/test_cmd.py index 3d3c67d2..d6533e4b 100644 --- a/tests/test_cmd.py +++ b/tests/test_cmd.py @@ -127,13 +127,13 @@ def test_analyse_logs_per_project_summary_and_gates_detail(tmp_path: Path) -> No assert result.exit_code == 0 assert "codelinks [" in result.output assert "markers" in result.output - assert "oneline-needs" not in result.output + assert "oneline need" not in result.output # --verbose: per-type breakdown also shown verbose_result = runner.invoke(app, [*base, "--verbose"]) assert verbose_result.exit_code == 0 assert "codelinks [" in verbose_result.output - assert "oneline-needs" in verbose_result.output + assert "oneline need" in verbose_result.output # --quiet: summary silenced quiet_result = runner.invoke(app, [*base, "--quiet"]) From 74551fca3159c111ae29ace435ed6ae1263982ef Mon Sep 17 00:00:00 2001 From: Marco Heinemann Date: Mon, 8 Jun 2026 08:25:09 +0200 Subject: [PATCH 6/6] =?UTF-8?q?=F0=9F=94=A7=20Import=20=5Fcount=20at=20mod?= =?UTF-8?q?ule=20top=20in=20test=20(ruff=20PLC0415)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pluralization test imported the helper inside the function; PLC0415 is only ignored for cmd.py, so the in-function import failed the ruff pre-commit hook. Move it to the top-level import. --- tests/test_analyse.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_analyse.py b/tests/test_analyse.py index 64474919..9617b7b0 100644 --- a/tests/test_analyse.py +++ b/tests/test_analyse.py @@ -4,7 +4,7 @@ import pytest -from sphinx_codelinks.analyse.analyse import SourceAnalyse +from sphinx_codelinks.analyse.analyse import SourceAnalyse, _count from sphinx_codelinks.config import SourceAnalyseConfig from tests.conftest import ( ONELINE_COMMENT_STYLE, @@ -235,8 +235,6 @@ def test_oneline_parser_warnings_are_collected(tmp_path): def test_count_pluralizes_nouns() -> None: - from sphinx_codelinks.analyse.analyse import _count - assert _count(0, "file") == "0 files" assert _count(1, "file") == "1 file" assert _count(2, "marker") == "2 markers"