capDL-tool: add --json mode - #90
Conversation
6187580 to
6cfa62a
Compare
lsf37
left a comment
There was a problem hiding this comment.
This looks good from my side, sorry it took so long to get to a review. @nspin can you please add a commit message to pacify gitlint?
@corlewis if you could also have a brief look over this, that would be great.
Eventually I'd like to add tests for this and also an automated cross-check if files produced by this tool can be read properly by the Rust side. But that can be in separate PRs.
corlewis
left a comment
There was a problem hiding this comment.
This looks good to me as well!
I did however struggle to follow some of the logic on a first read through, particularly in the very large where clause of translate. I have the feeling that this would be easier to read and more maintainable if most of those helpers were separated out into their own functions with type annotations, but this almost definitely isn't worth doing now.
I was also going to say something about the style in this file not matching the rest of the repository, but after a quick look online I think it's more consistent with modern Haskell style. Again, not worth doing but if anyone ever has the time it could be nice to update the old files.
| irqs = | ||
| [ (irq, translateId obj) | ||
| | (irq, obj) <- M.toAscList irqNode | ||
| ] |
There was a problem hiding this comment.
Is this really the correct style for multiline list comprehensions? This specific case could probably be a single line, but if it does have to be multiple lines then I would have expected that all of it is contained inside of the list brackets.
| irqs = | |
| [ (irq, translateId obj) | |
| | (irq, obj) <- M.toAscList irqNode | |
| ] | |
| [ (irq, translateId obj) | |
| | (irq, obj) <- M.toAscList irqNode | |
| ] |
There was a problem hiding this comment.
I'm having trouble finding an authoritative source that clearly states the convention here, but as far as I am aware, the current indentation follows a prevailing convention.
Were you thinking of something more complicated, or just basic processing tests like we do with the examples in this repository? Something like that for this format would be nice, to at least make sure that other changes to the tool don't break things in unexpected ways. |
I did a bit of refactoring to pull the largest |
Introduces a new output format. This format is a JSON-serialized representation of the spec, for consumption by the Rust implementation of the CapDL initializer. Co-authored-by: Ivan Velickovic <i.velickovic@unsw.edu.au> Signed-off-by: Nick Spinale <nick@nickspinale.com>
For now just basic processing tests on the examples we already have. Longer term, It'd be nice to generate specs and use them to cross-check the tools and loaders against each other, but that needs a bit of time to implement. |
lsf37
left a comment
There was a problem hiding this comment.
I'm happy with the changes. Will merge.
Adds
parse-capDL ... --json=spec.json ...which outputs the CapDL spec in a JSON format for use with the Rust CapDL initializer.The
SpecHaskell type here must be kept in sync with theSpecRust type in that repository.