Re: PCP Updates: RHBZ 1348349: pmchart(1): Table of metrics, instances and values

Heya Dave!

----- Original Message -----
> On 10/18/2016 07:37 PM, Nathan Scott wrote:
> > I think this should do it:
> > - you can find the tab at the top of the stack via PmChart::activeTab()
> > - with that, you can find the active chart using Tab::currentGadget()
> > - the Chart class inherits from Gadget, so can test on equality with the
> >    current gadget to find the selected chart I think.
> >
> > You might need to take special care with the order of event handling,
> > since you're also working with the same event that changes the current
> > selected chart, maybe (i.e. clicking on a chart).  Or it might Just Work,
> > as-is.
> Nope --- didn't work. The chart always seems to be the current one when
> the "selected" signal is received. I left the code to test for this in
> so that you can have a look.

OK, got it - its a subtle little bug in the way the code deals with the
across-charts picker.  Fix incoming, but basically we were activating
each chart previously instead of just the one - and we didn't notice it
because it happens so quickly & always ends up on the right one.  Heh.

When you merge, take care with the ordering of "my.tab->setCurrent(this);"
in Chart::selected() so that showInfo() is called after setting current.

> > Looks good to me.  Resizing working nicely too - one thing that might
> > help with the final row (and its values) being "oversized" could be to
> > insert an extra, empty row at the end?  That way the vertical resize
> > (which isn't helpful for this dialog really) resizes that empty row
> > while all the data rows would look consistently sized.
> Done.

*nod* - looks nice.

> > Not sure why, but the Hostname column is always reporting "unknown" for
> > me for all values.  Gadget::hosts() might be worth perusing in terms of
> > other code that extracts hostnames from individual metrics.
> I was able to extract the host name directly from the each chart item by
> making the ChartItem::hostname() method public.
> 
> Dave

And that's working now for me too - thanks!

The only other thing I can suggest is a little code refactoring.  I notice
the code implementing the new dialog is inline in chart.cpp - most of that
code looks like it belongs in the separate metricsdialog.{h,cpp}.  That'd
require the passing of a "Chart*" into some of the MetricsDialog routines,
and it'll keep those independent concerns a bit more separated.

Have a look at chartdialog.{h,cpp} as an example of that pattern.  For
reference that's the dialog that gets popped up if you select "New Chart"
or "Edit Chart" from the main menu.

ChartDialog is also an example of having a single dialog for all charts,
instead of one each, if you still want to explore that.  Notice also the
the way dialog pointers are maintained "globally" in pmchart.{h,cpp} - up
to you though of course.

Make a call on those minor things, and then merge it I reckon.

cheers.

--
Nathan

-=-=-=-=-=-=-=-=-=-=-=-
pcp mailing list
[email protected]
https://groups.io/g/pcp/messages
-=-=-
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#14567): https://groups.io/g/pcp/message/14567
View All Messages In Topic (8): https://groups.io/g/pcp/topic/2881991
Mute This Topic: https://groups.io/mt/2881991?uid=174580
New Topic: https://groups.io/g/pcp/post

Change Your Subscription: https://groups.io/g/pcp/editsub?uid=174580

Group Home: https://groups.io/g/pcp
Contact Group Owner: [email protected]

Terms of Service: https://groups.io/static/tos

Unsubscribe: https://groups.io/g/pcp/leave/354243/563757577/xyzzy
-=-=-=-=-=-=-=-=-=-=-=-