Community
Participate
Working Groups
We should update the JFace snippets to show the use of the generified viewers.
Can we do that in a branch - and switch once 4.4 is released? If we push it to master all people copying the code will be broken because 4.3 is not generified?
Sure! Are there any naming conventions for branches?
(In reply to comment #1) > Can we do that in a branch - and switch once 4.4 is released? If we push it > to master all people copying the code will be broken because 4.3 is not > generified? I'm not sure I understand. master will match what's in JFace's master branch (and all of the Milestone builds), and we want the snippets to evolve in lock-step with the JFace changes. If people want Kepler compatible snippets they have to use the R4_3 tag or R4_3_maintenance branch, no? PW
The problem is that they are all linked from the wiki - and people coming using the snippets are often not the hardcore devs who are able to check out stuff from the the git-repo
(In reply to comment #4) > The problem is that they are all linked from the wiki - and people coming > using the snippets are often not the hardcore devs who are able to check out > stuff from the the git-repo The wiki links can be updated to include the R4_3 tag if that's what someone wants. PW
This sounds good, I could do that.
As an example, http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/examples/org.eclipse.jface.snippets/Eclipse%20JFace%20Snippets/org/eclipse/jface/snippets/dialogs/Snippet058VistaProgressBars.java?id=R4_3 I'd prefer this than trying to have generification in the main branch in JFace but in a side branch in JFace snippets, since that would preclude building them in our build. PW
+1 for changing the wiki, should be easy to update it again after the Eclipse 4.4 release with search and replace.
I've updated the JFace Snippets Wiki with the following regex: Search: (http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/examples/org.eclipse.jface.snippets/Eclipse%20JFace%20Snippets/org/eclipse/jface/snippets/.*.java) Replace: $1?id=R4_3
I've uploaded a patch for the ComboViewer Snippet(Snippet063ComboViewer.java) https://git.eclipse.org/r/#/c/15243/
(In reply to comment #10) > I've uploaded a patch for the ComboViewer Snippet(Snippet063ComboViewer.java) > https://git.eclipse.org/r/#/c/15243/ Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=5ccf110dcd23225e36764b336e3a6b1ab8987f08 PW
*** Bug 417582 has been marked as a duplicate of this bug. ***
I plan to update the JFace snippets to the latest state of the JFace Generics work. I also plan to general clean-up of the snippets: - Align copyright notes (move all to the header) - Remove unnecessary casts - Clean-up unnecessary whitespace - Fix / remove incorrect Javadoc - Remove unnecessary imports - Get rid of generic warning - Remove unnecessary else statements - Replace (non-Javadoc) with @Override whenever possible All in all only trivial changes.
Also the Snippet032TableTreeViewer should be deprecated as TableTreeViewer is deprecated
I plan to clean-up the snippets in master. We can afterwards do the generification via an additional bug, I think that will make the review easier.
First round of minor cleanups https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e08278a5a40d59472fb3e9773a6496e9ee07cd6d
More trivial changes https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e87fdf2a88174651d15643dc17f69324d89175f0 https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=56baf71c47432dbacee1e55e5f2ef681af4f5d42
More cleanings to remove unnecessary comments, warnings, and performed simple refactorings to make code easier to understand: https://git.eclipse.org/r/25759
It took a while to understand this code. Motivated by my problem, I thought the code could be improved to make it easier to understand and modify it later: https://git.eclipse.org/r/25762.
After talking to Lars and Hendrik, we concluded that we should remove out-dated snippets. In case someone want's to see any excluded snippet, they can browser the Project UI repository. This information should be added in the wiki page. Patch: https://git.eclipse.org/r/#/c/25762/3
(In reply to Jeanderson Candido from comment #20) > Patch: https://git.eclipse.org/r/#/c/25762/3 Merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=97cd8edf53792b5e6142ae90b93ed85b195259ab Please update the https://wiki.eclipse.org/JFaceSnippets wiki page
More cleanups from Jeanderson https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=ff0c4061d5e4ca4cf9a0cd02f8beb55c25060f67
At the moment I think we have no open Gerrit reviews. Any new ones should be added here.
https://git.eclipse.org/r/26032 merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a2376cc939925ac38b1f5529fa1b9025b774a703 for Snippet010OwnerDraw
Minor refactorings on 058 and 059: https://git.eclipse.org/r/#/c/26058/
More cleanups https://git.eclipse.org/r/26062
(In reply to Jeanderson Candido from comment #25) > Minor refactorings on 058 and 059: https://git.eclipse.org/r/#/c/26058/ Applied with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d8eeae022b7e7ada61ac7c54a61a837816193fae
(In reply to Jeanderson Candido from comment #26) > More cleanups https://git.eclipse.org/r/26062 Merged with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8019a4017d46e103df55d599611a943935c056f2
More cleanups: https://git.eclipse.org/r/#/c/26078/
MyLabelProvaider is a nested class that inherits LabelProvider. There's a version duplicated into snippets 045, 027, and 045. Another version is duplicated into 036 and 044. Is this the case which we could move them out (e.g. MyLabelProviderType1 and MyLabelProviderType2) and delete nested classes from their snippets?
Cleanups for snippet 031 and 047: https://git.eclipse.org/r/26085
(In reply to Jeanderson Candido from comment #29) > More cleanups: https://git.eclipse.org/r/#/c/26078/ Thanks. Merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9ad2c3d502297ea40ba62f339111f786970e29b8
(In reply to Jeanderson Candido from comment #31) > Cleanups for snippet 031 and 047: https://git.eclipse.org/r/26085 Merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e2c23633b1f8ebcd26906fa9685c417d32e6511e
More cleanings for Snippet 039, 040, 041, 043: https://git.eclipse.org/r/#/c/26248/
The following snippets demonstrates deprecated API (CellEditor in 3.2, for 3.3 and above it's recommended to use the EditingSupport class): 034 - https://git.eclipse.org/r/#/c/26249/ 036 - https://git.eclipse.org/r/#/c/26250/ 038 - https://git.eclipse.org/r/#/c/26251/ I think we should explicit deprecate CellEditor since EditingSupport is newer and recommended to edit cells. If everyone accept this change, I will update the wiki page. Thanks.
I suggest to open a separate bug for the deprecation of CellEditor. As long as this does not happen we must maintain the snippets. Maybe the snippets for the old API can be combined?
(In reply to Lars Vogel from comment #36) > I suggest to open a separate bug for the deprecation of CellEditor. As long > as this does not happen we must maintain the snippets. Maybe the snippets > for the old API can be combined? Deprecating CellEditor allow us to delete those out-dated snippets? In this case, do you mind opening an issue for depreaction of CellEditor? I could work on that. Considering the purpose of this current issue (cleaning JFace snippets) and the fact that we already deleted a very old snippet, I think we should be consistent and delete the mentioned snippets. Finally, by combining snippets for old API, you mean a separated package for them?
(In reply to Jeanderson Candido from comment #37) > Deprecating CellEditor allow us to delete those out-dated snippets? In this > case, do you mind opening an issue for depreaction of CellEditor? I could > work on that. I expect that lots of people will object in deprecating CellEditor, AFAIK it is still widely use. Please open a bug report on this and see what the initial reaction will be. This will only be possible for Eclipse 4.5. if at all. > Considering the purpose of this current issue (cleaning JFace snippets) and > the fact that we already deleted a very old snippet, I think we should be > consistent and delete the mentioned snippets. As long as CellEditor is valid API (not deprecated) we should not delete the snippets. The other snippet was a different case, it demonstrated something which was a workaround for missing API. > Finally, by combining snippets for old API, you mean a separated package for > them? No, I don't mean that. I did not look at the snippets but in case they demonstrate almost the same functionality we could have one snippet showing all uses cases. This would allow us to delete two of them. As CellEditor is not "recommended" I think that would be OK. But again, I did not look at the snippets, so you have to review them and decide on this.
(In reply to Lars Vogel from comment #38) > (In reply to Jeanderson Candido from comment #37) > > Deprecating CellEditor allow us to delete those out-dated snippets? In this > > case, do you mind opening an issue for depreaction of CellEditor? I could > > work on that. > > I expect that lots of people will object in deprecating CellEditor, AFAIK it > is still widely use. Please open a bug report on this and see what the > initial reaction will be. This will only be possible for Eclipse 4.5. if at > all. > > > Considering the purpose of this current issue (cleaning JFace snippets) and > > the fact that we already deleted a very old snippet, I think we should be > > consistent and delete the mentioned snippets. > > As long as CellEditor is valid API (not deprecated) we should not delete the > snippets. The other snippet was a different case, it demonstrated something > which was a workaround for missing API. > > > Finally, by combining snippets for old API, you mean a separated package for > > them? > > No, I don't mean that. I did not look at the snippets but in case they > demonstrate almost the same functionality we could have one snippet showing > all uses cases. This would allow us to delete two of them. As CellEditor is > not "recommended" I think that would be OK. But again, I did not look at the > snippets, so you have to review them and decide on this. Changes have been abandoned but I will prepare a bug report for CellEditor deprecation.
We are discussing about CellEditor here: https://bugs.eclipse.org/bugs/show_bug.cgi?id=434574
More cleanings for snippets 25, 26, 27, 29, 31, 34, 36, and 38: https://git.eclipse.org/r/#/c/26415/
Cleanings for snippet 19, 21, and 24: https://git.eclipse.org/r/#/c/26470/
(In reply to Jeanderson Candido from comment #42) > Cleanings for snippet 19, 21, and 24: https://git.eclipse.org/r/#/c/26470/ Merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e5524df50f148dea4262b507fa9db024d11cd0d1
(In reply to Jeanderson Candido from comment #42) > Cleanings for snippet 19, 21, and 24: https://git.eclipse.org/r/#/c/26470/ Thanks. Merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=7ea3d826a45b3b678c3ba33ba63e8b1d367a0c8d
Another merge https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=ec5950daa0f4b262d4e5ba10699b01312269c167
Another merge https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9a64562661ad59e72f9bcf50281917a2f0750beb
With this patch we removed almost all warnings from the snippets: https://git.eclipse.org/r/26951 There are only 2 snippets with warnings due to a deprecated method (not included in this patch).
(In reply to Jeanderson Candido from comment #47) > With this patch we removed almost all warnings from the snippets: > https://git.eclipse.org/r/26951 > > There are only 2 snippets with warnings due to a deprecated method (not > included in this patch). Merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=acf906a2df9430e05abbf91fe2c6ab38e0cca939 I think we can close this bug. In case we want to do more work in the snippets, we should open a new bug. Thanks a lot Jeanderson for your contributions.
.
