RFR: JMC-5473: Quick search for Automated Analysis Result Table

[Jie Kang]

On Wed, Aug 28, 2019 at 10:46 AM Jie Kang <jkang at redhat.com> wrote:
>
> Hi all,
>
> Carmine has requested I continue this work in his stead. Please find
> an updated webrev for review below.
>
> Webrev: http://cr.openjdk.java.net/~jkang/jmc-5473/webrev.00/
> Bug: https://bugs.openjdk.java.net/browse/JMC-5473

Ah; I had a mistake in the previous webrev (the TableViewer was no
longer set), please find an updated one below. Apologies for the
mistake.

http://cr.openjdk.java.net/~jkang/jmc-5473/webrev.01/


Regards,

>
>
> Regards
>
>
>
> On Thu, Aug 1, 2019 at 10:24 AM Carmine Vincenzo Russo
> <carusso at redhat.com> wrote:
> >
> > Hi Jie,
> >
> > Thanks for pointing out these errors, I'll fix them and get back with an updated version.
> >
> > Thanks,
> > Carmine
> >
> > On Thu, Aug 1, 2019 at 4:16 PM Jie Kang <jkang at redhat.com> wrote:
> >>
> >> On Thu, Aug 1, 2019 at 6:53 AM Carmine Vincenzo Russo
> >> <carusso at redhat.com> wrote:
> >> >
> >> > Hi,
> >> >
> >> > The attached patch addresses the issue JMC-5473[0] in which a search
> >> > feature for the Automated Analysis Result Table was lacking.
> >> >
> >> > In ResultOverview.java, I added the text component and a simple layout to
> >> > give the page more consistency when the table is displayed.
> >> > I also produced ResultOverviewTest.java to test if the table has
> >> > components, the search feature operates, and two more to check if the
> >> > behaviour is what we expect with a nonsense search and a specific search.
> >> >
> >> > A side note, since there were no tests for the Automated Analysis Result, I
> >> > had to add the tab in JfrUi and allow the record analysis in two other
> >> > classes, OldRecordingsVerificationTest.java and JfrRecordingTest.java,
> >> > because they go through all the tabs listed in JfrUi during their tests.
> >> >
> >> > How does it look?
> >>
> >> Hi Carmine,
> >>
> >> Thanks for doing this; the tests are appreciated! I have some issues
> >> to address below:
> >>
> >> diff --git a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/R
> >> esultOverview.java
> >> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/ov
> >> erview/ResultOverview.java
> >> --- a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultOv
> >> erview.java
> >> +++ b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultOv
> >> erview.java
> >> [...]
> >> -
> >> +
> >>
> >> * Whitespace additions like the example shown above should be removed;
> >> there are others in the patch.
> >>
> >> -                       Map<Result, DataPageDescriptor> map = createResultMap();
> >> +                       GridLayout layout = new GridLayout(1, true);
> >> +                       this.form.getBody().setLayout(layout);
> >> +                       text = new Text(form.getBody(), SWT.BORDER |
> >> SWT.HORIZONTAL | SWT.SEARCH | SWT.RESIZE);
> >> +                       text.setLayoutData(new GridData(SWT.FILL,
> >> SWT.DEFAULT, true, false));
> >> +                       text.setMessage(Messages.ResultOverview_SEARCH_TABLE);
> >> +                       text.setToolTipText(Messages.ResultOverview_SEARCH_BAR);
> >> +                       Map<Result, DataPageDescriptor> map =
> >> createResultMap(text.getText());
> >>                         table = new ResultTableUi(form, toolkit,
> >> editor, loadedState, map);
> >> +                       ModifyListener listener = new ModifyListener() {
> >> +                               @Override
> >> +                               public void modifyText(ModifyEvent e) {
> >> +                                       Map<Result,
> >> DataPageDescriptor> map = createResultMap(text.getText());
> >> +                                       table.updateInput(map);
> >> +                               }
> >> +                       };
> >> +                       text.addModifyListener(listener);
> >>                 }
> >> * The additions to the Table UI done above should occur in the Table
> >> UI code, rather than in the overview which is managing the HTML UI and
> >> the Table UI, ie. the above should be within ResultTableUi.java
> >>
> >> * When switching to the Table and then back to the HTML, the resulting
> >> view is strange; see the linked image [1]. This may have to do with
> >> how the form's layout is modified by your addition. This form is the
> >> parent for the ResultTableUi. I would add children to it with custom
> >> layouts, but I would not modify the 'parent' layout itself.
> >>
> >> https://imgur.com/a/nS1m2T2
> >>
> >>
> >> Regards,
> >>
> >> >
> >> > Regards,
> >> > Carmine
> >> >
> >> > [0] https://bugs.openjdk.java.net/browse/JMC-5473
> >> >
> >> > --
> >> > Carmine Vincenzo Russo
> >
> >
> >
> > --
> > Carmine Vincenzo Russo