jeandersonbc / gsoc14-planning

Planning for accepted project on Google Summer of Code 14
1 stars 0 forks source link

Migrate old patches related to Viewers from last GSoC #17

Closed jeandersonbc closed 10 years ago

jeandersonbc commented 10 years ago

As discussed in #16, there might be other patches related to @hendrikstill's work rather than only at johna's branch.

The task is to check for patches from 2013 and merge them into jeandersonbc/eclipse.platform.ui

vogella commented 10 years ago

I'm sure @hendrikstill can help you with the work as your co-mentor. And if something is missing, just re-implement it.

If I use the "git log --oneline --author=Hendrik" command on master I see the following related commits:

4340ee4 Bug 415875 - [JFace][Snippets] Snippet055HideShowColumn hides column, but does not shows column again 09f1124 Bug 413974 - [Viewers] Add generics to the TableViewer 1184903 Bug 414067 - [Viewers] Add generics to CustomHashtable 5ccf110 Bug 414565 - [JFace][snippets] Update JFace snippets to generified viewers 328236b Bug 414559 - [JFace][snippets] Move requirement for JFace Snippets to Java 1.5 271b294 Bug 414356 - [Viewers] Add generics to the ListViewer d5c62ff Bug 412273 - [Viewers] Add generics to the ComboViewer da889ed Bug 413690 - Fix the compiler warnings in package org.eclipse.jface.wizard caused by moving to Java 1.5 df1c938 Bug 413685 - Fix compiler warnings in org.eclipse.jface.preference caused by moving JFace to Java 1.5 d4512cc Bug 413688 - Fix compiler warnings in org.eclipse.jface.util caused by moving JFace to Java 1.5 f04f4c0 Bug 413649 - Fix compiler warnings in org.eclipse.jface.layout caused by moving JFace to Java 1.5 47c1b8d Bug 413628 - Fix compiler warnings in org.eclipse.jface.fieldassist caused by moving JFace to Java 1.5 31c790f Bug 413624 - Fix compiler warnings in org.eclipse.jface.dialogs caused by moving JFace to Java 1.5 b5baf13 Bug 413689 - Fix the compiler warnings in package org.eclipse.jface.window caused by moving to Java 1.5 71da37a Bug 413686 - Fix compiler warnings in org.eclipse.jface.resource caused by moving JFace to Java 1.5 38d75bb Bug 411967 New snippet to demonstrate the usage of ComboViewers 819e6ff Bug 413529 - Fix compiler warnings in org.eclipse.jface.action acfd04f Bug 413499 - Fix compiler warnings in org.eclipse.jface.commands 9975266 Bug 413499 - Fix compiler warnings in org.eclipse.jface.commands 7dadc6e Bug 395213 - Move requirement for JFace to Java 1.5? 7f5a131 Bug 370632 - NPE when copying or moving resources linked to non-local filesystem

I think the following should be cherry-picked in reverse order:

09f1124 Bug 413974 - [Viewers] Add generics to the TableViewer 1184903 Bug 414067 - [Viewers] Add generics to CustomHashtable 271b294 Bug 414356 - [Viewers] Add generics to the ListViewer d5c62ff Bug 412273 - [Viewers] Add generics to the ComboViewer

2014-06-03 8:49 GMT+02:00 Jeanderson Barros Candido < notifications@github.com>:

As discussed in #16 https://github.com/jeandersonbc/gsoc14-eclipse-planning/issues/16, there might be other patches related to @hendrikstill https://github.com/hendrikstill's work rather than only at johna's branch.

The task is to check for patches from 2013 and merge them into jeandersonbc/eclipse.platform.ui

— Reply to this email directly or view it on GitHub https://github.com/jeandersonbc/gsoc14-eclipse-planning/issues/17.

jeandersonbc commented 10 years ago

Thanks a lot for the tips @vogella. Work in progress :+1:

jeandersonbc commented 10 years ago

Commit "Bug 412273 - [Viewers] Add generics to the ComboViewer" fully merged without problems. Conflicts solved:

  1. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractListViewer.java
  2. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/BaseLabelProvider.java
  3. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ComboViewer.java
  4. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ContentViewer.java
  5. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/LabelProvider.java
  6. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/Viewer.java
jeandersonbc commented 10 years ago

Commit "Bug 414356 - [Viewers] Add generics to the ListViewer" fully merged without problems

jeandersonbc commented 10 years ago

Commit "Bug 414067 - [Viewers] Add generics to CustomHashtable" fully merged without problems Conflicts solved:

  1. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/CustomHashtable.java
jeandersonbc commented 10 years ago

Commit "Bug 413974 - [Viewers] Add generics to the TableViewer" merged

Conflicts solved:

  1. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTableViewer.java
  2. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/CellLabelProvider.java
  3. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnLabelProvider.java
  4. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewer.java
  5. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TableViewer.java
  6. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TableViewerRow.java
  7. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ViewerCell.java
  8. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/WrappedViewerLabelProvider.java
