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

DSL support always added when importing project into workspace #380

Closed
mauromol opened this issue Nov 8, 2017 · 8 comments
Closed

DSL support always added when importing project into workspace #380

mauromol opened this issue Nov 8, 2017 · 8 comments

Comments

@mauromol
Copy link

mauromol commented Nov 8, 2017

Consider the following ZIP file:
https://www.dropbox.com/s/vj8m1jnm4nuwk8c/TestGroovy2.zip?dl=0

Open any workspace and do Import | Existing Project Into Workspace, select Archive file and select the above file.
After the import has finished, you'll find a new "TestGroovy2" project in your workspace, which has DSL support enabled.
But if you look at the project inside the ZIP file, DSL support is not enabled.

In other words, when importing the project, Greclipse adds the following line to .classpath file:

<classpathentry exported="true" kind="con" path="GROOVY_DSL_SUPPORT"/>

This is unexpected, because that import wirzard should just import the project "as is".

The same happens if you import a project from the file system (instead of a ZIP file), with or without copying it to the new workspace. This is annoying if the .classpath file is shared in the SCM repository.

@eric-milles
Copy link
Member

One note on this: There is preference in Groovy > DSLD > Automatically add DSL Support to all Groovy objects. I uncheck it in my workspaces.

@mauromol
Copy link
Author

mauromol commented Nov 8, 2017

Interesting, thank you! But nevertheless i would expect it to be applied to newly created projects, not to existing ones just being imported.

@eric-milles
Copy link
Member

I'm inclined to say this is working as designed. When importing a Groovy project, the workspace has a setting saying add or don't add DSL support if it is missing.

@mauromol
Copy link
Author

Hmmm... sorry, I do not agree here. If I want to "import an existing project", I don't expect the import process to change my existing project. No other plugin I know does this. The workspace setting should be applied only to newly created projects, otherwise it is harmful.

Also, this workspace setting is, indeed, a workspace setting, not a project setting that can be shared in the SCM repository together with the project itself. It would not make sense, indeed, to make it a project setting. But as of now, I don't have any way to avoid Greclipse to change my projects in an undesired way every time a project is checked out and imported into a developer workspace. Especially because I'm inclined to think that "Add DSL support" is checked by default (I never checked it explicitly).

If you still think that this is "by design", then I think this design decision was wrong and I hope I could explain it why it is harmful for us.

@eric-milles
Copy link
Member

eric-milles commented Nov 17, 2017 via email

@mauromol
Copy link
Author

mauromol commented Nov 17, 2017

Workaround 3 is not viable, I need to check .classpath in for many reasons.
Workaround 4 is not a workaround ;-)
Workaround 2 is what I do right now, but it's error-prone. I cannot force every team member to remember to do the same. And since workspace settings are not shareable along with the project, I can't ensure they do that and don't commit again the re-added DSLD support in .classpath file.

Disabling the feature at workspace level, again, is hard to enforce in a team, especially because that setting is "on" by default. We create many workspaces on a weekly basis and every team member has its own policy about workspace handling. Shareable project settings exists exactly for this reason.

I still think this is a design flaw: as I said, I never saw any other plugin that changes project files when importing with "Import Existing Project Into Workspace" wizard. It's simply wrong.

Also, trying to see it from another point of view: what is the real point of this option??? DSLD support can be added with just a right click on the project at any time. The only useful use case I can think of is on new Groovy project creation.

@eric-milles
Copy link
Member

It is simply not true that other plug-ins don't edit the .project or .classpath files when a project is imported. Ivy and Maven both do this for sure. Design flaw in your opinion or not, it's not changing.

@mauromol
Copy link
Author

Are you sure that Maven and ivy plugins are changing the project within "Import Existing Project Into Workspace" wizard? Beware, I'm not talking about "Import Maven Project" wizard, but "Input Existing Project Into Workspace". The latter one is just a way to link our copy into the workspace a project which is already set up but resides on another place of the file system or in a ZIP file. I don't use Maven and M2E, but Buildship for Gradle projects behaves exactly in this way: "Import Gradle Project" is meant to properly set up an Eclipse project from a Gradle project (and involves nature/builder additions), while no change operation at all is done with "Import Existing Project Into Workspace".

Could you please at least consider changing the default value of this option from "on" to "off"?

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

No branches or pull requests

2 participants