Community
Participate
Working Groups
In the dialog where you can chose the working sets to be displayed in the Package Explorer view, please add a button to sort working sets alphabetically.
Both WorkingSet dialogs available from the package explorer are already sorted alphabetically for with the exception of the first element in one case. Could you take a screencapture to show me what the dialog looks like to you?
I simply mean the one you get when you open the Package Explorer view's menu, then select "Select Working Sets...". The dialog's header says "Check working sets visible in Package Explorer:". You can also add new working sets there, but they won't be sorted in any way (aside from moving then up and down manually). In the Package Explorer view itself, the working sets are also not sorted alphabetically.
Aha, you're referring to the state it gets into after a new set is added and the dialog is still open. Gotcha. The dialog provided by the workbench does insert at the proper point - the JDT specific dialog does not. Redirecting.
Yes, but I wasn't complaining that it not sorts alphabetically automatically, that's fine with me. I'd just like to have a sort button :) But if you guys decide to change it to automatic sorting, that would be fine with me as well. Also, what's a bit funny: When you add a new working set, close the dialog, then reopen it, the new project is no longer the last one in the last (as it used to be before closing the dialog), but it will be the last one of the _selected_ working sets, right before the first working set which is not selected. That's really a bit odd.
+1. I have at least 35 working sets these days, and being able to sort them alphabetically would be great. Nevertheless, I still like the fact that I can also re-arrange my working sets to my heart's content. This is the first comment in over 18 months... I am hoping this can get picked up for inclusion in Eclipse 3.4.
There are two thing here: 1) have an option in the 'Configure Working Set' dialog to keep the PE working sets sorted (except 'Other Projects' which should always be on top) ==> this bug 2) a bug in the 'Working Set Assignments' dialog that gets out of alpha sort order when a new working set is created out of that dialog ==> see new bug 258792
Created attachment 124696 [details] Fixed. Added a new checkbox for sorting all working sets in the configuration dialog and also sort any newly added working sets.
Created attachment 124789 [details] Fixed the conflicts with 190438
Created attachment 124794 [details] copyrights Update
Comment on attachment 124789 [details] Fixed the conflicts with 190438 Missing copyright update.
Comment on attachment 124794 [details] copyrights Update This patch is not good. After I apply it I get warnings because the patch adds private members that are never used: WorkingSetConfigurationDialog.sortWorkingSetList() WorkingSetConfigurationDialog.fIsWorkingSetsSorted Either it is a bug that you don't use them or it is a bug that they are in the patch.
Created attachment 124817 [details] Looks like the last patch was corrupt :( Hope this one works !
Created attachment 124818 [details] Pls take this one.
Please make the patch minimal i.e. don't change things which are not essential to this fix, e.g. don't make field or variables final. The patch has too many changes! Some other issues: - sortWorkingSetList() is a hack. Instead make a subclass of the comparator that handles the 'Other Projects' working set. - the sort code is duplicated. Duplicated code is always bad. You can solve this with your own comparator that can be used at both places - I mentioned that before: only use 'f' prefix for field (fSettings) - store and read the setting in the model and not in the dialog (add an isSortingEnabled() to the dialog and pass the value in the constructor
Created attachment 124950 [details] Added new comparator class , other minor changes.
The patch looks pretty good. Some minor issues: - the comparator has nothing to do with 'search'. Please move it to the working set package - why does the comparator inherit WorkingSetComparator from if he doesn't use any of its code? - "model" means headless (no UI) and hence mentioning "checkbox" in the Javadoc is not good. This also applies to the method/Javadoc in the dialog. Just mention the what the method returns. - delete the outdated and unused constructor
Created attachment 124970 [details] Fixed the minor changes.
Raksha, here some more details on your latest patch: You don't need to sort the list manually in the dialog: simply set the comparator on the table viewer. The comparator should not re-implement the normal working set sorting but only extend org.eclipse.jdt.internal.ui.search.WorkingSetComparator to handle the OthersWorkingSetUpdater.ID. The new WorkingSetModel.isSortingEnabled() has a problem: it does not update the model but required that setActiveWorkingSets is called afterwards. I suggest you get rid of that method and add setActiveWorkingSets(IWorkingSet[], boolean) where you pass in both values. Don't make the public static final IElementComparer COMPARER as this is dangerous if more instances of the model are created. Don't add dialog settings to the model, simply store the sort flag along with the other working set properties (see the TAG_* constants). Please fix the copyright on the new comparator class: * Copyright (c) 2000, 2009 IBM Corporation and others. ==> * Copyright (c) 2009 IBM Corporation and others. Javadoc: <code>true</code> if the sorting is enabled, else <code>false</code> we normally write: <code>true</code> if sorting is enabled, <code>false</code> otherwise
Created attachment 125248 [details] Patch with review changes.
>Javadoc: > <code>true</code> if the sorting is enabled, else <code>false</code> >we normally write: > <code>true</code> if sorting is enabled, <code>false</code> otherwise This is still not fixed. Please make sure that newer patches do not exhibit issues that got previously mentioned. Ah, and please add the (class) Javadoc that's missing. Missing Javadoc is a NO GO. - in WorkingSetModel all keys begin with "TAG". Why did you not stick to that convention? - the following code in WorkingSetModel is broken (can cause NPE): if (isSortingEnabled == null) { fIsSortingEnabled= false; } fIsSortingEnabled= Boolean.valueOf(isSortingEnabled).booleanValue(); - WorkingSetsViewerComparator is to simple for its own CU. Such types should be directly written as anonymous class where used - WorkingSetConfigurationDialog: the else code is useless: } else { fTableViewer.setComparator(null); } - I have no clue why you added the code to WorkingSetConfigurationDialog.createWorkingSet(). In my opinion this is not needed at all. - there's no need to set the input again when changing a comparator
Raksha, did you ever test your last patch? Upon exit you will always get the following exception because the key contains invalid characters (spaces): !ENTRY org.eclipse.ui.workbench 4 2 2009-02-11 14:33:43.916 !MESSAGE Problems occurred when invoking code from plug-in: "org.eclipse.ui.workbench". !STACK 0 org.w3c.dom.DOMException: INVALID_CHARACTER_ERR: An invalid or illegal XML character is specified. at com.sun.org.apache.xerces.internal.dom.CoreDocumentImpl.createAttribute(CoreDocumentImpl.java:554) at com.sun.org.apache.xerces.internal.dom.ElementImpl.setAttribute(ElementImpl.java:495) at org.eclipse.ui.XMLMemento.putBoolean(XMLMemento.java:438) at org.eclipse.jdt.internal.ui.workingsets.WorkingSetModel.saveState(WorkingSetModel.java:398) at org.eclipse.jdt.internal.ui.packageview.PackageExplorerPart.saveState(PackageExplorerPart.java:928) at org.eclipse.ui.internal.ViewFactory$1.run(ViewFactory.java:329) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.ui.internal.ViewFactory.saveViewState(ViewFactory.java:311) at org.eclipse.ui.internal.ViewFactory.saveState(ViewFactory.java:290) at org.eclipse.ui.internal.WorkbenchPage.saveState(WorkbenchPage.java:3395) at org.eclipse.ui.internal.WorkbenchWindow.saveState(WorkbenchWindow.java:2738) at org.eclipse.ui.internal.Workbench.saveState(Workbench.java:2427) at org.eclipse.ui.internal.Workbench.recordWorkbenchState(Workbench.java:2039) at org.eclipse.ui.internal.Workbench.access$11(Workbench.java:2036) at org.eclipse.ui.internal.Workbench$15.run(Workbench.java:889) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.ui.internal.Workbench.busyClose(Workbench.java:887) at org.eclipse.ui.internal.Workbench.access$15(Workbench.java:844) at org.eclipse.ui.internal.Workbench$23.run(Workbench.java:1088) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ui.internal.Workbench.close(Workbench.java:1086) at org.eclipse.ui.internal.Workbench.close(Workbench.java:1058) at org.eclipse.ui.internal.WorkbenchWindow.busyClose(WorkbenchWindow.java:720) at org.eclipse.ui.internal.WorkbenchWindow.access$0(WorkbenchWindow.java:699) at org.eclipse.ui.internal.WorkbenchWindow$5.run(WorkbenchWindow.java:815) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ui.internal.WorkbenchWindow.close(WorkbenchWindow.java:813) at org.eclipse.jface.window.Window.handleShellCloseEvent(Window.java:741) at org.eclipse.jface.window.Window$3.shellClosed(Window.java:687) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:92) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1003) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1027) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1012) at org.eclipse.swt.widgets.Decorations.closeWidget(Decorations.java:307) at org.eclipse.swt.widgets.Decorations.WM_CLOSE(Decorations.java:1644) at org.eclipse.swt.widgets.Control.windowProc(Control.java:3914) at org.eclipse.swt.widgets.Canvas.windowProc(Canvas.java:342) at org.eclipse.swt.widgets.Decorations.windowProc(Decorations.java:1577) at org.eclipse.swt.widgets.Shell.windowProc(Shell.java:1937) at org.eclipse.swt.widgets.Display.windowProc(Display.java:4588) at org.eclipse.swt.internal.win32.OS.DefWindowProcW(Native Method) at org.eclipse.swt.internal.win32.OS.DefWindowProc(OS.java:2389) at org.eclipse.swt.widgets.Shell.callWindowProc(Shell.java:477) at org.eclipse.swt.widgets.Control.windowProc(Control.java:4002) at org.eclipse.swt.widgets.Canvas.windowProc(Canvas.java:342) at org.eclipse.swt.widgets.Decorations.windowProc(Decorations.java:1577) at org.eclipse.swt.widgets.Shell.windowProc(Shell.java:1937) at org.eclipse.swt.widgets.Display.windowProc(Display.java:4588) at org.eclipse.swt.internal.win32.OS.DefWindowProcW(Native Method) at org.eclipse.swt.internal.win32.OS.DefWindowProc(OS.java:2389) at org.eclipse.swt.widgets.Shell.callWindowProc(Shell.java:477) at org.eclipse.swt.widgets.Control.windowProc(Control.java:4002) at org.eclipse.swt.widgets.Canvas.windowProc(Canvas.java:342) at org.eclipse.swt.widgets.Decorations.windowProc(Decorations.java:1577) at org.eclipse.swt.widgets.Shell.windowProc(Shell.java:1937) at org.eclipse.swt.widgets.Display.windowProc(Display.java:4588) at org.eclipse.swt.internal.win32.OS.DispatchMessageW(Native Method) at org.eclipse.swt.internal.win32.OS.DispatchMessage(OS.java:2394) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3470) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2388) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2352) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2204) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:499) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:492) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:556) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:511) at org.eclipse.equinox.launcher.Main.run(Main.java:1270) at org.eclipse.equinox.launcher.Main.main(Main.java:1246)
Created attachment 125488 [details] Patch with review changes.
Patch is good. I opened two trivial bugs: - bug 264720: remove duplicate creation of the comparator - bug 264722: Get rid of duplicate working set comparators Fixed in HEAD. Available in builds > N20090211-2000.
