Community
Participate
Working Groups
With the recent addition of some awesome session grouping by Jacek, the log view is getting more usable. Another neat addition we can add is the ability to group by various things (similar to the Problems view). I suggest: Group By - None - Session to start :)
*** Bug 208642 has been marked as a duplicate of this bug. ***
*** Bug 208638 has been marked as a duplicate of this bug. ***
tagging bugday.... Jacek ;P?
Created attachment 82406 [details] mylyn/context/zip
Created attachment 82979 [details] refactoring I refactored a bit current view to separate LogSession from it's grouping behaviour. All the functionality remains the same, and next I'm going to build grouping support on this. The patch is not for commiting :)
Created attachment 83056 [details] first try This one is working, so you can give it a try :) - go to Filters and there choose one of Group by: none, session, or plugin. There are still some errors and testing needed.
Beautiful work Jacek! It needs some polish, but it's almost there. btw, there were compile errors in the patch ;o! Severity and Description Path Resource Location Creation Time Id The field AbstractEntry.parent is not visible org.eclipse.ui.views.log/src/org/eclipse/ui/internal/views/log LogEntry.java line 43 1195246773171 52285 The field AbstractEntry.parent is not visible org.eclipse.ui.views.log/src/org/eclipse/ui/internal/views/log LogEntry.java line 44 1195246773171 52286
Oh also, instead of putting it in the filter dialog, can you move the Group By to be in the chevron? Similar to how it is done in the Problems view? It would be on top of Filters... so the menu would look like this: Group By-> ----------- Filters... ----------- Activate on new events Show text filter Thanks Jacek!
In AbstractEntry, why do you call List.add(0, Object)? Not that this is wrong, but if we always add an object to the front of a list, we should consider using a LinkedList or something else. When you insert an element at the beginning of an ArrayList, all the other elements have to be shuffled back one :) If we don't need to insert the children at the beginning of the list, we should get the best performance out of using an ArrayList. Also, you might consider moving AbstractEntry.addChildren(Child, int) to LogSession. This function seems to be pretty specific to the LogSession object (I could be wrong, I just took a quick glance). When you do this, you will have to make the 'children' List protected, but that should not be a big deal. I love the functionality. Adding the plug-in grouping is really slick. I was just hoping to turn session on and off, but this definitely takes it another step up! Thanks Jacek!
Guys thanks for comments. I'm happy you like this stuff! (In reply to comment #7) Regarding not compiling - I must have missed LogEntry.java as it doesn't have any references to parent field now. (In reply to comment #8) Regarding Group by in menu - cool to have it better visible :) btw, think of other ways of grouping, as mechanism is quite flexible now. A friend suggested me to do a group by Exception.. (In reply to comment #9) > In AbstractEntry, why do you call List.add(0, Object)? Not that this is wrong, > but if we always add an object to the front of a list, we should consider using > a LinkedList or something else. When you insert an element at the beginning of > an ArrayList, all the other elements have to be shuffled back one :) If we > don't need to insert the children at the beginning of the list, we should get > the best performance out of using an ArrayList. I think that was originally - before the patch, as new entries are added to the top of the list, to appear at the top of the view.. I'll revise this. > Also, you might consider moving AbstractEntry.addChildren(Child, int) to > LogSession. This function seems to be pretty specific to the LogSession object > (I could be wrong, I just took a quick glance). When you do this, you will > have to make the 'children' List protected, but that should not be a big deal. I'm affraid that will make limit not work if entries will be grouped by Plug-in and limited to up to 10 last entries. On the other hand, it would be nice not to cut subentries of an entry. So maybe instead method should go to Group instead. So generally my intention was to have Group - a container of entries, LogEntry container of subentries and status data, LogSession container of entries and session data. LogSession and Group are somehow overlapping but one is for representation and other for logic, so maybe I'll still have to sort it out...
Created attachment 83136 [details] first try this one compiles against HEAD, but it's nothing new. (earlier I haven't updated after bug 209474)
Created attachment 84099 [details] patch Have a look at this - final patch! Last few days I was testing it in all ways. As it's not as small as usual :), we can review it together on ST or phone call. What took me most time: - more flexible model (AbstractEntry - for tree structure, Group - for groups, LogSession - group with session information, LogEntry - abtstract entry with additional information) - making sure that Prev/Next buttons work in EventDetailsDialog - making sure other filters still work (Most recent session, events limit)
Thanks Jacek, I will review on my plane ride and try to get it in for 3.4M4.
Nice patch, it works great minus a few issues I came across. Once these issues are handled, I think we can release this elegant patch :) 1) You shouldn't be able to get to an Event Details dialog on things like a "Session" or a "Plug-in" (ie., groups). Currently, you can just double click on a session and have one popup or navigate to it from a valid log entry. 2) IAE java.lang.IllegalArgumentException: Argument cannot be null at org.eclipse.swt.SWT.error(SWT.java:3700) at org.eclipse.swt.SWT.error(SWT.java:3634) at org.eclipse.swt.SWT.error(SWT.java:3605) at org.eclipse.swt.widgets.Widget.error(Widget.java:441) at org.eclipse.swt.widgets.Text.setText(Text.java:1848) at org.eclipse.ui.internal.views.log.EventDetailsDialog.updateProperties(EventDetailsDialog.java:361) at org.eclipse.ui.internal.views.log.EventDetailsDialog.resetSelection(EventDetailsDialog.java:316) at org.eclipse.ui.internal.views.log.EventDetailsDialogAction.resetSelection(EventDetailsDialogAction.java:62) at org.eclipse.ui.internal.views.log.LogView$13.selectionChanged(LogView.java:522) at org.eclipse.jface.viewers.Viewer$2.run(Viewer.java:162) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.core.runtime.Platform.run(Platform.java:857) at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:46) at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:199) at org.eclipse.jface.viewers.Viewer.fireSelectionChanged(Viewer.java:160) at org.eclipse.jface.viewers.StructuredViewer.updateSelection(StructuredViewer.java:2044) at org.eclipse.jface.viewers.StructuredViewer.setSelection(StructuredViewer.java:1638) at org.eclipse.jface.viewers.TreeViewer.setSelection(TreeViewer.java:1095) at org.eclipse.jface.viewers.Viewer.setSelection(Viewer.java:392) at org.eclipse.ui.internal.views.log.EventDetailsDialog.setEntrySelectionInTable(EventDetailsDialog.java:333) at org.eclipse.ui.internal.views.log.EventDetailsDialog.nextPressed(EventDetailsDialog.java:255) at org.eclipse.ui.internal.views.log.EventDetailsDialog.buttonPressed(EventDetailsDialog.java:205) at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:623) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:227) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:952) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3745) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3356) at org.eclipse.jface.window.Window.runEventLoop(Window.java:825) at org.eclipse.jface.window.Window.open(Window.java:801) at org.eclipse.ui.internal.views.log.EventDetailsDialog.open(EventDetailsDialog.java:170) at org.eclipse.ui.internal.views.log.EventDetailsDialogAction.run(EventDetailsDialogAction.java:90) at org.eclipse.ui.internal.views.log.LogView$14.doubleClick(LogView.java:528) at org.eclipse.jface.viewers.StructuredViewer$1.run(StructuredViewer.java:799) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.core.runtime.Platform.run(Platform.java:857) at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:46) at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:199) at org.eclipse.jface.viewers.StructuredViewer.fireDoubleClick(StructuredViewer.java:797) at org.eclipse.jface.viewers.AbstractTreeViewer.handleDoubleSelect(AbstractTreeViewer.java:1415) at org.eclipse.jface.viewers.StructuredViewer$4.widgetDefaultSelected(StructuredViewer.java:1173) at org.eclipse.jface.util.OpenStrategy.fireDefaultSelectionEvent(OpenStrategy.java:237) at org.eclipse.jface.util.OpenStrategy.access$0(OpenStrategy.java:234) at org.eclipse.jface.util.OpenStrategy$1.handleEvent(OpenStrategy.java:295) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:952) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3745) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3356) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2395) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2359) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2225) at org.eclipse.ui.internal.Workbench$4.run(Workbench.java:468) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:288) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:463) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:106) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:106) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:76) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:362) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:175) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:516) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:456) at org.eclipse.equinox.launcher.Main.run(Main.java:1194) at org.eclipse.equinox.launcher.Main.main(Main.java:1170) 3) Viewing the details of the next event consistently and moving to the last entry usually results in an IOBE java.lang.ArrayIndexOutOfBoundsException: 0 at org.eclipse.ui.internal.views.log.EventDetailsDialog.nextPressed(EventDetailsDialog.java:251) at org.eclipse.ui.internal.views.log.EventDetailsDialog.buttonPressed(EventDetailsDialog.java:205) at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:623) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:227) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:952) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3745) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3356) at org.eclipse.jface.window.Window.runEventLoop(Window.java:825) at org.eclipse.jface.window.Window.open(Window.java:801) at org.eclipse.ui.internal.views.log.EventDetailsDialog.open(EventDetailsDialog.java:170) at org.eclipse.ui.internal.views.log.EventDetailsDialogAction.run(EventDetailsDialogAction.java:90) at org.eclipse.ui.internal.views.log.LogView$14.doubleClick(LogView.java:528)
Created attachment 84161 [details] patch v2 Thanks, 1) yes I did this intentionally with plan to add some info there in future. But ok, now it's disabled. 2) should be good now 3) I can't repro this, I always have next button grayed out when in last element. Anyway added a check to avoid exception there. If you still have the example log which caused that I'd like to see it :)
Created attachment 84162 [details] mylyn/context/zip
Good stuff, however, two more issues: 1) To get the IAE, simply switch the grouping to NONE (or PLUGIN) and simply start using the event details dialog and keep hitting the 'View Details of Next Event" button. You should get it, it's worked on multiple log files for me. 2) We should also update the event details dialog actions of next/previous to be like when there are no groupings. Those buttons get enabled and disabled depending if there are any entries to go through. Currently, if you highlight the first entry PLUGIN OR SESSION, notice how the button for previous entry is still active. We're almost there Jacek.
AD 1) yes I was getting IAE as you described in comment 14 pt. 2), and fixed in patch v2. In comment 15 I meant I can't reproduce IAOBE (pt. 3) in comment 14) AD 2) give me a second :)
Created attachment 84261 [details] patch v3 third time lucky :)
Beautiful work Jacek, I committed everything to head with a couple minor changes. I changed the ordering of how the grouping items are added to the menu (made sure None was on the button to be consistent with other views in Eclipse). Thanks!
Created attachment 84273 [details] mylyn/context/zip
verified on I20071211-0010
