-
Notifications
You must be signed in to change notification settings - Fork 30
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
[nfc] Add ability to test code snippets #172
Conversation
28065f7
to
a016c82
Compare
a016c82
to
9e363b2
Compare
e1cccfa
to
60c018c
Compare
This now includes basic delimited support. The magic delimiters
This will extract the entire code block to a file. It will replace the original code block with:
|
b9461ff
to
2191922
Compare
Just to be sure, the line comment syntax is |
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.
could we add a few sentences to CONTRIBUTING.md that explains this a little bit?
Yes! That's a mistake. The filter will still handle that, but that is illegal FIRRTL. The intent is that the code should put the |
4ae925e
to
c869e65
Compare
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 is good for a final review. I will open a second PR which fixes all the code examples. We can then turn this on in CI.
|
||
local f = io.open(filename, 'w') | ||
f:write(elem.text) | ||
f:write("\n") |
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 newline helps for readability and avoids a Verilator lint warning that we don't have to waive.
scripts/extract-firrtl-code.lua
Outdated
systemverilog = "sv" | ||
} | ||
local ext = extMap[elem.classes[1]] | ||
local skip = extMap[elem.classes[2]] == notest |
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 allows opting out from testing for a specific snippet using syntax like {.firrtl .notest}
on the code block. This is very helpful for things that are really annoying to test like Verilog that includes input files or FIRRTL code blocks that are intentionally not legal through the use of pseudo-type parameters (see memories).
9dc8b20
to
cf436d0
Compare
@mwachs5 wrote:
Added in the force push! |
9568f78
to
4629954
Compare
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 is my first exposure to pandoc filters, so I don't have any strong comments on the lua implementation. The usage and goal makes sense to me in general.
This demonstrates a flow that can be used to validate that all code snippets used in the FIRRTL specification correctly parse. This is implemented using a simple pandoc filter to extract the bodies of all code blocks with known languages (FIRRTL and Verilog), write them to the build directory, and then run them all through an appropriate tool based on file extension (firtool or verilator). This may be a little cumbersome as every code example has to be completely legal. To alleviate this, this PR includes the ability to add special delimiters, indicated by lines with the text `snippetbegin`/`snippetend` which will cause the filter to extract only the lines between the delimiters. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
4629954
to
fcbd3df
Compare
This demonstrates a flow that can be used to validate that all code
snippets used in the FIRRTL specification correctly parse. This is
implemented using a simple pandoc filter to extract the bodies of all code
blocks with known languages (FIRRTL and Verilog), write them to the build
directory, and then run them all through an appropriate tool based on file
extension (firtool or verilator).
This may be a little cumbersome as every code example has to be completely
legal. To alleviate this, this PR includes the ability to add special
delimiters, indicated by lines with the text
snippetbegin
/snippetend
whichwill cause the filter to extract only the lines between the delimiters.
CC: @dtzSiFive