types: Add StrPath typing, fix new_session, part 2#598
Merged
Conversation
Reviewer's GuideThis PR refines the internal type documentation by focusing solely on the new StrPath alias and simplifies the session startup logic by dropping the explicit string type check in favor of a truthiness check, allowing broader path-like inputs to be handled by pathlib. Updated class diagram for the Session classclassDiagram
class Session {
+new_window(..., start_directory: StrPath | None, ...): Window
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #598 +/- ##
==========================================
+ Coverage 79.84% 80.30% +0.45%
==========================================
Files 23 23
Lines 1915 1919 +4
Branches 294 294
==========================================
+ Hits 1529 1541 +12
+ Misses 266 261 -5
+ Partials 120 117 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey @tony - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
why: Standardize path handling to accept both str and PathLike objects what: - Import StrPath from libtmux._internal.types - Update start_directory parameter type annotation to StrPath | None - Update docstring to reflect str or PathLike support - Maintain existing if start_directory: logic for proper command building
…rectory why: Standardize path handling to accept both str and PathLike objects what: - Import StrPath from libtmux._internal.types - Update start_directory parameter type annotations to StrPath | None - Update docstrings to reflect str or PathLike support - Fix logic pattern from 'is not None' to truthy check for proper command building - Clean up import organization
…directory why: Standardize path handling to accept both str and PathLike objects what: - Import StrPath from libtmux._internal.types - Update start_directory parameter type annotations to StrPath | None - Update docstring to reflect str or PathLike support
what: - Introduced StartDirectoryTestFixture for parameter testing - Added tests for various start_directory scenarios including None, empty string, absolute path, and pathlib.Path - Updated test_pane_context_manager to verify pane count after context exit - Added test for pathlib.Path acceptance in Pane.split
what: - Added StartDirectoryTestFixture for comprehensive testing of start_directory scenarios - Implemented tests for None, empty string, absolute path, and pathlib.Path inputs - Verified correct command generation based on start_directory values - Ensured compatibility with pathlib.Path in new_session method
what: - Added comprehensive tests for `start_directory` parameter handling in `Session.new_window` - Included scenarios for None, empty string, absolute path, and pathlib.Path - Verified correct command generation and window creation based on `start_directory` values - Ensured compatibility with pathlib.Path for start_directory input
what: - Added tests for start_directory parameter handling in Window.split - Included scenarios for None, empty string, absolute path, and pathlib.Path - Verified correct pane creation and command generation based on start_directory values - Ensured compatibility with pathlib.Path for start_directory input
what: - Removed expected_in_cmd and expected_not_in_cmd from StartDirectoryTestFixture. - Updated test cases to handle user_path and relative_path scenarios. - Enhanced logic for formatting start_directory with actual paths in tests. - Ensured compatibility with pathlib.Path for start_directory input across tests in test_pane.py, test_server.py, test_session.py, and test_window.py.
what: - Added monkeypatching to change the current working directory to tmp_path in test cases. - Simplified assertions for current path checks by removing unnecessary conditional statements. - Ensured consistency in handling expected paths across test files: test_pane.py, test_server.py, test_session.py, and test_window.py.
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
1 participant
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.
I'm making mistakes, but follow up to #597, #596
Summary
This PR adds uniform
StrPathtype support forstart_directoryparameters across all methods in libtmux, enabling PathLike objects (likepathlib.Path) to be used alongside strings.Changes Made
Type Annotations Updated
Server.new_session:start_directory: str | None→start_directory: StrPath | NoneSession.new_window:start_directory: str | None→start_directory: StrPath | NonePane.splitandPane.split_window:start_directory: str | None→start_directory: StrPath | NoneWindow.splitandWindow.split_window:start_directory: str | None→start_directory: StrPath | NoneImplementation Details
StrPathimport to all affected modulesif start_directory:(notif start_directory is not None:) to properly handle empty stringspathlib.Path(start_directory).expanduser()for proper tilde expansionTesting
None, empty string, absolute path string, andpathlib.PathobjectsFiles Modified
src/libtmux/server.pysrc/libtmux/session.pysrc/libtmux/pane.pysrc/libtmux/window.pytests/test_server.pytests/test_session.pytests/test_pane.pytests/test_window.pyBenefits
pathlib.Pathobjects directly without converting to stringsstart_directoryparameters in the codebaseTesting Results
Fixes typing inconsistencies and enhances usability for path-related parameters throughout libtmux.
Summary by Sourcery
Update StrPath documentation and enhance Session.new_window to better handle start_directory values
Enhancements: