-
Notifications
You must be signed in to change notification settings - Fork 111
types: Add StrPath
typing, fix new_session
, part 2
#598
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
I'm making mistakes, but follow up to #597, #596
Summary
This PR adds uniform
StrPath
type support forstart_directory
parameters 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 | None
Session.new_window
:start_directory: str | None
→start_directory: StrPath | None
Pane.split
andPane.split_window
:start_directory: str | None
→start_directory: StrPath | None
Window.split
andWindow.split_window
:start_directory: str | None
→start_directory: StrPath | None
Implementation Details
StrPath
import 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.Path
objectsFiles Modified
src/libtmux/server.py
src/libtmux/session.py
src/libtmux/pane.py
src/libtmux/window.py
tests/test_server.py
tests/test_session.py
tests/test_pane.py
tests/test_window.py
Benefits
pathlib.Path
objects directly without converting to stringsstart_directory
parameters 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: