search

pboyer / antlr4

ANTLR (ANother Tool for Language Recognition) is a powerful parser generator for reading, processing, executing, or translating structured text or binary files.
http://antlr.org
Other
26 stars 1 forks source link

Sync go testsuite stg, switch off other langs. #18

Closed jwkohnen closed 8 years ago

jwkohnen commented 8 years ago

Sync testsuite's go templates to the java source code and temporarily switch off code generation for other language targets than Go (it causes too much noise).

Do not pull, yet. I think this has a regression.

@willfaught In this diff I see a change that I vagely remember you have introduced. Maybe you have patched the Java source instead of its template? I'll put an inline comment in this PR.

pboyer commented 8 years ago

Let me know when you want me to take a look.

jwkohnen commented 8 years ago

Thanks, and sorry for the confusion. The source of these changes seem to differ from what I vaguely remembered. I did some heavy lifting on the templates and ran into some issues. I think I could/should ask some questions that maybe provide answers to both issues, the ones at hand here and for my work on templates. But I need to do a bit of "homework" first... properly learn ANTLR's StringTemplate engine.

jwkohnen commented 8 years ago

Commit 3434e802be38f6e5e4473ab68b20677d9b758d50 introduced the all the manual changes to the generated sources that pop up here.

What I did: In a docker instance with an empty local maven repository I've run a script that chronologically checks out every commit since pboyer's fork back in the day until this PR branch's tip (168 commits) and copies the file TestParserExec.java--as it was commited--to a log, then runs a clean build, generate the test files from the template and copies the freshly generated TestParserExec.java to the log as well.

So I have positively confirmed the commit 3434e802be38f6e5e4473ab68b20677d9b758d50 is the only one that introduces changes that are reset by this PR.

The last commit that changed the template such that the generated file TestParserExec.java changed was 7fc028409e369d28d43c9636c748baffa60bec63. Except for the time period between these two commits, the git history is littered with out of sync copies of this file.

So, @willfaught's intended changes are accounted for (it's all in 3434e802be38f6e5e4473ab68b20677d9b758d50): they need to be reworked into the templates. Other than that this PR is regression-free. Please pull, because the change in the pom.xml makes keeping this in sync a lot easier.

The only file in the directory runtime-testsuite/test/org/antlr/v4/test/runtime/go that is directly editable is BaseTest.java. Every other file in that directory is generated. The template for these files is runtime-testsuite/resources/org/antlr/v4/test/runtime/go/Go.test.stg. Whenever that file is touched you need to generate the test files as described in runtime-testsuite/README.md. Before this PR you need to reset the unrelated file (git checkout <file>...) and commit the go related test files.

All other templates are generated on the fly. runtime-testsuite is the only child project that needs to manually generate and commit them. Whatever the reason is. This build system is very, very brittle. :\

willfaught commented 8 years ago

To summarize, are these changes the result of just re-generating the files in the test directory? And the changes in 3434e80 should be made in their counterpart templates?

Edit: If so, LGTM.

jwkohnen commented 8 years ago

Oh, sorry, I thought I had answered already: Yes and yes. :)

willfaught commented 8 years ago

@wjkohnen Is this ready to be merged?

pboyer commented 8 years ago

@wjkohnen Please let us know when this is ready to go. cc @willfaught

pboyer commented 8 years ago

@wjkohnen @willfaught I still feel a bit in the dark about what's going on here. I'm going to close this PR until we have more clarification.

willfaught commented 8 years ago

I think that's fine. It was just a development convenience, and the code has already been re-generated.