Conversation
This comment has been minimized.
This comment has been minimized.
4a55334 to
56173ab
Compare
This comment has been minimized.
This comment has been minimized.
25ea629 to
7da5d3a
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…l modules why: Enable structured logging with `extra` context for filtering, testing, and aggregation. Library modules need NullHandler per Python best practices. what: - Add `logging.getLogger(__name__)` to every module - Add NullHandler in library `__init__.py` - Add TmuxpLoggerAdapter with process() override for Python <3.13 compat - Simplify tmuxp_echo to pure print wrapper (decoupled from logging) - Add setup_log_file() for centralized --log-file handler setup - Fix setup_logger to target "tmuxp" logger, skip NullHandler check - Change default CLI log level from INFO to WARNING - Fix timestamp format bug: %H:%m:%S -> %H:%M:%S - Add future annotations and fix import ordering in _compat.py
…oss all modules why: Structured `extra` keys (tmux_session, tmux_window, tmux_pane, tmux_config_path) enable filtering, aggregation, and test assertions on log records rather than string matching. what: - Add structured DEBUG/INFO/WARNING/ERROR log calls to workspace, CLI, and utility modules with appropriate extra keys - Route all raw print() calls through tmuxp_echo() in CLI commands - Fix get_pane() exception catch type to match sibling methods - Change before_script failure log from DEBUG to ERROR - Remove catch-log-reraise in plugin version check
why: Verify structured extra keys on log records using caplog.records, per AGENTS.md guidelines — assert on attributes, not string matching. what: - Add caplog tests for builder, freezer, finders, loader, validation, importers, and plugin version_check - Add log-level filtering test for --log-file - Add ANSI-free assertions to JSON/NDJSON output tests (ls, search) - Add non-TTY stderr ANSI-free test for load command
why: colorama wraps fixed ANSI escape string constants the stdlib can provide directly. Removing it shrinks the dependency tree. what: - Replace all colorama Fore/Style references in log.py with raw ANSI escapes via _ansi_colors from tmuxp._internal.colors - Remove colorama and types-colorama from pyproject.toml
….format_rule why: ls and debug-info bypassed OutputFormatter with raw sys.stdout.write, breaking the 2-channel output architecture for machine-readable output. what: - Add OutputFormatter.emit_object() for single top-level JSON objects - Route ls --json/--ndjson and debug-info --json through emit_object() - Add Colors.format_rule() for Unicode box-drawing horizontal rules - Add unit tests for emit_object in JSON, NDJSON, and HUMAN modes
why: CLAUDE.md requires all functions and methods to have working doctests. what: - Add Examples section to TmuxpLoggerAdapter demonstrating extra merging
why: Developers need guidance on the 2-channel output architecture (logger for diagnostics, tmuxp_echo/OutputFormatter for user output). what: - Add "Output channels" section with diagnostics vs user-facing rules - Document print() prohibition in command/business logic - Expand "Avoid" entry for print() with structured extra guidance
6 tasks
Member
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
why: PR 1 needs changelog entries for the logging work. what: - Add bug fixes: CLI log level, get_pane() exception, OutputFormatter routing - Add development section: structured logging, colorama removal, print→tmuxp_echo
why: logger.exception() dumps tracebacks at ERROR level (visible at default WARNING) alongside tmuxp_echo() user-friendly messages, causing users to see double output. what: - Change plugin load failed from exception to debug with exc_info - Change plugin import failed from exception to debug with exc_info - Change workspace build failed from exception to debug with exc_info
why: The structured logging migration removed the user-visible [Loading] message that shows which workspace file is being loaded. what: - Add tmuxp_echo with [Loading] and privacy-masked workspace path - Uses PrivatePath (already imported) and cli_colors (already available)
why: Inlining tmux stdout in the log message string defeats structured log aggregation and filtering. what: - Move display-message output from format arg to extra tmux_stdout key
why: All functions must have working doctests per project conventions. what: - Add Examples section with string and integer pass-through tests
why: The bare error message loses the script path, making it harder to diagnose which before_script failed in structured log systems. what: - Add tmux_config_path extra with the before_script path to error log
why: The raise...from e chain on the next lines already preserves the exception traceback; logging it too is redundant per project standards. what: - Remove exc_info=True from pane lookup debug log before reraise
why: The assertion checked for "Loading" or "Loaded" but the restored user-facing message now uses "[Loading]" format. what: - Update assertion to match the restored [Loading] message format
tony
added a commit
that referenced
this pull request
Mar 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
extracontext (tmux_session,tmux_window,tmux_pane,tmux_config_path) across all workspace, CLI, and utility modulesTmuxpLoggerAdapterfor persistent identity context on session/window/pane objects, portable across Python 3.10–3.13+NullHandlerin library__init__.pyandlogging.getLogger(__name__)declarations in all source modulescoloramaruntime and type-stub dependencies; replace with stdlib ANSI constantsOutputFormatter.emit_object()for single-object JSON output; routels --jsonanddebug-info --jsonthrough itColors.format_rule()with Unicode box-drawing charactersprint()calls throughtmuxp_echo()for consistent output channelsget_pane()exception catch type to match sibling methodsChanges by area
Logging infrastructure (
log.py)TmuxpLoggerAdapter:LoggerAdaptersubclass that mergesextradicts portably (manualprocess()override for Python < 3.13)setup_logger(): Defaults to"tmuxp"logger (not root), skipsNullHandlerwhen checking for existing handlers, selectsDebugLogFormatterat DEBUG leveltmuxp_echo(): Addsstacklevel=2for accurate caller attributionWorkspace modules
builder.py: Session/window/pane lifecycle logging viaTmuxpLoggerAdapterfinders.py: Structured DEBUG for local file search, WARNING for ambiguous multi-file directoriesfreezer.py,importers.py,loader.py,validation.py: DEBUG logging with session/config contextCLI and output
_output.py:OutputFormatter.emit_object()for JSON-mode commandscli/__init__.py: Fixsetup_loggercall site, routeprint()throughtmuxp_echo()load.py: Add missinglog_leveltoCLILoadNamespace, fix log-file handler setupls.py,debug_info.py: Route--jsonthroughOutputFormatterDependencies
pyproject.toml/uv.lock: Removecoloramaandcolorama-stubsDesign decisions
merge_extra=True: Python 3.13 addedmerge_extratoLoggerAdapter.__init__, but tmuxp supports 3.10+, soprocess()manually merges — forward-compatible with the newer APIsetup_loggerdefaults to"tmuxp"not root: Prevents handler setup from affecting unrelated libraries when tmuxp is the application entry pointlogger.warning+tmuxp_echofor ambiguity warnings: Structured log record goes to aggregators;tmuxp_echooutput goes to the terminal for user visibilityStacking
This PR is the base for:
tmuxp loadVerification
Verify no
print()calls remain in util.py:$ rg 'print\(' src/tmuxp/util.pyVerify all source modules have logger declarations:
$ rg -L 'logger = logging.getLogger' src/tmuxp/ --glob '*.py' --files-without-matchVerify no f-strings in log calls:
$ rg 'logger\.\w+\(f"' src/tmuxp/Test plan
test_builder_logs_session_created— verifies INFO record withtmux_sessionextratest_builder_logs_window_and_pane_creation— verifies DEBUG records withtmux_window/tmux_paneextratest_find_workspace_file_logs_warning_on_multiple— verifies WARNING on ambiguous workspace dirstest_freeze_logs_debug— verifies freeze session/window DEBUG recordstest_import_teamocil_logs_debug/test_import_tmuxinator_logs_debug— verifies import DEBUG with session nametest_get_pane_logs_debug_on_failure— verifies structured DEBUG on pane lookup failuretest_oh_my_zsh_auto_title_logs_warning— verifies WARNING when DISABLE_AUTO_TITLE unsetuv run ruff check .— cleanuv run mypy— cleanuv run py.test -x— all pass