jeandersonbc commented 10 years ago

Following commits were fully merged. Tests running as expected

jeandersonbc commented 10 years ago

The following patch was fully merged (tests running okay):

To solve the merge conflict, I changed Comparator<String> to Comparator<Object since the return type of Policy.getComparator() is not compatible to Comparator<String> but Comparator<Object>.

I left the warning regarding a casting to ((ContentViewer<E,I>) viewer).getLabelProvider(); because I want to investigate it later whether this warning suppress is safe.

Conflicts solved:

jeandersonbc commented 10 years ago

Patch https://git.eclipse.org/r/#/c/16308/ fully merged. Tests running fine. Conflicts solved:

  1. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ITreeContentProvider.java
  2. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ITreePathContentProvider.java
  3. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StructuredViewer.java
  4. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeColumnViewerLabelProvider.java
  5. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeNodeContentProvider.java
  6. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreePath.java
  7. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java
  8. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewerRow.java
jeandersonbc commented 10 years ago

Patch https://git.eclipse.org/r/#/c/16409/3 fully merged. Tests running fine.

I decided keep the older version from Snippet063ComboViewer so I can keep focus on the Viewers. Also, a conflict resolution for it seems to be not trivial. Later, I will keep focus on updating the Snippets.

Conflicts solved:

  1. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ArrayContentProvider.java
  2. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ContentViewer.java
  3. examples/org.eclipse.jface.snippets/EclipseJFaceSnippets/org/eclipse/jface/snippets/viewers/Snippet063ComboViewer.java
jeandersonbc commented 10 years ago

Patch https://git.eclipse.org/r/#/c/16539/ fully merged. Tests running fine.

jeandersonbc commented 10 years ago

Patch https://git.eclipse.org/r/#/c/16488/ fully merged. Conflicts solved:

  1. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/BaseLabelProvider.java
  2. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ContentViewer.java
  3. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/DecoratingLabelProvider.java
  4. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/DecoratingStyledCellLabelProvider.java
  5. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/DecorationContext.java
  6. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/DelegatingStyledCellLabelProvider.java
  7. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeColumnViewerLabelProvider.java
  8. bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ViewerColumn.java

@hendrikstill, I would like your opinion. First, I didn't have any problem to merge this specific patch. However, it seems that after merging, the API changes although it is backwards compatible. Because of this, Eclipse suggests to update 3.10.qualifier to 3.11.qualifier.

More precisely, we have the following:

public class StructuredViewerInternals {
...
protected static void setAssociateListener(StructuredViewer viewer,
            AssociateListener listener) { ... }
protected static Widget[] getItems(StructuredViewer viewer, Object element) { ... }
...
}
public class StructuredViewerInternals {
...
protected static <E,I> void setAssociateListener(StructuredViewer<E,I> viewer,
            AssociateListener listener) {...}
protected static <E,I> Widget[] getItems(StructuredViewer<E,I> viewer, E element) { ... }
...
}

Tests runs fine after changes.

In my opinion, this should not be a problem since it is a minor change in the API. What do you think?

See the last commit at https://github.com/jeandersonbc/eclipse.platform.ui I will wait for your feedback to make it easier to revert this change if necessary.

Thanks

PS.: This issue was caused in an early commit ("Patch https://git.eclipse.org/r/#/c/16409/3 fully merged. Tests running fine.") due to a new constructor and method in ArrayContentProvider. I didn't noticed before because running tests doesn't check for bundle id.

hendrikstill commented 10 years ago

You can find the guidelines for the versioning at http://wiki.eclipse.org/Version_Numbering It says:

The minor segment number must be incremented when a plug-in changes in an "externally visible" way.
Examples of externally visible changes include binary compatible API changes, ...

I would say this is correct. But it's good that you ask that question. The correct versioning is very important to not get in trouble when we try to merge your changes into the master branch. Maybe you could ask on the platform-ui-dev mailing list, to verify this.

jeandersonbc commented 10 years ago

@hendrikstill it seems like there's no problem on updating JFace version id. I will continue to migrate your patches. Thanks :)

hendrikstill commented 10 years ago

Okay cool. I'm happy that the ui-dev guys are so "open minded" about the jface generics. The suggestion from Tom Schindl to make a complete new JFace, was already discussed last year. We decided to contribute to the original JFace API, to avoid that nobody uses the new API. But I think we can keep that suggestion in mind. I guess you already saw how difficult (and sometimes ugly) it is to do not change the API.

jeandersonbc commented 10 years ago

Commit "565e5cb Bug 413973 - [Viewers] Add generics to the TreeViewer" fully merged. Tests running with no problems.

jeandersonbc commented 10 years ago

All patches related to viewers fully merged.