fix(template): put the shebang on line 1 — at the generator, not in 20 outputs - #26
Conversation
…0 outputs
Every launcher this generator emits begins with a BLANK LINE, then the shebang.
That is shellcheck SC2148 and a script the kernel will not dispatch by shebang.
Verified on origin across four repos (berrywiki, stapeln, gossamer, paint-type):
line 1 of each `<repo>-launcher.sh` is empty.
Cause: `templates/launcher.sh.tera` opened with a Tera comment block and only
then the shebang, so the rendered output carried a leading newline.
WHY THIS BELONGS HERE. The estate currently has ~20 open one-line PRs, each
deleting that blank line in one generated launcher. Codacy said so on nearly
every one of them - "the file is explicitly marked as auto-generated by
launch-scaffolder; manual edits will be lost during the next
`launch-scaffolder realign`" - and stapeln#38 quantified the gap: 20 files
identified, 1 fixed. Merging twenty downstream fixes leaves the machine that
emits the defect untouched, and the next realign overwrites all of them.
THE FIRST ATTEMPT AT THIS FIX WAS WRONG, AND THE TEST CAUGHT IT. Moving the
shebang above a `{#- ... -#}` block rendered as:
#!/usr/bin/env bash# SPDX-License-Identifier: MPL-2.0
- concatenated, because `{#-` strips whitespace BEFORE the tag and ate the
newline after the shebang. A test asserting only on the template SOURCE would
have passed that happily and shipped a worse file than the one it replaced.
The correct pairing is `{#` (do not strip before) with `-#}` (strip after).
Two regression tests, because the property that matters is the RENDERED output:
* template_source_opens_with_shebang - the template invariant
* rendered_launcher_starts_with_shebang_on_line_one - renders through Tera
and asserts line 1, which is the assertion that caught the bad fix
Also repairs a pre-existing failure that was red on origin/main before this
branch: standard::tests::baked_standard_parses asserted spec_version == "0.1.0"
against a standard declaring 0.2.0. It failed for a reason unrelated to parsing,
which is what it is named for. The version is now DERIVED from the baked source
rather than hardcoded, so it cannot go stale again.
Full workspace suite: 19 passed, 0 failed (baseline on origin/main was 16
passed, 1 failed).
Follow-up, not done here: the ~20 already-generated launchers still carry the
blank line until a `realign` regenerates them. The open per-repo PRs remain
valid as immediate relief; this stops the defect being reintroduced.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe launcher template now emits ChangesLauncher validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change fixes generated launcher shebang placement and prevents the baked standard version assertion from drifting; no actionable merge-blocking risk remains. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
The implementation successfully addresses the requirement to ensure the shebang is correctly positioned on the first line of generated scripts by adjusting the Tera template and whitespace configuration. The test suite has been updated to dynamically verify the specification version, reducing the risk of regression. No security flaws or major logic bugs were identified that should prevent the PR from being merged. The overall quality metrics are within project standards, though there is one maintainability finding regarding function length in the template logic.
1 comment outside of the diff
crates/launcher-common/src/template.rs
line 27🟡 MEDIUM RISK
Therenderfunction has grown to 99 lines, primarily due to manual Tera context population. Splitting this into logical sub-sections (e.g., project, runtime, and metadata) would improve readability and bring the function within project complexity limits.Try running the following prompt in your IDE agent:
Refactor the
renderfunction incrates/launcher-common/src/template.rsto reduce its length. Extract the context insertion logic for 'project', 'runtime', and 'metadata' into separate private helper functions that take&mut Contextand the relevant configuration structs as arguments.
Test suggestions
- Verify that the template source code starts with the shebang string.
- Verify that the rendered output of the template starts with the shebang on line 1 followed by a newline.
- Verify that the baked standard parses correctly and the spec_version matches the version declared in the BAKED_STANDARD source.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
1 participant
The defect
Every launcher this generator emits begins with a blank line, then the shebang — shellcheck SC2148, and a script the kernel will not dispatch by shebang.
Verified on origin across four repos (
berrywiki,stapeln,gossamer,paint-type): line 1 of each<repo>-launcher.shis empty.Why here rather than in the twenty PRs
The estate has ~20 open one-line PRs each deleting that blank line in one generated launcher. Codacy said so on nearly every one:
and
stapeln#38quantified it: 20 files identified, 1 fixed. Merging twenty downstream fixes leaves the machine that emits the defect untouched.The first attempt was wrong, and the test caught it
Moving the shebang above a
{#- … -#}block rendered as:— concatenated, because
{#-strips whitespace before the tag and ate the newline after the shebang. A test asserting only on the template source would have passed that and shipped something worse than the original. Correct pairing is{#(don't strip before) with-#}(strip after).Tests
template_source_opens_with_shebangrendered_launcher_starts_with_shebang_on_line_oneAlso repaired
standard::tests::baked_standard_parseswas already red onorigin/main: it assertedspec_version == "0.1.0"against a standard declaring0.2.0, failing for a reason unrelated to parsing. The version is now derived from the baked source, so it cannot go stale again.Full workspace suite: 19 passed, 0 failed (baseline on
origin/main: 16 passed, 1 failed).Follow-up
The ~20 already-generated launchers keep the blank line until a
realignregenerates them. The open per-repo PRs stay valid as immediate relief; this stops reintroduction.🤖 Generated with Claude Code