cambridgehackers / connectal

Connectal is a framework for software-driven hardware development.
MIT License
161 stars 46 forks source link

bsvpreprocess.py hits AssertionError if backticks used in block comment #142

Closed acw1251 closed 7 years ago

acw1251 commented 7 years ago

I've started auto-generating documentation by parsing bsv files, and one of my conventions is causing bsvpreprocess.py to hit an AssertionError when preprocessing some files.

The issue comes from having backticks (`) in comment blocks (/* ... */) like this: https://github.com/csail-csg/recycle-bsv-lib/blob/master/src/bsv/PrintTrace.bsv#L36

(In case you are curious, the generated documentation for this file is here: https://github.com/csail-csg/recycle-bsv-lib/blob/master/doc/markdown/PrintTrace.md)

I think this can be fixed by either completely ignoring comment blocks (maybe around here: https://github.com/cambridgehackers/connectal/blob/master/scripts/bsvpreprocess.py#L55) or by adding block comments to the comment variable instead of the noncomment variable.

jameyhicks commented 7 years ago

I think comment blocks should go in the comment variable, although I have not looked at the code in a while.

You're using markdown for the documentation? Could I convince you to use sphinx instead? It's not handled rendered directly by github, but we could run a travis job to build the documentation and publish it.

hanw commented 7 years ago

For markdown, I recently found out a nice tool called madoko that can generate html, pdf, tex all from a markdown file. (developed by MSR)

https://www.madoko.net/ https://www.npmjs.com/package/madoko

jameyhicks commented 7 years ago

But does it index and cross reference BSV types, modules, interfaces, methods? sphinx does that and generates html, pdf, tex, epub, ...

On Fri, Mar 24, 2017 at 4:07 PM Han Wang notifications@github.com wrote:

For markdown, I recently found out a nice tool called madoko that can generate html, pdf, tex all from a markdown file. (developed by MSR)

https://www.madoko.net/ https://www.npmjs.com/package/madoko

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/cambridgehackers/connectal/issues/142#issuecomment-289130265, or mute the thread https://github.com/notifications/unsubscribe-auth/ACU3s-yxXetxsrt4Nzcso2lbL9eDwuW2ks5rpCJqgaJpZM4Mosa2 .

jameyhicks commented 7 years ago

I don't have that much documentation, so I'm happy to change formats but I do not want to lose functionality.

On Fri, Mar 24, 2017 at 4:17 PM Jamey Hicks jamey.hicks@gmail.com wrote:

But does it index and cross reference BSV types, modules, interfaces, methods? sphinx does that and generates html, pdf, tex, epub, ...

On Fri, Mar 24, 2017 at 4:07 PM Han Wang notifications@github.com wrote:

For markdown, I recently found out a nice tool called madoko that can generate html, pdf, tex all from a markdown file. (developed by MSR)

https://www.madoko.net/ https://www.npmjs.com/package/madoko

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/cambridgehackers/connectal/issues/142#issuecomment-289130265, or mute the thread https://github.com/notifications/unsubscribe-auth/ACU3s-yxXetxsrt4Nzcso2lbL9eDwuW2ks5rpCJqgaJpZM4Mosa2 .

hanw commented 7 years ago

It only takes markdown as input files, and it is more suitable for writing documents, instead of generating documentation. It's better to stay with sphinx in this case.

acw1251 commented 7 years ago

I'm using markdown for now because Github will render it for you (including BSV syntax highlighting). Eventually it would be nice to have documentation that doesn't require browsing through Github, so I'll look at sphinx and madoko for future use. Even if I choose to stay with markdown, it shouldn't be hard to make a sphinx output for the documentation generation script.

I added the line

source = re.sub(re.compile(r"/\*.*?\*/", re.MULTILINE | re.DOTALL), "", source)

just before this line to strip out block comments, and compilation works again. I'll try adding block comments to the comment variable now.

If bsvpreprocess.py is just used for preprocessing the file, you can use the -E flag in bsc to do that. It adds `line statements to mark which file the lines come from.

hanw commented 7 years ago

This may be a dumb question, probably should asked earlier. But why do we need to write a preprocess script? Is the bsc -E preprocessing not sufficient?

hanw commented 7 years ago

oops, didn't read andy's reply in full. =P

acw1251 commented 7 years ago

I added the workaround on the csail-csg fork so the riscy processors could compile again. It's not optimal, but it works for now.

jameyhicks commented 7 years ago

I pushed a change to bsvpreprocess to handle / / comments. Does it help?

On Fri, Mar 24, 2017 at 4:57 PM Andy Wright notifications@github.com wrote:

I added the workaround on the csail-csg fork so the riscy processors could compile again. It's not optimal, but it works for now.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/cambridgehackers/connectal/issues/142#issuecomment-289141110, or mute the thread https://github.com/notifications/unsubscribe-auth/ACU3s9X5TFshQzC0N7Lt1eiGwg1Axuduks5rpC5PgaJpZM4Mosa2 .

acw1251 commented 7 years ago

I still have the problem, but a small fix for multiline block comments fixes it. The fix is in PR #143

acw1251 commented 7 years ago

This is all fixed now from PR #143

jameyhicks commented 7 years ago

yay!