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

syntax for specifying maven group + artifact ids #6872

Closed
gavinking opened this issue Jan 19, 2017 · 8 comments
Closed

syntax for specifying maven group + artifact ids #6872

gavinking opened this issue Jan 19, 2017 · 8 comments
Assignees
Milestone

Comments

@gavinking
Copy link
Member

@gavinking gavinking commented Jan 19, 2017

@FroMage has introduced an artifact annotation for specifying Maven group and artifact ids. I'm not happy with that since our import statements already have a welldefined syntax for specifying these. I will introduce the following syntax:

module org.hibernate.core org.hibernate:"hibernate-core" "2.1.1" { ... }
@gavinking gavinking added this to the 1.3.2 milestone Jan 19, 2017
@gavinking gavinking self-assigned this Jan 19, 2017
@gavinking gavinking changed the title syntax for specifying maven group artifact ids syntax for specifying maven group artifact ids Jan 19, 2017
@gavinking gavinking changed the title syntax for specifying maven group artifact ids syntax for specifying maven group + artifact ids Jan 19, 2017
@FroMage
Copy link
Contributor

@FroMage FroMage commented Jan 20, 2017

I don't follow your argument about syntax since that syntax is obviously different from that of module imports, which don't feature both Ceylon and Maven parts.

I should also note that ATM we allow artifact to have an optional artifactId which lets us set the groupId and declare that the artifactId is the Ceylon module name entirely.

That said, I don't object to this, I don't see either option as strictly better.

@gavinking
Copy link
Member Author

@gavinking gavinking commented Jan 20, 2017

that syntax is obviously different from that of module imports, which don't feature both Ceylon and Maven parts.

Not really. For a module declaration it is:

"module" packagePath (groupId ":" artifactId)? version

For a module import it is:

"import" (packagePath | groupId ":" artifactId) version

Same thing. Just that with import statements specifying both the module name and the group/artifact ids would be redundant.

@gavinking
Copy link
Member Author

@gavinking gavinking commented Jan 20, 2017

Actually, I suppose the syntax would really be:

module org.hibernate.core maven:org.hibernate:"hibernate-core" "2.1.1" { ... }
gavinking added a commit that referenced this issue Jan 20, 2017
@gavinking
Copy link
Member Author

@gavinking gavinking commented Jan 20, 2017

@FroMage I have added support for this syntax to the language grammar and AST. You just need to wire it in now.

@lucono
Copy link

@lucono lucono commented Jan 26, 2017

I really like this new syntax.

For cross-platform modules, will it support specifying values for multiple foreign repo systems?

module com.acme.lib.core maven:com.acme.lib:"lib-core" npm:"acme-lib-core" "2.1.1" {
    // ...
}

So that:

ceylon: module = com.acme.lib.core
maven: groupId = com.acme.lib, artifactId = lib-core
npm: module name = acme-lib-core

And for all of these, version = 2.1.1

@gavinking
Copy link
Member Author

@gavinking gavinking commented Jan 26, 2017

will it support specifying values for multiple foreign repo systems?

It doesn't yet, but it will.

lucaswerkmeister added a commit to ceylon/ceylon.ast that referenced this issue Feb 8, 2017
This was broken by eclipse/ceylon@cf165b0 (for eclipse/ceylon#6872) –
apparently the parser needs some more lookahead now, so let’s give it a
fake version.
@FroMage
Copy link
Contributor

@FroMage FroMage commented Feb 14, 2017

Done.

@lucaswerkmeister
Copy link
Member

@lucaswerkmeister lucaswerkmeister commented Feb 14, 2017

It doesn't yet, but it will.

Is that coming before the next release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants