Repository navigation
Import scripts with one base name; placeholder warnings for declared keys - #73
Merged
Merged
Conversation
readEmitted returns one part per file; the scripts/hooks loop now groups them by the ingredient name (lowercased base name) and merges with a shared mergeEmitted helper. drift-promote also uses this helper.
One script/hook ingredient per base name; old Forges keep working, a re-import updates the variant with both files.
Also add proper assertions in test 7 (S3), test 9 for grouped script rejection location (S1), and unit tests for mergeEmitted (N5).
Add fileWhere helper for the three-way path logic (skill dir layout, grouped script/hook, other). Use path.posix.dirname instead of split/slice/join. Copy merged.warnings into report.warnings for the script/hook loop (N1).
Warn when the key is declared by ANY resolved ingredient (with or without a default), not just when declared without a default. A default reaches only its own ingredient's files.
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
craftar@0.34.2to npm after approval in thenpmenvironment.importgroups scripts and hooks that share a base name (compared without case:run-x.ps1+run-x.sh,X.PS1+x.sh) into one ingredient whosefileslists every file. Before, the second file became a bogus same-profile variant<name>--<profile>withas: <name>. Two files whose names differ only in case are rejected (that ingredient only; they would collide on a case-insensitive checkout). A file left in a script or hook folder that import writes, and not in itsfiles, is warned. A secret or marker rejection in a grouped script names the file that holds it.mergeEmittedis the one merge, shared withdrift promote.{{key}}is warned only when some ingredient declares the key underparams(with or without a default), or when it is underintegrations.at Forge schema 3 or above. Any other{{…}}, such as an Angular template, is left verbatim silently.Plan.missingParamsand the sync refusal for a declared key with no value are unchanged.--jsonwarnings arrays can be shorter (no key changes), anddoctor --strictcan exit 0 where it exited 1.Test plan
npm run typecheck: passnpm run build: passnpx vitest run --exclude test/ci.test.ts --maxWorkers=6: 3414 passed, 11 skipped (141 files), at e5e87fctest/import-script-groups.test.tsandtest/param-warning-declared.test.ts, each shown failing before the fix. Mutation runs provemergeEmitted, theintegrations.exception and the same-run staging overlay (now tested with a pair of skills).