-
Notifications
You must be signed in to change notification settings - Fork 112
Improved option management, add hook management #516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
tony
wants to merge
80
commits into
master
Choose a base branch
from
improved-options
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+9,490
−349
Conversation
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
6cd43f0 to
3399c7d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #516 +/- ##
==========================================
+ Coverage 46.21% 49.28% +3.06%
==========================================
Files 19 38 +19
Lines 1876 3058 +1182
Branches 294 470 +176
==========================================
+ Hits 867 1507 +640
- Misses 894 1371 +477
- Partials 115 180 +65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closed
e5c6186 to
37898a4
Compare
97d74c1 to
36c5907
Compare
- Update SparseArray type parameter to include all value types - Add proper type checking for both dict and SparseArray cases - Fix return type casting in _show_option method - Resolve mypy errors related to string indexing of SparseArray
- Add version-specific assertions for terminal features - Handle rxvt* features in tmux 3.4+ - Make test more robust by checking individual features
WHAT: - Add comprehensive test coverage for complex option values and edge cases - Add tests for terminal features edge cases including empty/malformed features - Add tests for style validation and error handling - Fix type safety issues in terminal features tests WHY: - Ensure proper handling of tmux's complex option types: * Empty feature lists (tmux returns [''] not []) * Malformed feature strings with colons * Features containing spaces and special chars - Improve type safety by: * Adding proper type assertions for runtime checks * Using t.cast() to handle complex union types * Fixing string vs int indexing for SparseArray - Make tests more maintainable by: * Breaking complex assertions into steps * Using descriptive variable names * Adding clear comments about tmux behavior
WHAT: - Add type assertions for style option values - Convert style values to strings before comparison - Fix mypy errors in test_style_option_validation WHY: - Ensure type safety when handling style option values - Prevent potential runtime errors from type mismatches - Make test assertions more robust and explicit
WHAT: - Add version check for status-format option (added in tmux 2.9) - Update test assertions to handle version-specific behavior WHY: - Ensure tests pass on older tmux versions - Properly handle version-specific options - Improve test robustness across tmux versions
WHAT: - Add version check for update-environment option (added in tmux 3.0) - Update test assertions to handle version-specific behavior - Add type checking for update-environment value WHY: - Ensure tests pass on older tmux versions - Properly handle version-specific options - Improve test robustness across tmux versions
- Add command_error hook field (tmux 3.5+) - Guard pane-colours test with version check (tmux 3.3+) - Update terminal-overrides expectations for tmux 3.5 - Fix escape-time doctests to not rely on default value
Allow all matrix jobs to run even if one fails
Based on examination of tmux source code:
1. terminal-overrides defaults:
- tmux 3.0a-3.1b: Have default values (xterm*, screen*)
- tmux 3.2+: Defaults removed (commit 527f66ed, April 2020)
2. terminal-overrides parsing:
- Fix split("=") to split("=", 1) to handle values containing "="
- Example: "RGB=\E[...=%p1%d...]" was failing to parse
3. Style option behavior:
- When bg=default is set, tmux stores bg as color 8 (default)
- When converting to string, color 8 is omitted from output
- Test was incorrectly expecting "bg=default" in output
- This behavior is consistent across all tmux versions
Verification method:
- Created git worktrees for tmux 3.0a, 3.1b, 3.2a
- Examined options-table.c, style.c, colour.c in each version
- Tested terminal-overrides parsing with actual tmux commands
References:
- tmux-3.0a/options-table.c:219-223 (terminal-overrides defaults)
- tmux-3.2a/options-table.c:314-318 (empty defaults)
- tmux-3.0a/style.c:174-245 (style_tostring implementation)
- tmux-3.0a/style.c:231-235 (bg color 8 omitted)
Based on examination of tmux source code commit f03b6113 (May 2020):
In tmux 3.2, style options changed from OPTIONS_TABLE_STYLE to
OPTIONS_TABLE_STRING with OPTIONS_TABLE_IS_STYLE flag to support
format expansion in styles.
This changed how styles are stored and displayed:
**tmux ≤3.1 behavior:**
- Styles parsed and normalized when stored
- Output uses style_tostring() which normalizes:
- "bold" → "bright"
- bg=default (color 8) omitted from output
- Example: Input "fg=red,bg=default,bold" → Output "fg=red,bright"
**tmux ≥3.2 behavior:**
- Styles stored as literal strings (enables format expansion like #{...})
- Output shows the exact string that was set
- Example: Input "fg=red,bg=default,bold" → Output "fg=red,bg=default,bold"
Verification method:
- Created git worktrees for tmux 3.0a, 3.1b, 3.2a, 3.3a, 3.4, 3.5
- Examined source code changes across versions
- Tested directly with tmux commands to verify behavior
References:
- tmux commit f03b6113 (tmux 3.2 style type change)
- tmux-3.0a/options.c:126-127 (OPTIONS_IS_STYLE → style_tostring)
- tmux-3.0a/style.c:174-245 (style_tostring implementation)
- tmux-3.0a/attributes.c:26-50 (attributes_tostring: bold→bright)
tmux <3.2 normalizes 'bold' to 'bright' in style output. Added version guard to expect correct output format. Verification method: - Created git worktrees for tmux versions 3.0a, 3.1b, 3.2a, 3.3a, 3.4, 3.5 - Examined source code to trace style handling across versions - Tested directly with tmux commands to verify actual behavior In tmux ≤3.1, styles are parsed and normalized when stored: - attributes_fromstring() accepts both "bold" and "bright" as aliases - attributes_tostring() always outputs "bright" (never "bold") References: - tmux-3.0a/attributes.c:32-50 (attributes_fromstring table) - tmux-3.0a/attributes.c:26-28 (attributes_tostring output) - tmux-3.0a/options.c:126-127 (OPTIONS_IS_STYLE → style_tostring) - tmux commit f03b6113 (tmux 3.2 style format change)
Format expansion in style options (e.g., fg=#{...}) was added in
tmux 3.2 (commit f03b6113). Earlier versions reject styles with
format syntax as invalid.
Verification from tmux source code:
- Examined cmd-set-option.c in tmux 3.2 (commit f03b6113)
- Found validation logic: styles WITH #{...} skip validation
- Styles WITHOUT #{...} are still validated
- In tmux <3.2, ALL styles are validated, rejecting #{...} syntax
From commit f03b6113 message:
"Any styles without a '#{' are still validated when they are set
but any with a '#{' are not."
From cmd-set-option.c after f03b6113:
```c
if ((oe->flags & OPTIONS_TABLE_IS_STYLE) &&
strstr(value, "#{") == NULL &&
style_parse(&sy, &grid_default_cell, value) != 0) {
```
The test now only runs the format expansion check on tmux 3.2+.
References:
- tmux commit f03b6113 (style format expansion support)
- cmd-set-option.c validation logic with strstr check
Fixes test_wait_for_pane_content_exact_match_detailed failure on tmux 3.1b where capture_pane() was called immediately after send_keys() before the content had been flushed/rendered by tmux. The race condition manifested as: AssertionError: assert 'UNIQUE_TEST_STRING_123' in '' Adding a 0.1s delay after send_keys() allows tmux to flush the content before capture_pane() is called, fixing the timing issue on older versions.
why: Ensure libtmux constants cover latest tmux features for complete API support what: - Add ServerOptions: default_client_command, extended_keys_format - Add WindowOptions: tiled_layout_max_columns - Add PaneOptions: pane_scrollbars, pane_scrollbars_style - Add Hooks: pane_title_changed, client_light_theme, client_dark_theme
why: Ensure 100% API coverage for tmux options with proper typing validation what: - Add OptionTestCase NamedTuple for parametrized testing - Add 66 test cases covering all option scopes (Server, Session, Window, Pane) - Test integer, boolean, choice, string, and style option types - Handle boolean option conversion (on/off → bool) - Use safe test values for dangerous options (exit-empty, exit-unattached) - Support version-gated options with min_version parameter
why: Ensure 100% API coverage for tmux hooks with proper set/show/unset validation what: - Add HookTestCase NamedTuple for parametrized testing - Add 62 test cases covering all hook categories: - Alert hooks (3): activity, bell, silence - Client hooks (7): active, attached, detached, focus-in/out, resized, session-changed - Session hooks (3): created, closed, renamed - Window hooks (5): linked, renamed, resized, unlinked, session-window-changed - Pane hooks (6): died, exited, focus-in/out, mode-changed, set-clipboard - After-* hooks (38): all command completion hooks - Add version-gated tests for tmux 3.5+ hooks (pane-title-changed, client-light/dark-theme)
why: Document sparse array behavior for hooks and options handling what: - Add class-level doctests showing non-sequential index support - Add doctests to add(), append(), iter_values(), as_list() methods - Demonstrate dict-style access, index checking, sorted iteration
why: Document full pipeline from raw tmux output to typed Hooks dataclass what: - Add class-level doctests showing hook parsing and access patterns - Add from_stdout() doctests with parameter docs and examples - Demonstrate sparse index preservation and multiple hook types
why: Document hook-style usage of explode_arrays with SparseArray output what: - Add doctest example showing force_array=True for hooks output - Demonstrate SparseArray type and index preservation
why: Enable easier management of multiple indexed hooks at once what: - Add get_hook_indices() to get sorted list of existing indices - Add get_hook_values() to get all values as SparseArray - Add set_hooks_bulk() to set multiple hooks from dict/list/SparseArray - Add clear_hook() to unset all indexed values for a hook - Add append_hook() to append at next available index - Add module docstring with version compatibility notes (3.0-3.5+) - All methods support method chaining via Self return type
why: Ensure bulk operations API works correctly with parametrized tests what: - Add BulkOpTestCase NamedTuple with test_id, operation, setup, expected - Add categorized test cases: GET_INDICES, GET_VALUES, SET_BULK, CLEAR, APPEND - Create parametrized test_bulk_hook_operation() for 12 data-driven tests - Add standalone tests for iteration, SparseArray input, method chaining
why: CI fails on tmux 3.0a because some hooks/options don't exist in that version what: - client-focus-in, client-focus-out, client-active: require tmux 3.3+ - window-resized: requires tmux 3.3+ - editor option: requires tmux 3.2+ - prompt-history-limit option: requires tmux 3.3+
why: CI fails on tmux 3.2a, 3.3a, and 3.5 due to incorrect version requirements what: - scroll-on-clear: change min_version from 3.2 to 3.3 (added in 3.3) - allow-passthrough "all" value: change min_version from 3.3 to 3.4 (added in 3.4) - Remove client-light-theme and client-dark-theme tests (not released yet, will be in 3.6+)
why: Commented code in docstrings violates project convention and provides no documentation value since it won't run or be tested what: - get_hook_indices(): add full doctest with setup/teardown - get_hook_values(): add doctest verifying SparseArray behavior - set_hooks_bulk(): add doctests for dict, list, and clear_existing modes - clear_hook(): add doctest demonstrating hook removal - append_hook(): add doctest showing sequential index addition All doctests verified to pass with pytest --doctest-modules
why: test_wait_for_any_content_exact_match was flaky due to race
condition - pane content captured before terminal finished redrawing
after clear command.
what:
- Add wait_until_pane_ready() after send_keys("clear") to ensure
shell prompt is ready before capturing content
- Uses idiomatic waiter pattern instead of arbitrary time.sleep()
why: Clean up test code by removing unused/impossible code paths. what: - Remove unused xfail_reason from pane_title_changed (test uses pytest.skip() before xfail can trigger) - Remove dead assertion block in test_options.py that contradicted the preceding assert (lines 445-456)
why: HooksMixin feature was built but not documented in CHANGES. what: - Add HooksMixin section under "New features" - Document hook methods: set_hook, show_hook, unset_hook - Document bulk operations: get_hook_indices, get_hook_values, set_hooks_bulk, clear_hook, append_hook - Note HooksMixin availability on Server, Session, Window, Pane
why: SparseArray internal API had no documentation page. what: - Create docs/internals/sparse_array.md with automodule directive - Add sparse_array to docs/internals/index.md toctree
345915f to
226ddb2
Compare
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.
Extracted from #513
Warning
APIs below are subject to change (both params, return types, and structures)
Changes
New internal:
OptionsMixinshow_options(): High level objects, e.g. preserving sparse-array data such ascommand-alias[1],command-alias[99], and inside of a dataclass object.command_alias:SparseArray({0: {'split-pane': 'split-window'})_show_options(): Map of options split up by key, with raw values.command-alias[1] split-pane=split-window_show_options_raw(): Raw stdout fromtmux show-optionsIndividual options:
show_option(): High level objects, e.g. preserving sparse-array data such ascommand-alias[1],command-alias[99]Not sure if this will scale, but will try to get this:
_show_option():_show_option_raw(): Raw stdout fromtmux show-option [option name]New internal:
HooksMixinNew features
Window.set_option()Learned params:
format->-Funset->-uglobal->-gunset_panes->-U: Also unset other panse in windowsprevent_overwrite:-osuppress_warnings:-qappend:-aWindow.show_option()Window.show_options()Breaking changes
Deprecations
Window.set_window_option()in favor ofWindow.set_option()Window.show_window_option()in favor ofWindow.show_option()Window.show_window_options()in favor ofWindow.show_options()Summary by Sourcery
Refactor option and hook management, deprecating old methods.
New Features:
OptionsMixinto manage tmux options.HooksMixinto manage tmux hooks.show_option(),show_options(), andset_option().run_hook(),set_hook(),show_hooks(),show_hook(), andunset_hook().SparseArrayto handle sparse arrays in options and hooks.Tests: