groovy / groovy-eclipse

Eclipse Groovy Development Tools
657 stars 192 forks source link

DSL support always added when importing project into workspace #380

Closed mauromol closed 6 years ago

mauromol commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

I did not design the feature. Personally, I disable DSLD in my workspace and uncheck the auto-add preference. You could do the same.

Workaround 2: pending change gets made for .classpath file, undo it

Workaround 3: add .classpath to your .gitignore (or equivalent for your SCM system)

Workaround 4: add DSLD support to your project and it will not get re-added

mauromol commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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"?