Improve pretty printing #1520
Improve pretty printing #1520
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1520 +/- ##
==========================================
+ Coverage 77.04% 77.36% +0.31%
==========================================
Files 295 297 +2
Lines 61515 62090 +575
==========================================
+ Hits 47397 48034 +637
+ Misses 12503 12441 -62
Partials 1615 1615
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit 5d15cd5 Results
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Thats a one big PR 😅 I mostly reviewed the pattern and general go code, and Looks good!
I have two suggestions:
-
One suggestion for test: perhaps add a test-case where a complex program that covers all (or an extensive set of) AST nodes, is parsed from the source and pretty printed, and compared it with the original source. This can eliminate any potential mistakes in manually asserting the "docs". The test can be of two levels:
- Level 1: Original source code is properly formatted. The round trip should produce the original source code.
- Level 2: Further extend above by randomly increasing/changing space/newlines, and check the whether the round trip produces the above properly formatted source.
-
It might be good to have the pretty printer code separated from the AST. i.e: make it a visitor (in a separate PR of course).
| if conditionsDoc := b.PreConditions.Doc(preConditionsKeywordDoc); conditionsDoc != nil { | ||
| conditionDocs = append( | ||
| conditionDocs, | ||
| prettier.HardLine{}, |
There was a problem hiding this comment.
Can prettier.HardLine{} be re-used (e.g: a global variable) or is there a possibility that it would get updated in the downstream code/ inside the prettier library?
There was a problem hiding this comment.
All documents are immutable, so they won't get updated in downstream code / inside the prettier library.
It's an empty struct, so IDK if there's any improvement other than saving two characters
Sorry about the size 😅 It's a lot of tests, maybe should have split it up, but kept on hacking away. These are both two great suggestions. As this PR only improves, but does not yet fully complete the pretty printing implementation, I'll add such a test in a follow up. Also, as this PR is mostly improving on the existing functionality / structure, I'll open a refactor in a separate follow-up PR |
Co-authored-by: Supun Setunga <supun.setunga@gmail.com>
|
yeah, of course! |
Work towards #209
Description
Finish the implementation of pretty printing all AST elements:
Improve parsing:
execute"blocks" (special functions)Also, improve the layout of the pretty printing tool (
tools/prettier).Give it a try with
go run ./tools/pretty!masterbranchFiles changedin the Github PR explorer