Skip to content
This repository has been archived by the owner on Mar 21, 2021. It is now read-only.

Remove terminal commas #461

Open
MathieuAA opened this issue Apr 27, 2020 · 12 comments
Open

Remove terminal commas #461

MathieuAA opened this issue Apr 27, 2020 · 12 comments

Comments

@MathieuAA
Copy link
Member

MathieuAA commented Apr 27, 2020

This issue comes from a discussion between @colameo and I about terminal commas.
This issue's goal is to define the necessary rules to make terminal commas obsolete in the future, and/or at least make them optional in all the relevant cases first.

Any opinion is welcome

@colameo if the description doesn't convey your meaning please do rephrase it.

@colameo
Copy link
Member

colameo commented Apr 28, 2020

IMO all classifier elements of type:

<classifier> <name> {
    elements = element (',' element)*
}

should be without commas ,

there is really no benefit of having a separator and because it's optional it makes even harder to parse because weird definitions like the followings are allowed and also valid:

entity Department {
  guid UUID required,
  name String required unique
  description TextBlob,
  advertisement Blob
  logo ImageBlob
}

@MathieuAA
Copy link
Member Author

If we remove the commas completely, it would be a breaking change (and not a small one). Making them optional first and printing warnings about them should be a good start. Maybe the linter can help for this (I really have to dig it up).

As a side note, if we remove commas out of the equation, this leaves the newline as a separator. Not so sure about this one.

@colameo
Copy link
Member

colameo commented May 2, 2020

could we at least remove commas from application and deployment config sections? that would simplify the handling of options a lot ... (at least for the current refactoring work I'm doing).

@MathieuAA
Copy link
Member Author

MathieuAA commented May 2, 2020

Yes, and relationships too. I'm merging JCore and the generator and this is taking me some time. I'll get to it right after

@MathieuAA
Copy link
Member Author

@colameo I'll be working on it today

@MathieuAA
Copy link
Member Author

Okay, the only case where commas aren't optional is with the enum, I'm making them optional.

@MathieuAA
Copy link
Member Author

The next step would be to update the linter and make the JDL importer use it

@funder7
Copy link

funder7 commented Jun 15, 2020

Hi, I came here from issue #347, even ```export-jdl`` command should export a JDL without commas.
I'm writing it here just as reminder :-)

@MathieuAA
Copy link
Member Author

Hello, don't worry it's not forgotten ;) just don't have time to do it...

@funder7
Copy link

funder7 commented Jun 16, 2020

Okay! :-D

@MathieuAA
Copy link
Member Author

You know what? I'll do this this weekend. Thanks :)

@funder7
Copy link

funder7 commented Jun 18, 2020

Yeah!! Let's do it! :-D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants