Community
Participate
Working Groups
20070918 Editor templates preference page: It would be good if clicking on a column header would sort the templates by the column. I would like to see all SWT templates together.
Already discussed with Benno. We think a better approach would be to group them using a tree like in the upcoming Templates view. Would that solve your need?
yes sure
I believe this is a useful feature. Especially when you are searching for templates and when you have lots of them.
Just hit this problem. Tree or not, I heartily support this enhancement request.
*** Bug 380115 has been marked as a duplicate of this bug. ***
Created attachment 242255 [details] Implementation for sorting columns in template preference page +1 for this enhancement. Although I don't know the current state of the tree implementation, I have implemented the sorting functionality for templates anyways. I would be willing to push it to gerrit also, if the patch is reviewed and approved.
Created attachment 242257 [details] Screenshot of Template Preference Page
Please add an explicit CoO sign-off comment here. http://wiki.eclipse.org/Development_Resources/Contributing_via_Git#via_Bugzilla (In reply to Nicolaj Hoess from comment #6) > Created attachment 242255 [details] [diff] > Implementation for sorting columns in template preference page The proposed patch sorts the columns. Here are some suggestion for improvement: 1. The instance variable "fViewerComparator" should be made local to #createContents and passed to TemplateColumnSelectionAdapter. 2. TemplateViewerComparator#compare, consider using ((TemplateLabelProvider)labelProvider).getColumnText(e1, fSortColumn); to get the values from the table and comparing rather than using the switch-case statement. Refer NewKeysPreferencePage#compare for implementation details. 3. While updating copyright year, overwrite the last modified year with current year. e.g. Copyright (c) 2000, 2014 4. While modifying the header comment follow the template: Your Name <email@example.com> - Bug Title - https://bugs.eclipse.org/BUG_NUMBER Bug title and url is missing in the submitted patch.
Created attachment 244199 [details] Improved implementation for sorting columns in template pref page This contribution complies with http://www.eclipse.org/legal/CoO.php Mathew, I really appreciate your qualified feedback. I have applied all the suggested changes in the attached patch. As mentioned in comment #6 I am also willed to push the changes to gerrit, if that would save you some work ;)
The patch looks good. Few minor modifications: 1. Mark the private class TemplateViewerComparator as "final" and remove "static" modifier. 2. Mark all instance variables within TemplateColumnSelectionAdapter as "final". The above modifications can be done before committing the code and hence i don't see the need for an updated patch. Thanks.
I applied the changes and pushed to gerrit master: https://git.eclipse.org/r/#/c/28482/ Unfortunately the build failed with the following error message: [...] [INFO] o.h.m.e.h.MavenExecutionResultHandler - [1] org.apache.maven.InternalErrorException: Internal error: java.lang.RuntimeException: Failed to resolve target definition /home/hudson/genie.eclipse.platform/.hudson/jobs/eclipse.platform.text-Gerrit/workspace/.maven/repo/org/eclipse/eclipse-sdk-prereqs/4.4.0-SNAPSHOT/eclipse-sdk-prereqs-4.4.0-SNAPSHOT.target [DEBUG] Closing connection to remote [ERROR] Internal error: java.lang.RuntimeException: Failed to resolve target definition /home/hudson/genie.eclipse.platform/.hudson/jobs/eclipse.platform.text-Gerrit/workspace/.maven/repo/org/eclipse/eclipse-sdk-prereqs/4.4.0-SNAPSHOT/eclipse-sdk-prereqs-4.4.0-SNAPSHOT.target: Failed to load p2 metadata repository from location http://download.eclipse.org/egit/updates-3.1: No repository found at http://download.eclipse.org/egit/updates-3.1. -> [Help 1] [...] I already tried to restart the build (pushed the commit again) and it failed at the same point. To be complete: In a final check before pushing the changes I discovered a problem in the sorting. The implementation does the sorting using String#compareTo(..) (line 1567), therefore the sort order ended up like this: A < B < a < b. In my opinion this is not the expected result so I changed it to String#compareToIgnoreCase(..). This results in A = a < B = b which IS the expected sort order. If this was an undesired change I will upload another patch set ofc.
(In reply to Nicolaj Hoess from comment #11) > /home/hudson/genie.eclipse.platform/.hudson/jobs/eclipse.platform.text- > Gerrit/workspace/.maven/repo/org/eclipse/eclipse-sdk-prereqs/4.4.0-SNAPSHOT/ > eclipse-sdk-prereqs-4.4.0-SNAPSHOT.target: Failed to load p2 metadata > repository from location http://download.eclipse.org/egit/updates-3.1: No > repository found at http://download.eclipse.org/egit/updates-3.1. -> [Help 1] > [...] Looks like the egit p2 site is missing some stuff. Opened Bug 437389 PW
(In reply to Paul Webster from comment #12) > (In reply to Nicolaj Hoess from comment #11) > > /home/hudson/genie.eclipse.platform/.hudson/jobs/eclipse.platform.text- > > Gerrit/workspace/.maven/repo/org/eclipse/eclipse-sdk-prereqs/4.4.0-SNAPSHOT/ > > eclipse-sdk-prereqs-4.4.0-SNAPSHOT.target: Failed to load p2 metadata > > repository from location http://download.eclipse.org/egit/updates-3.1: No > > repository found at http://download.eclipse.org/egit/updates-3.1. -> [Help 1] > > [...] > > Looks like the egit p2 site is missing some stuff. Opened Bug 437389 > > PW Something seems broken here. Text has no relation to EGit, so why should it want to load this?
(In reply to Dani Megert from comment #13) > > Something seems broken here. Text has no relation to EGit, so why should it > want to load this? The gerrit auto-build sets up the CBI build target platform and includes the latest I build in it (by using the eclipse.platform.releng.aggregator/eclipse.platform.releng.prereqs.sdk/eclipse-sdk-prereqs.target), and then builds all of the bundles in the text repo. EGit appears to have broken their 3.1 update site which is referenced from the sdk-prereqs target platform, and that shows up in the Gerrit build. I've opened a bug against EGit to fix their site, and the other option is for David to change the sdk target platform to use another egit site. PW
(In reply to Paul Webster from comment #14) > (In reply to Dani Megert from comment #13) > > > > Something seems broken here. Text has no relation to EGit, so why should it > > want to load this? > > The gerrit auto-build sets up the CBI build target platform and includes the > latest I build in it Right, but neither Text nor the SDK has any relationship to EGit.
(In reply to Dani Megert from comment #15) > > Right, but neither Text nor the SDK has any relationship to EGit. Maybe it was an artifact of the way the Juno build evolved. I've opened Bug 437400 PW
(In reply to Dani Megert from comment #15) > Right, but neither Text nor the SDK has any relationship to EGit. For the records: the Releng tools refer to EGit.
(In reply to Nicolaj Hoess from comment #11) > To be complete: In a final check before pushing the changes I discovered a > problem in the sorting. The implementation does the sorting using > String#compareTo(..) (line 1567), therefore the sort order ended up like > this: A < B < a < b. In my opinion this is not the expected result so I > changed it to String#compareToIgnoreCase(..). This results in A = a < B = b > which IS the expected sort order. > > If this was an undesired change I will upload another patch set ofc. I also agree with you that in the final result we have A = a < B = b. I had a look at the implementation in NewKeysPreferencePage#compareColumn and i think we better use getComparator().compare(left, right); which internally make use of the collators from java.text package to allow locale-sensitive ordering.
Created attachment 244448 [details] Nicolaj's patch with minor modification This patch contains all of Nicolaj's work plus the changes suggested in comment 10 and comment 18. @Dani: Please see if the patch can be released once the 'master' branch is open.
(In reply to Manju Mathew from comment #19) > Created attachment 244448 [details] [diff] > Nicolaj's patch with minor modification > > This patch contains all of Nicolaj's work plus the changes suggested in > comment 10 and comment 18. > @Dani: Please see if the patch can be released once the 'master' branch is > open. Does it work as expected and is the fix OK?
(In reply to Dani Megert from comment #20) > Does it work as expected and is the fix OK? I have reviewed and tested the patch and it looks good. With this patch user will be able to sort all the columns of the "Templates" preference page.
Released patch with http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=86acb41b790e5680793363b7e7f51da044190efa
