#145 Update to Checkstyle 8.19

[Calixte]

fix for #145

[rnveach]

Duplicate effort of #158

According to https://github.com/checkstyle/eclipse-cs/pull/142/files and https://github.com/checkstyle/eclipse-cs/pull/158/files ,

• net.sf.eclipsecs.checkstyle/.classpath has to be updated
• net.sf.eclipsecs.checkstyle/META-INF/MANIFEST.MF needs more changes
• ConfigurationReader changes are not the same and have to be explained.
• We should have some tests for ConfigurationReader changes
• We need to know if update to checkstyle 8.19 #158 (comment) is still a concern

[Calixte]

• I can do the net.sf.eclipsecs.checkstyle/.classpath change.
• I kept my changes to net.sf.eclipsecs.checkstyle/META-INF/MANIFEST.MF down to what #135 Update to Checkstyle 8.18 #142 is doing. The other changes seem to belong in a follow-up commit prepare 8.19.0 release.
• The changes to ConfigurationReader suggested by update to checkstyle 8.19 #158 are not possible with the current architecture. If you remove tabWidth from under TreeWalker in net.sf.eclipsecs.checkstyle/metadata/com/puppycrawl/tools/checkstyle/checkstyle-metadata.xml, then the ConfigurationReader will not be able to able to read the tabWidth property for TreeWalker. If you don't remove it, then ChecksTest complains that the tabWidth property is not possible for TreeWalker. Therefore, ConfigurationReader can only read the tabWidth property from Checker.
• I don't believe getting the visual offset from any other XML node than Checker is possible without a major change. I think the proposed change matches the expectations that one has when reading the checkstyle 8.19 documentation.

[Calixte]

I pushed another version that relies on the column char index instead of the column count to decide where to show the marker. Since it's not trying to compute the offset by using the tab width, it now works wherever you put the tabWidth parameter (LineLength, Checker, TreeWalker...).

[romani]

@Calixte , I appreciate your contribution, it would be awesome if we cooperate with #168 to make a fix.

I personally would love to release eclipse-cs to each version of checkstyle 8.19, 8.20, .... , but project right now in such a state that probably it is ok to leap to 8.24.

[Calixte]

Sure.
I do have a branch per version on my fork if you decide you want to release each of them.

[romani]

@Calixte , sorry for delay, I will switch to eclipse-cs very soon, to unblock your updates.

[romani]

@Calixte or @ManfredTremmel , can you onboard me on how to start development of plugin ?
I have bunch of errors right after projects import:

Plugin execution not covered by lifecycle configuration: 
org.apache.maven.plugins:maven-clean-plugin:2.5:clean 
(execution: default-clean-1, phase: initialize)  
pom.xml /net.sf.eclipsecs-updatesite
  line 10 Maven Project Build Lifecycle Mapping Problem


Plugin execution not covered by lifecycle configuration: 
org.eclipse.tycho:tycho-compiler-plugin:1.4.0:compile
 (execution: default-compile, phase: compile) 
pom.xml /net.sf.eclipsecs.branding
  line 10 Maven Project Build Lifecycle Mapping Problem

....

As soon I be able to run all my local, I can start accepting fixes. I appreciate your help.

[ManfredTremmel]

@romani I would be glad to do it, but I have no experience in developing Eclipse plugins. In the case of eclipse-cs, I've simply checkout out from git, build on command line and imported then into eclipse. Eclipse was happy with the stuff, I'm using 2019-09.

[Calixte]

You should right click on one of those errors, click Quick Fix, then Select All, then select "Discover new m2e connectors", Finish. It will suggest a plugin to install, agree and install. Errors should be gone after Eclipse restarts and builds.

[rnveach]

Basically eclipse has its own maven plugin that tells the 2 how to cooperate together. For maven to work in eclipse, eclipse needs to know how to behave when certain phases are called on like compile, test, etc. The warning just means that the default maven plugin doesn't know how to handle the areas its flagged and wants you to look for more plugins.

Checkstyle project gives me the same errors and for the most I ignore them and don't have any issues with development.

[romani]

@Calixte , thanks a lot.
problem is fixed as you suggested:

[romani]

@ManfredTremmel , please review this PR and approve it if all is done correctly.

[Calixte]

Good to hear.
Let me know if you want me to push the changes where I create the release note and up the version everywhere (like 0115725) in the same pull request.

[ManfredTremmel]

@romani I'm not member of the checkstyle project, so I can't approve it. Code Review from my side is ok.

[romani]

@ManfredTremmel or @Calixte ,
at https://github.com/checkstyle/eclipse-cs/wiki/How-to-update-to-a-new-Checkstyle-core-version#update-meta-data there is

Requires PDE: Start a runtime workspace (a new eclipse workspace using the eclipse-cs plugin projects) and test the changes.

I do not understand this.
I do not know how to run eclipse-cs , please explain me.

[romani]

Let me know if you want me to push the changes where I create the release note and up the version everywhere (like 0115725) in the same pull request.

lets skip this for now, It mostlikely to be done at https://github.com/checkstyle/eclipse-cs/wiki/How-to-release#prepare-and-test-release-build , I have never did this, so I will try to follow instructions from original maintainers for some time (before we become familiar with all steps.)

[romani]

I did build of update-jar by maven mvn clean package, tested it:

looks, ok.
Looks like we are ready for merge and release.
@Calixte , please rebase.

[rnveach]

@romani Did you review #158 (comment) to see if it is still a concern or not?

[romani]

I do not know how to reproduce this. In current state of project .... it is ok to do some awkward steps and cause braking compatibility.

[romani]

ok, PRs commits are rebased and merged .... to let me actually try release process.