openEHR / adl-antlr

Antrl4 grammars for ADL
Apache License 2.0
1 stars 4 forks source link

Recommendations #18

Open ghost opened 8 years ago

ghost commented 8 years ago

By the way, the Maven plugin is configured default as this: http://www.antlr.org/api/maven-plugin/latest/usage.html

(the version in this example should be 4.5)

Generating Javafiles will be then: mvn antlr4:antlr4 The Javafiles will be then in target/generated-sources/antlr4/....(package)...

You find all worked out in my fork: https://github.com/BertVerhees/adl-antlr

If you do it like this, handling the grammars will be very easy for Java-programmers, all the packages are allright, everything is found and works, and have nice ClassNames.

Bert

wolandscat commented 8 years ago

I thought I read somewhere that src/main/antlr was the appropriate directory - is that only for pre-Antlr4 versions? Or does Maven even care?

Root grammars - note that Odin is also a root grammar, as it is used on its own, as well as being imported into ADL.

I don't think the 'import' directory idea is a particularly good one - it is based on a naive idea of why grammars are cut into separate pieces. In the ADL grammars here, all the grammar files are equally part of the 'ADL grammar' and 'ODIN grammar' - it's just more convenient to cut them up for re-use purposes. It would also make it harder for the publication tools to find the grammars - because they are referenced now from the ADL and ODIN specification projects using paths. We can do it if the Antlr tools absolutely require it, but it seems pointless and annoying to me.

But I agree that the most likely way for these tools to be used is with Maven, and we want to make things easy for developers. I'll have a look at making some more changes.

ghost commented 8 years ago

On 28-10-15 14:53, Thomas Beale wrote:

I thought I read somewhere that src/main/antlr was the appropriate directory - is that only for pre-Antlr4 versions? Or does Maven even care?

You can define in it the pom.xml, but antlr4 is the default.

Root grammars - note that Odin is also a root grammar, as it is used on its own, as well as being imported into ADL.

That is a problem, maybe it must occur two times in the structure. I tried it, it seems to work fine

I don't think the 'import' directory idea is a particularly good one - it is based on a naive idea of why grammars are cut into separate pieces. In the ADL grammars here, all the grammar files are equally part of the 'ADL grammar' and 'ODIN grammar' - it's just more convenient to cut them up for re-use purposes. It would also make it harder for the publication tools to find the grammars - because they are referenced now from the ADL and ODIN specification projects using paths. We can do it if the Antlr tools absolutely require it, but it seems pointless and annoying to me.

It is only to support Java/Maven based environments. Maybe a text to explain it accompanying the grammars could work too.

But I agree that the most likely way for these tools to be used is with Maven, and we want to make things easy for developers. I'll have a look at making some more changes.

There are more build tools, but I am glad I can find my way in Maven, and not eager to learn another (it is all a bit different for the same purpose). Other developers are brought up in other circumstances.

So, a text-file explaining it is a fine solution.

pieterbos commented 8 years ago

The CamelCased names would be nice - it looks a bit ugly in java code right now.

I don't think the directory structure matters. If you use this grammar, you will probably copy it in your own project anyway, or reference it with a custom source directory. In Gradle - build tool of my preference that is gaining widespread adoption fast - it's src/main/antlr by default, but very easily changed.

wolandscat commented 8 years ago

I don't particularly want to change the rules to CamelCase, it's much harder to read, and I don't like making things hard to read (why we use CamelCase anywhere is a complete mystery to me - it's a mistake of IT, like XML!). Probably the best thing is to get a wider consensus first, and that requires these grammars to be more widely used.

ghost commented 8 years ago

Ok Thomas, you don't like camel case and XML. It does not matter much. It are just conventions, no big deal. I would not call it a mistake, every convention has pros and cons.

Regarding the grammar, the used names are only visible to Java-developers, and used to java conventions. They have bad luck you call their convention a mistake and so they must work with code they experience as ugly.

I remember somewhere having a tool in an editor to switch convention. Maybe it is in an Apache library, somewhere I found it.

Have a nice evening. Op 28 okt. 2015 18:00 schreef "Thomas Beale" notifications@github.com:

I don't particularly want to change the rules to CamelCase, it's much harder to read, and I don't like making things hard to read (why we use CamelCase anywhere is a complete mystery to me - it's a mistake of IT, like XML!). Probably the best thing is to get a wider consensus first, and that requires these grammars to be more widely used.

— Reply to this email directly or view it on GitHub https://github.com/openEHR/adl-antlr/issues/18#issuecomment-151909837.

ghost commented 8 years ago

On 28-10-15 18:00, Thomas Beale wrote:

I don't particularly want to change the rules to CamelCase, it's much harder to read, and I don't like making things hard to read (why we use CamelCase anywhere is a complete mystery to me - it's a mistake of IT, like XML!). Probably the best thing is to get a wider consensus first, and that requires these grammars to be more widely used.

— Reply to this email directly or view it on GitHub https://github.com/openEHR/adl-antlr/issues/18#issuecomment-151909837.

I found it: https://plugins.jetbrains.com/plugin/7160?pr=idea

;-)

pieterbos commented 8 years ago

I have no problems with snake_casing - i quite like that in the grammar files and in languages such as ruby. However:

In java, you now get a mix of snake case and camel case, in unexpected places. It would be great if antlr would automatically convert named rules to the naming convention of the target language (does not have to be java!) - however, it does not. So now you get code like:

private void parseCAttribute(CComplexObject parent, C_attribute_defContext attributeDefContext) {

The capital C in Context comes from ANTLR generated code - i can't change it to _context. So I would expect:

private void parseCAttribute(CComplexObject parent, CAttributeDefContext attributeDefContext) {

Which is more consistent.

But running (or even creating) a script that changes this is easy, so no big problems.

wolandscat commented 8 years ago

If we could preserve the readability in the grammars and do some sort of conversion to deal with those languages using CamelCase it would be good. If we target Ruby, C++, Erlang, Eiffel, PHP, snake_case is preferable.

ghost commented 8 years ago

I think it is a lost battle. Best is that people make their conversion. We better stick to one grammar and call that the official one.

We have managed always like this, we should go on the same way. Classes in capital and attributes in lowercase snake_case.

I take care of my conversions as part of my development process.

Bert Op 29 okt. 2015 19:11 schreef "Thomas Beale" notifications@github.com:

If we could preserve the readability in the grammars and do some sort of conversion to deal with those languages using CamelCase it would be good. If we target Ruby, C++, Erlang, Eiffel, PHP, snake_case is preferable.

— Reply to this email directly or view it on GitHub https://github.com/openEHR/adl-antlr/issues/18#issuecomment-152270106.

wolandscat commented 8 years ago

Let me summarise changes I could make here:

Anything else?