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

Fixes to C# branch: unit tests and code generation #76

Open
wants to merge 3 commits into
base: c-sharp
Choose a base branch
from

Conversation

rzubek
Copy link

@rzubek rzubek commented Oct 14, 2024

Hi! I'm glad to have found this project, this parser generator is awesome!

I wanted to contribute some improvements, in the hope that the C# variant gets finalized and merged into the main branch - it would be amazing to have C# coverage in addition to the other languages.

I see that there was an old PR#70, which got merged into c-sharp branch, but some of the desiderata were still waiting to be addressed. This pull request addresses the following:

  • C# generator now creates only one file per grammar, which is compatible with C# standard practice (unlike Java which requires a file per class)
  • Upgraded project files (makefile and test csproj) to .NET Core 6/8 for better multi-platform compatibility

Regarding .NET - these changes are only relevant for people running make and unit tests. I upgraded project and test files, because Core 6/8 are the only ones with current long-term support - older versions of .NET are now past their end-of-life date, and running them will be increasingly difficult.

However, this is only for testing. The generated C# code is highly backwards-compatible, and I would expect it to work with .NET going back a decade or more.

Testing:

I ran unit tests on Ubuntu (x86) with dotnet sdks 6.0 and 8.0:

$ make clean test-cs
find examples -name '*.class' -o -name '*.pyc' -exec rm {} \;
find test/grammars -type f -a ! -name '*.peg' -a ! -name __init__.py -exec rm {} \;
./bin/canopy --lang cs test/grammars/choices.peg
./bin/canopy --lang cs test/grammars/extensions.peg
./bin/canopy --lang cs test/grammars/node_actions.peg
./bin/canopy --lang cs test/grammars/predicates.peg
./bin/canopy --lang cs test/grammars/quantifiers.peg
./bin/canopy --lang cs test/grammars/sequences.peg
./bin/canopy --lang cs test/grammars/terminals.peg
cd test/cs && dotnet test --framework net6.0
  Determining projects to restore...
  All projects are up-to-date for restore.
  canopy -> /home/rob/code/canopy/test/cs/bin/Debug/net6.0/canopy.dll
Test run for /home/rob/code/canopy/test/cs/bin/Debug/net6.0/canopy.dll (.NETCoreApp,Version=v6.0)
Microsoft (R) Test Execution Command Line Tool Version 17.8.0 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:   127, Skipped:     0, Total:   127, Duration: 339 ms - canopy.dll (net6.0)

I hope this is useful in bringing this branch closer to merging with main - and please let me know if I can help with C# support!

@rzubek
Copy link
Author

rzubek commented Oct 14, 2024

By the way, one thing that's left unresolved is namespaces.

This port inherits the Java namespace definition, which follows the directory structure. This is a requirement in Java, but almost always the incorrect thing to do in C#, where namespaces are expected to follow logical project structure.

Maybe it would make sense to add another command-line argument to the compiler, which would pass it over to the builder, to let the user customize the namespace?

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.

1 participant