vcspull add - More corner cases #484
Merged
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
why: Path-first adds from inside a workspace still wrote './' sections even after adopting tilde labels. what: - compute a preferred workspace label and migrate existing './' entries to the tilde label, keeping config_items in sync
why: Guard against regressions when runs from inside the workspace with and without . what: - add tests confirming tilde-labelled workspaces and './' upgrades
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## master #484 +/- ## ========================================== - Coverage 79.09% 77.84% -1.26% ========================================== Files 13 13 Lines 1789 1882 +93 Branches 382 398 +16 ========================================== + Hits 1415 1465 +50 - Misses 235 271 +36 - Partials 139 146 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
why: The path is None heuristic dates back to the deprecated name+URL flow; path-first adds should only create ./ when explicitly requested. what: - Remove workspace_root_path is None branch that implied path is None - Set preserve_cwd_label directly from explicit_dot flag - Fix variable redefinition in _prepare_no_merge_items refs: Legacy name+URL flow deprecation
…th-first adds why: Path-first adds should derive workspace from repo parent directory, and handle home directory edge case. what: - _resolve_workspace_path returns repo.parent when workspace_root is None - workspace_root_label handles workspace_path == home to return ~/ - Prevents ~/./ labels when workspace is the home directory refs: Path-first workflow correctness
why: Legacy name+URL flow tests expected ./ labels; path-first workflow uses tilde labels. what: - Update test fixtures to provide repo paths instead of path=None - Update expectations: ~/ instead of ./, ~/code/ for parent directories - Fix workspace label expectations to match parent directory behavior - Remove normalization expectations for path-first adds refs: Path-first workflow migration
why: Path-first adds should not auto-migrate existing ./ sections; explicit --workspace ./ is the only way to create/target ./ labels. what: - Remove migration logic from _ensure_workspace_label_for_merge - Remove migration logic from _prepare_no_merge_items - Existing ./ sections now preserved when they match the target workspace refs: Simplify legacy heuristics
why: Automatic ./ migration removed; test should verify ./ sections are preserved. what: - Rename test_handle_add_command_upgrades_dot_workspace_section - Update test to verify existing ./ sections are preserved - Change assertions to expect ./ label when matching existing section refs: Remove ./ migration behavior
…st adds why: normalize_workspace_roots was only needed for legacy name+URL flow cleanup; path-first adds derive correct labels directly. what: - Remove normalize_workspace_roots call from merge path - Remove in-place label normalization from no-merge path - Remove unused normalize_workspace_roots import (ruff auto-fix) - Simplify config_was_relabelled tracking refs: Eliminate legacy normalization machinery
why: Refactoring had no user-visible behavior changes; update test snapshots for new explicit ./ workspace tests. what: - Remove CHANGES entry about internal legacy heuristics cleanup - Add snapshots for path-explicit-dot-workspace tests refs: Internal cleanup
tony force-pushed the vcspull-add-bug-pt-4 branch from dbd97bc to e285b82 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.
Earlier:
vcspull add- even more bug fixes #483vcspull add: Fix more issues #482vcspull add: Usability overhaul and tests #481vcspull add- Refactoring and fixes #480Changes
Refactor: cli/add path-first handling trims legacy heuristics
why: Stop
vcspull addfrom inventing a./workspace when the caller suppliesonly a repository path, and keep duplicate sections intact without sidecar
structures. what:
honoring
./when the user passes--workspace ./--no-mergeflow onto ordered loader items so duplicate sectionsround-trip without config_items bookkeeping
modes to guard the new behavior
Testing
python -m pytest tests/cli/test_add.pyuv run py.test tests/test_cli.py(fails: offline sandbox cannot reachhttps://pypi.org/simple/hatchling/ to build the project)