Re: [PATCH v2 8/8] kernel-shark: Handle the case when the marker points to a filtered entry

[Slavomir Kaslev <kaslevs@vmware.com>]

On Fri, 2019-04-19 at 07:46 +0000, Yordan Karadzhov wrote:
> 
> On 19.04.19 г. 10:29 ч., Slavomir Kaslev wrote:
> > On Thu, Apr 18, 2019 at 6:23 PM Yordan Karadzhov <
> > ykaradzhov@vmware.com> wrote:
> > > Markers can point to entries that are filtered out. When
> > > switching
> > > the marker it may happen that new Active Marker points to an
> > > entry
> > > that is filtered. In this case no actions must be taken and a
> > > warning
> > > message for the user must be displayed.
> > > 
> > > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> > > ---
> > >   kernel-shark/src/KsTraceViewer.cpp | 27 +++++++++++++++++----
> > > ------
> > >   kernel-shark/src/KsTraceViewer.hpp |  2 ++
> > >   2 files changed, 19 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/kernel-shark/src/KsTraceViewer.cpp b/kernel-
> > > shark/src/KsTraceViewer.cpp
> > > index 04a38b8..85da64f 100644
> > > --- a/kernel-shark/src/KsTraceViewer.cpp
> > > +++ b/kernel-shark/src/KsTraceViewer.cpp
> > > @@ -61,7 +61,8 @@ KsTraceViewer::KsTraceViewer(QWidget *parent)
> > >     _graphFollowsCheckBox(this),
> > >     _graphFollows(true),
> > >     _mState(nullptr),
> > > -  _data(nullptr)
> > > +  _data(nullptr),
> > > +  _em(this)
> > >   {
> > >          this->setSizePolicy(QSizePolicy::Preferred,
> > > QSizePolicy::Maximum);
> > > 
> > > @@ -493,15 +494,21 @@ void KsTraceViewer::markSwitch()
> > >                  QModelIndex index =
> > >                          _proxyModel.mapFromSource(_model.index(r
> > > ow, 0));
> > > 
> > > -               /*
> > > -                * The row of the active marker will be colored
> > > according to
> > > -                * the assigned property of the current state of
> > > the Dual
> > > -                * marker. Auto-scrolling is temporarily disabled
> > > because we
> > > -                * do not want to scroll to the position of the
> > > marker yet.
> > > -                */
> > > -               _view.setAutoScroll(false);
> > > -               _view.selectRow(index.row());
> > > -               _view.setAutoScroll(true);
> > > +               if (index.isValid()) {
> > > +                       /*
> > > +                        * The row of the active marker will be
> > > colored according to
> > > +                        * the assigned property of the current
> > > state of the Dual
> > > +                        * marker. Auto-scrolling is temporarily
> > > disabled because we
> > > +                        * do not want to scroll to the position
> > > of the marker yet.
> > > +                        */
> > > +                       _view.setAutoScroll(false);
> > > +                       _view.selectRow(index.row());
> > > +                       _view.setAutoScroll(true);
> > > +               } else {
> > > +                       _view.clearSelection();
> > > +                       QString err("The marker's entry is
> > > filtered out.");
> > > +                       _em.showMessage(err);
> > 
> > When is this error message going to be cleared? I don't think it
> > changes atm unless a new error comes along.
> 
> The message will be displayed in a small widget (QErrorMessage).
> This 
> widget has an "OK" button and a "Show this message again" checkbox.
> 
> To clear the message you can either press "OK" or close the widget.
> If you uncheck the "Show this message again" checkbox, no message
> will 
> be shown next time when the error occurs.

That makes sense, in that case it lgtm

Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com>

-- Slavi