Fix per-package target setting not being used#57
Merged
Conversation
This test demonstrates the bug where per-package target settings are ignored. The test will fail with the current code, showing: Expected: custom-dir/package.with.custom.target Actual: ./sources/package.with.custom.target The bug is in config.py line 103, which uses the default-target variable instead of the package's individual target setting. Related to #53
The bug was in config.py line 103, which used the 'target' variable
(always equal to default-target) instead of package["target"]
(which may contain a custom per-package target setting).
This fix ensures that:
- Packages with custom target settings are checked out to their
specified directory
- Packages without custom target continue to use default-target
- Variable interpolation like ${settings:docs-directory} works correctly
The previously failing test now passes, demonstrating the fix works.
Fixes #53
Use pathlib.Path().as_posix() for cross-platform path comparison. This handles both Unix (/) and Windows (\) path separators correctly by normalizing both sides of the comparison. Also update CLAUDE.md to document preference for pathlib over os.path for path operations in tests and new code.
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
Fixes #53: Per-package
targetsetting is now correctly used when constructing checkout pathsThis PR fixes the bug where individual package
targetsettings were ignored when determining the checkout path. The issue was thatconfig.pyline 103 was using thedefault-targetvariable instead of the package's individualtargetsetting.Changes
1. Failing Test (Commit ef5b4d3)
Added
test_per_package_target_override()that demonstrates the bug:${settings:docs-directory})2. The Fix (Commit 733ecf2)
File:
src/mxdev/config.pyline 104Changed from:
To:
This ensures the path uses the package's actual target (either custom or default) instead of always using the default-target.
3. Cross-Platform Test Fix (Commit 5800927)
Updated test to use
pathlib.Path().as_posix()for path comparison, which handles both Unix (/) and Windows (\) path separators correctly.4. Documentation (Commits 733ecf2 + 5800927)
CHANGES.mdwith fix entryCLAUDE.mdcode style guidelinesResults
✅ All 24 tests pass (14 config + 10 other test files)
✅ All CI checks pass on Ubuntu, Windows, macOS (Python 3.8-3.14)
✅ Packages with custom
targetsettings are now checked out to correct directories✅ Packages without custom
targetcontinue to usedefault-target✅ Variable interpolation like
${settings:docs-directory}works correctlyTest Plan
Example Usage