Skip to content

cpp: Enable codegen #414

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

Merged
merged 2 commits into from
Jun 18, 2025
Merged

cpp: Enable codegen #414

merged 2 commits into from
Jun 18, 2025

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Jun 17, 2025

🤔 What's changed?

  • Adds cpp to the root make file so the parser is updated with make generate
  • Regenerate cpp parser

⚡️ What's your motivation?

Investigate #412
Resolve #333 for cpp.

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

📋 Checklist:

* Adds cpp to the root make file so the parser updated with `make generate`
* Regenerate cpp parser
* Fix #333 for cpp
@mpkorstanje mpkorstanje force-pushed the fix/enable-ccp-code-gen branch from a044ed1 to 485816d Compare June 17, 2025 14:02
@mpkorstanje
Copy link
Contributor Author

Looking at #413 the acceptance tests pass except for #400.

@musicinmybrain
Copy link
Contributor

Looking at #413 the acceptance tests pass except for #400.

I haven’t figured out how to fix this, but I prepared #415 along the way.

@musicinmybrain
Copy link
Contributor

The (first) failing acceptance test in this case is:

diff --unified ../testdata/good/prefixed-keywords.feature.tokens acceptance/testdata/good/prefixed-keywords.feature.tokens
--- ../testdata/good/prefixed-keywords.feature.tokens   2025-06-17 07:49:09.772177553 -0400
+++ acceptance/testdata/good/prefixed-keywords.feature.tokens   2025-06-17 11:32:48.135765735 -0400
@@ -7,6 +7,6 @@
 (7:5)StepLine:(Context)Sipoze ke /there is agent J/
 (8:5)StepLine:(Conjunction)Ak /there is agent K/
 (9:5)StepLine:(Action)Le /I erase agent K's memory/
-(10:5)StepLine:(Outcome)Le sa a /there should be agent J/
+(10:5)StepLine:(Action)Le /sa a there should be agent J/
 (11:5)StepLine:(Conjunction)Men /there should not be agent K/
 EOF

which is exactly the example in #414.

@mpkorstanje
Copy link
Contributor Author

Thanks for the quick PR. We can fix this in small steps. I'll check/merge tomorrow.

To further fix things, in general terms, in the implementation there should be a match_{somethint step related} method that checks which of the keywords matches. It does this by checking the given/when/then keywords separately. These keywords should be combined and sorted on length before this is done.

The solutions to #400 might provide some inspiration.

But I'm on my phone right now so that's all I can usefully contribute for today.

@musicinmybrain
Copy link
Contributor

I’ve managed to fix the last acceptance test failure in #416.

@mpkorstanje mpkorstanje merged commit b227dd3 into main Jun 18, 2025
5 checks passed
@mpkorstanje mpkorstanje deleted the fix/enable-ccp-code-gen branch June 18, 2025 18:31
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.

Comment inside Free Form Description not allowed
2 participants