lingua-franca
lingua-franca copied to clipboard
Add capability to generate LF code and create a CLI program for formatting
Tasks:
- [x] Implement missing cases in visitor pattern
- [x] Implement line wrapping (Billy, Peter) -- (edit: @petervdonovan is making another pass at this)
- [x] Preserve comments - @petervdonovan
- [x] Allow the user to disable formatting for specific sections of their code (see comments below) - @petervdonovan
- [x] Add CI check that applies formatter to all *
.lf
files in thetest
directory and checks for equivalence (@lhstrh) - [x] Add methods for testing semantic equivalence of AST subtrees
- [x] Rename
org.lflang.lfc
toorg.lflang.cli
- @billy-bao - [x] Create
lff
package and createlff
application that applies formatter - @billy-bao - [x] Factor out WIP toward new
FedGenerator
(to be submitted in a separate PR)
Do you think it would be possible to combine this with target language formaters and run, let's say, clang-format on the code bodies?
Yes, I believe we plan to do that, although it may be a separate PR.
I just realized that this approach to formatting can make code like this unreadable:
test = new TraceTesting(
events_size = 5,
trace_size = 165,
trace = (
0,0,0,1,1,0,0,0,0,0,0,
500000000,0,0,0,1,1,1,0,0,0,0,
250000000,0,0,1,1,0,1,0,0,0,0,
250000000,1,1,0,1,0,1,0,0,0,0,
0,0,1,0,1,0,1,1,1,0,0,
500000000,0,1,0,1,0,1,0,1,1,1,
250000000,0,1,0,1,0,1,1,1,0,1,
250000000,1,1,0,1,0,1,0,1,0,1,
250000000,0,1,0,1,1,1,0,1,0,1,
250000000,0,1,1,1,0,1,0,1,0,1,
500000000,1,1,0,1,1,1,0,1,0,1,
0,0,1,0,1,0,1,1,1,0,1,
500000000,0,1,0,1,0,1,0,1,1,1,
250000000,0,1,0,1,0,1,1,1,0,1,
250000000,1,1,0,1,0,1,0,1,0,1
), training = false)
The placement of newlines in this trace definition seems important, but if we are generating straight from the AST, that information is erased. It ends up looking like:
test = new TraceTesting(
events_size = 5,
trace_size = 165,
trace = (
0,
0,
0,
1,
1,
0,
0,
0,
0,
0,
0,
500000000,
0,
0,
0,
...
My basic idea here was that the goal of the programmer is to craft an AST, not a text file (programs != ASCII art), but this is an exception.
We have the option to let the programmer include a comment to force the original format to be preserved. Something like:
trace = ( // keep-format
0,0,0,1,1,0,0,0,0,0,0,
500000000,0,0,0,1,1,1,0,0,0,0,
250000000,0,0,1,1,0,1,0,0,0,0,
...
This probably would be easy to implement, but it is not very pretty. I'm open to other ideas.
I don't quite understand the code example. Is this LF code or target code?
Also other code formatters face the problem that you describe. Special comments that switch of the formatting are the standard solution. When it comes to formatting, there is also a lot of opinion involved about the right or best style. We could also consider making certain decisions configurable, but this is probably something we would tackle further down the road. However, personally, I like the philosophy of the Python formatter black, which does not allow configuration.
I don't quite understand the code example. Is this LF code or target code?
It is LF code, copied directly from a modal models test.
Yes, I also like Black and was inspired by it. I do not really want configurability. I just brought up this issue because a near-term goal is to format all of our tests with this formatter, and because I think even a Black fanatic would agree that the lists of numbers that appear in the modal models tests do benefit greatly from their particular use of whitespace.
Ok, I see. I would be absolutely in favour of adding a comment that can switch formatting off. Black uses this:
# fmt: off
np.array(
[
[1, 0, 0, 0],
[0, -1, 0, 0],
[0, 0, 1, 0],
[0, 0, 0, -1],
]
)
# fmt: on
The main problem that I see, is that we need to be somehow able to switch formatting back on or limit the scope. In your proposed solution:
trace = ( // keep-format
0,0,0,1,1,0,0,0,0,0,0,
500000000,0,0,0,1,1,1,0,0,0,0,
250000000,0,0,1,1,0,1,0,0,0,0,
...
would the // keep-format
only apply to the annotated AST node?
We could also consider using the new annotations for this purpose. This might be clearer than comments, espacially since it would clearly separate annotations from comments in target code blocks that influence target code formatting.
would the
// keep-format
only apply to the annotated AST node?
Yes. As it turns out, having a single comment that applies to the annotated AST node is much easier to implement than using two comments to toggle the formatter on and off. This is because, in order for the formatter to work correctly, it has to associate comments with AST nodes regardless. I would like to keep the // keep-format
approach unless anyone feels sure that a different approach is better.
I think force-pushing to this branch that has dependents is not a great idea. This created a lot of issues in the fed-gen branch because the history was lost.
Can we try to merge this soon?
I promise that the change I just made was trivial and that this has mostly just been awaiting review for 23 days.
Sorry, this was not on my radar as my review wasn't requested. I'll have a look at this soon. I agree, we should get this merged...