fix: support multi-path scope values in [_.codebase.scope]#56
Open
olavostauros wants to merge 1 commit into
Open
fix: support multi-path scope values in [_.codebase.scope]#56olavostauros wants to merge 1 commit into
olavostauros wants to merge 1 commit into
Conversation
The codebase_target_for_rule() function treated the entire scope value as a single path, so a TOML string like or-true = ".mise/tasks lib" would produce a single target /repo/.mise/tasks lib (with space) instead of two separate targets /repo/.mise/tasks and /repo/lib. This meant multi-path scope overrides either errored (path doesn't exist) or silently scanned nothing — users thought they had multi-path coverage but got false passes. Changes: - lib/codebase-config.sh: Replace codebase_target_for_rule() with codebase_targets_for_rule() that splits the scope value on whitespace and emits one target line per token. - .mise/tasks/lint/_default: Iterate over the multi-line output so each target is linted independently. - test/lint/default.bats: Add test covering multi-path scope with space-separated path list. The fix uses shell word-splitting on the scope string (unquoted in a for loop), which naturally handles TOML string values with internal spaces. Each token is resolved as a relative path against the repo root, or passed through unchanged if absolute. Fixes KnickKnackLabs#22
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 the
_.codebase.scopeTOML configuration parser so that multi-path scope values (e.g.,or-true = ".mise/tasks lib") expand to separate targets instead of being corrupted into a single garbage path.Previously,
codebase_target_for_rule()emitted the entire scope value as one string, producing a target like/repo/.mise/tasks lib(with an embedded space). This either errored (path doesn't exist) or silently scanned nothing — giving users false confidence that they had multi-path coverage.Changes
lib/codebase-config.shcodebase_target_for_rulewithcodebase_targets_for_rule. Splits the scope value on whitespace via shell word-splitting (unquoted$scopein aforloop), emitting one target line per token. Each token is resolved as a relative path against$repo_root, or passed through if absolute..mise/tasks/lint/_defaulttest/lint/default.batslint: honors multi-path scope (space-separated targets)test.Design decisions
$scope— TOML string values like".mise/tasks lib"return.mise/tasks libfrommise config get(quotes stripped, space preserved). Shell word-splitting naturally splits this into.mise/tasksandlib.gum-table = "scripts") emit one line as before. The dot/empty case still maps to the repo root directly.pre-commithook — The hook delegates tocodebase lint, which now handles the iteration internally.Validation
mise run test— all 219 tests passcodebase lint .— no new violations introduced[_.codebase.scope]parser strips internal spaces, corrupting multi-path scopes #22 produces correct multi-path expansionFixes #22