🧹 Extract inline awk script to improve idstack-gen-skills readability#49
🧹 Extract inline awk script to improve idstack-gen-skills readability#49savvides wants to merge 1 commit into
Conversation
Extract the long `awk` script from `render_template()` in `bin/idstack-gen-skills` to a separate standalone file (`bin/render-skill.awk`) and use the `-f` flag to include it. This significantly improves maintainability and readability of `idstack-gen-skills`. Co-authored-by: savvides <1580637+savvides@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request refactors the bin/idstack-gen-skills script by extracting its inline awk template rendering logic into a dedicated file, bin/render-skill.awk. The review feedback recommends removing an unused variable (after_frontmatter) from the BEGIN block and adding robust error handling for getline operations to prevent silent failures when reading external files.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| BEGIN { | ||
| after_frontmatter = 0 | ||
| frontmatter_count = 0 | ||
| in_allowed_tools = 0 | ||
| } |
| /\{\{PREAMBLE\}\}/ { | ||
| while ((getline line < preamble_file) > 0) print line | ||
| close(preamble_file) | ||
| next | ||
| } | ||
| /\{\{MANIFEST_SCHEMA\}\}/ { | ||
| while ((getline line < schema_file) > 0) print line | ||
| close(schema_file) | ||
| next | ||
| } |
There was a problem hiding this comment.
If getline encounters an error (e.g., if the file is unreadable or permission is denied), it returns -1. Currently, this silent failure is unhandled, which would result in the placeholder being silently removed without inserting the preamble or schema content. Checking the return value of getline and exiting on error prevents silent failures and ensures generation robustness.
/\{\{PREAMBLE\}\}/ {
while ((status = (getline line < preamble_file)) > 0) {
print line
}
if (status < 0) {
print "ERROR: failed to read " preamble_file > "/dev/stderr"
exit 1
}
close(preamble_file)
next
}
/\{\{MANIFEST_SCHEMA\}\}/ {
while ((status = (getline line < schema_file)) > 0) {
print line
}
if (status < 0) {
print "ERROR: failed to read " schema_file > "/dev/stderr"
exit 1
}
close(schema_file)
next
}
🎯 What: Extracted a large inline awk script from
bin/idstack-gen-skillsinto its own filebin/render-skill.awk.💡 Why: It simplifies and improves the readability of
bin/idstack-gen-skills, addressing a code health issue where an overly long functionrender_templatewas heavily bloated by a complex inlineawkscript string.✅ Verification: Verified the logic change by running the complete set of tests (
./test/smoke-test.shand./test/integration-test.sh). The CLI dry-run also verifies nothing changes during rendering.✨ Result: The logic is successfully decoupled into a standalone file without breaking behavior or changing standard usage patterns.
PR created automatically by Jules for task 4205765716975549657 started by @savvides