Skip to content

Automatically generate eclipse settings#249

Merged
koppor merged 1 commit into
masterfrom
eclipse_config
Oct 24, 2015
Merged

Automatically generate eclipse settings#249
koppor merged 1 commit into
masterfrom
eclipse_config

Conversation

@koppor

@koppor koppor commented Oct 20, 2015

Copy link
Copy Markdown
Member

gradlew eclipse now generates formatter and save action settings.

Since the default settings of Eclipse won't change in the near future (see Eclipse bug 479928) and since it takes a few steps to configure Eclipse correctly, we try to create the required configurations

  • warnings,
  • formatter,
  • cleanup,
  • and save actions

automatically.

Following minor issues remain:

  • When saving, Eclipse formats some lines. Not only the changed ones (as told) and not all (if configured to format all)
  • On Windows, the inserted lines are separated by LF and not CRLF as the part generated by gradle in org.eclipse.jdt.core.prefs. Works without issues in Eclipse, but causes difficulties in diffing with git.
  • When regenerating the config, the content is simply appended and not replaced. The user has to run gradlew eclipseClean manually
  • The org.eclipse.jdt.core.pref file is not sorted alphabetically: First come the gradle generated propreties and then our properties. This does not harm, but if a user changes a setting in eclipse. the file gets reordered. No drawbacks in the functionality, but hard to diff.

I propose to reformat the whole source code using the new formatter settings (solves the last issue) in the near future and to ask all committers to regenerate their Eclipse config with the new gradle target.

I tested the formatter with Eclipse Mars SR1 and JabRefPreferences. It looks good at first sight 😸

@matthiasgeiger

Copy link
Copy Markdown
Member

When saving, Eclipse formats some lines. Not only the changed ones (as told) and not all (if configured to format all)

This might be caused by some "duplicates" in the gradle script:
cleanup.format_source_code_changes_only=false
vs.
sp_cleanup.format_source_code_changes_only=true

What is "sp_cleanup"? Are those the save actions?

@koppor

koppor commented Oct 21, 2015

Copy link
Copy Markdown
Member Author

sp_cleanup are the save actions. I think, the cleanup is called manually, the save actions always when saving. I tried true and false at sp_cleanup.format_source_code_changes_only and had less changed when setting it to true.

@simonharrer

Copy link
Copy Markdown
Contributor

I do not like that this is polluting the build.gradle file. Can you please put this in an eclipse.gradle file and just import it from the build.gradle?

https://docs.gradle.org/current/userguide/userguide_single.html#sec%3aconfiguring_using_external_script

@koppor

koppor commented Oct 21, 2015

Copy link
Copy Markdown
Member Author

I had the choice between

  1. polluting one file (only read by our build experts) and not polluting the main directory (seen by every person working with the source) and
  2. not polluting one file and polluting the main directory

I opted for the first one. If @stefan-kolb agrees, I choose the second option and will generate the additional file eclipse.gradle.

@stefan-kolb

Copy link
Copy Markdown
Member

This is a lot of stuff. I second creating an exclusive eclipse.gradle file.

@koppor koppor added this to the v2.80 milestone Oct 22, 2015
@koppor koppor changed the title [WIP] Automatically generate eclipse settings Automatically generate eclipse settings Oct 24, 2015
@koppor

koppor commented Oct 24, 2015

Copy link
Copy Markdown
Member Author

Result of discussion: merge, leave the minor issues (see PR description) open, and format the complete code later.

koppor added a commit that referenced this pull request Oct 24, 2015
Automatically generate eclipse settings
@koppor koppor merged commit eb86a32 into master Oct 24, 2015
@koppor koppor deleted the eclipse_config branch October 26, 2015 00:07
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

Successfully merging this pull request may close these issues.

4 participants