Skip to content
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

Address Issue with Array-Initialization -- Other General Cleanup #63

Merged

Conversation

engineerjoe440
Copy link
Collaborator

Related Issues

Changes

  • Remove method_statement from iec.lark
  • Remove MethodStatement from blark.transformers
  • Restore previous modification affecting array initialization
  • (TODO) Correct issue affecting function-block declaration with parameter initialization

Closing Thoughts

TBD

@klauer
Copy link
Owner

klauer commented Apr 19, 2023

I think I've got a fix for this. There's a bit of a loss of information when transforming array_initialization. Non-bracketed and bracketed versions are parsed/transformed into the same dataclass. Then round-tripping it to code results in brackets always being re-added - which can be undesirable.

This commit splits them up and tracks whether the original code had brackets or not:
a17df30

If this works for you, feel free to just fast-forward your branch / cherry-pick the commit / etc

I haven't had a chance to test this on our full set of projects just yet. I did want to make it a possibility for others to reproduce my efforts, though, so there's a little script in our organization's "plc-summary" repository that allows you to parse the public code we have.

It'd be neat to one day make a GitHub Actions workflow that does this automatically. That said, it'd take a bit too long to do on each commit and probably frustrate us in the end...

@engineerjoe440 engineerjoe440 force-pushed the bugfix/array-initialization-with-zero branch from 47c0703 to a17df30 Compare April 19, 2023 19:39
@engineerjoe440
Copy link
Collaborator Author

Yes!!!! This is great! Thanks for digging into this. I was actually just going to spin back up on some of this work, today.

I've got to say... that "plc-summary" is VERY cool!!!! That's terrific! 😲

@engineerjoe440 engineerjoe440 marked this pull request as ready for review April 19, 2023 19:57
This was linked to issues Apr 19, 2023
@klauer
Copy link
Owner

klauer commented Apr 19, 2023

Yes!!!! This is great! Thanks for digging into this. I was actually just going to spin back up on some of this work, today.

Feels good to close up another loose end!

I didn't mean to rush you, for what it's worth - I'll be away for a couple weeks after tomorrow, so I wanted to get this fixed up so blark can get a new tag.

I've got to say... that "plc-summary" is VERY cool!!!! That's terrific! 😲

Thanks! I'm pretty fond of it too 😁

Looking back at test_transformer, looks like we can remove 2 additional xfails! I'm going to push a commit, approve, merge, and tag.

@engineerjoe440
Copy link
Collaborator Author

No rush at all, @klauer! I'm glad to get this "loose end" tied up too! 🎉

So excited to see this project keep moving forward! 😀

@klauer klauer merged commit 4ea1ea0 into klauer:master Apr 19, 2023
@engineerjoe440 engineerjoe440 deleted the bugfix/array-initialization-with-zero branch April 19, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove method_statement from grammar Array Initialization with Zero
2 participants