-
Notifications
You must be signed in to change notification settings - Fork 78
fix: missing Assembly section for instructions with no variables #1212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Remove conditional check that hides Assembly section when assembly field is empty. Now instructions like fence.i will properly show "Assembly:: fence.i" in the instruction appendix instead of omitting the section entirely. Co-authored-by: AFOliveira <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix is quite accurate, I'm just not sure if this will show "" instead of emptyness after the instruction name
…templates - Remove conditional check in instructions_appendix that was hiding Assembly section - Add .strip to all assembly concatenations to remove trailing whitespace - Ensures instructions with no variables (like fence.i) properly show Assembly section Co-authored-by: AFOliveira <[email protected]>
|
@jordancarlin Would you be willing to test what our Copilot friend produced? I'm very low on time. I think the first commit may be enough, though |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1212 +/- ##
=======================================
Coverage 46.05% 46.05%
=======================================
Files 11 11
Lines 4942 4942
Branches 1345 1345
=======================================
Hits 2276 2276
Misses 2666 2666
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix the failing CI test
|
Do you know what triggered this? |
|
@dhower-qc I'm playing with CoPilot :) |
|
nifty |
We should be able to tell if it worked correctly based on how the golden instruction appendix changes in the GitHub diff (though it looks like that wasn’t updated here) without having to test anything locally. |
Instructions with
assembly: ""(e.g.,fence.i,ebreak,wfi) were missing the Assembly section in the generated instruction appendix due to a conditional check that hid the section entirely when the assembly field was empty.Changes
backends/instructions_appendix/templates/instructions.adoc.erb: Removed conditional that hid Assembly section wheninst.assemblywas empty.stripto assembly string concatenations to remove trailing whitespaceResult
Instructions with no variables now display
Assembly:: <instruction_name>instead of omitting the section entirely.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.