Add tooltips for each of the models. #41

nwrogers · 2017-03-30T21:53:57Z

Tooltips include one for each column header and one for each row of
data.

Fixes: #29

milianw · 2017-04-03T08:33:49Z

src/models/callercalleemodel.cpp

+            case Binary:
+                return tr("The name of the executable the symbol resides in. May be empty when debug information is missing.");
+            case SelfCost:
+                return tr("The number of CPU cycles directly used by this symbol while inside the function.");


The number of samples directly attributed to this symbol.

milianw · 2017-04-03T08:34:56Z

src/models/callercalleemodel.cpp

+            case SelfCost:
+                return tr("The number of CPU cycles directly used by this symbol while inside the function.");
+            case InclusiveCost:
+                return tr("The number of CPU cycles used by this symbol both directly and indirectly. The inclusive cost includes the costs of all functions called by this symbol plus it's self cost.");


The number of samples attributed to this symbol, both directly and indirectly. This includes the costs of all functions called by this symbol plus its self cost.

milianw · 2017-04-03T08:35:59Z

src/models/callercalleemodel.cpp

+            case InclusiveCost:
+                return tr("The number of CPU cycles used by this symbol both directly and indirectly. The inclusive cost includes the costs of all functions called by this symbol plus it's self cost.");
+            case Callers:
+                return tr("Callers Tooltip");


remove the "Tooltip" here and below

milianw · 2017-04-03T08:38:03Z

src/models/callercalleemodel.cpp

@@ -107,10 +124,13 @@ QVariant CallerCalleeModel::cell(Columns column, int role, const Data::Symbol& s
        return QVariant::fromValue(entry.callers);
    } else if (role == SourceMapRole) {
        return QVariant::fromValue(entry.sourceMap);
+    } else if (role == Qt::ToolTipRole) {
+        QString toolTip = formatSymbolBinaryString(symbol.symbol, symbol.binary) + QLatin1String("\n") +


please use a template enclosed in tr here, i.e. something like:

tr("%1\ninclusive cost: %2\nself cost: %3").arg(...).arg(...);

milianw · 2017-04-03T08:38:54Z

src/models/callercalleemodel.h

+                case Binary:
+                    return QObject::tr("The name of the executable the symbol resides in. May be empty when debug information is missing.");
+                case Cost:
+                    return QObject::tr("The number of CPU cycles used by this symbol.");


...of samples...

milianw · 2017-04-03T08:46:03Z

src/models/treemodel.cpp

@@ -41,6 +41,25 @@ BottomUpModel::BottomUpModel(QObject* parent)

 BottomUpModel::~BottomUpModel() = default;

+namespace {
+QString formatSymbolBinaryString(const QString& symbol, const QString& binary)


see above, don't duplicate this, share it in a header

milianw · 2017-04-03T08:46:10Z

src/models/treemodel.cpp

+    case Binary:
+        return tr("The name of the executable the symbol resides in. May be empty when debug information is missing.");
+    case Cost:
+        return tr("The number of CPU cycles used by this symbol.");


milianw · 2017-04-03T08:46:30Z

src/models/treemodel.cpp

@@ -69,6 +102,13 @@ QVariant BottomUpModel::displayData(const Data::BottomUp* row, Columns column)
    return {};
 }

+QVariant BottomUpModel::displayToolTip(const Data::BottomUp* row, quint64 sampleCount)
+{
+    QString toolTip = formatSymbolBinaryString(row->symbol.symbol, row->symbol.binary) + QLatin1String("\n") +


tr() template

milianw · 2017-04-03T08:46:41Z

src/models/treemodel.cpp

+    case Binary:
+        return tr("The name of the executable the symbol resides in. May be empty when debug information is missing.");
+    case InclusiveCost:
+        return tr("The number of CPU cycles used by this symbol both directly and indirectly. The inclusive cost includes the costs of all functions called by this symbol plus it's self cost.");


milianw · 2017-04-03T08:46:44Z

src/models/treemodel.cpp

+    case InclusiveCost:
+        return tr("The number of CPU cycles used by this symbol both directly and indirectly. The inclusive cost includes the costs of all functions called by this symbol plus it's self cost.");
+    case SelfCost:
+        return tr("The number of CPU cycles directly used by this symbol while inside the function.");


milianw

some little nitpicks, otherwise lgtm

milianw · 2017-04-04T14:49:21Z

src/models/callercalleemodel.h

+                case Binary:
+                    return QObject::tr("The name of the executable the symbol resides in. May be empty when debug information is missing.");
+                case Cost:
+                    return QObject::tr("The number of CPU samples used by this symbol.");


use the same string as above/below, i.e. no CPU, and "attributed to"

Also mention that this is inclusive cost here

Does this sound good: "The number of samples attributed to this symbol, both directly and indirectly. This is the symbols inclusive cost."?

milianw · 2017-04-04T14:49:57Z

src/models/treemodel.cpp

+    case Binary:
+        return tr("The name of the executable the symbol resides in. May be empty when debug information is missing.");
+    case Cost:
+        return tr("The number of CPU samples used by this symbol.");


dito, no CPU, use "attributed to"

also mention that this is inclusive cost

milianw · 2017-04-04T14:50:30Z

src/models/callercalleemodel.h

+                case Location:
+                    return QObject::tr("The source file name and line number where the cost was measured. May be empty when debug information is missing.");
+                case Cost:
+                    return QObject::tr("The number of samples attributed to this code location.");


make this "...directly attributed to"

milianw · 2017-04-10T08:54:35Z

ping?

milianw · 2017-04-10T14:23:19Z

src/models/callercalleemodel.cpp

+    } else if (role == Qt::ToolTipRole) {
+        switch (column) {
+            case Symbol:
+                return tr("The symbol's function name. May be empty when debug information is missing.");


you are the native speaker, but isn't it "The symbols function name", i.e. without the apostroph?

"The symbol's function name." is correct. The apostrophe is used to show possession. i.e. "This is the function name of the symbol." is equal to "The symbol's function name."

milianw · 2017-04-10T14:24:19Z

src/models/treemodel.cpp

+{
+    switch (column) {
+    case Symbol:
+        return tr("The symbol's function name. May be empty when debug information is missing.");


here again: apostrophe or not?

milianw · 2017-04-10T14:24:37Z

src/models/treemodel.cpp

+{
+    switch (column) {
+    case Symbol:
+        return tr("The symbol's function name. May be empty when debug information is missing.");


apostrophe or not?

milianw · 2017-04-10T14:24:49Z

src/models/treemodel.cpp

+    case Binary:
+        return tr("The name of the executable the symbol resides in. May be empty when debug information is missing.");
+    case Cost:
+        return tr("The symbols inclusive cost, i.e. the number of samples attributed to this symbol, both directly and indirectly.");


note how there's no apostrophe here

This should be "The symbol's inclusive cost"

Tooltips include one for each column header and one for each row of data. Fixes: KDAB#29

milianw

thank you

milianw requested changes Apr 3, 2017

View reviewed changes

nwrogers force-pushed the TooltipsForModels branch 2 times, most recently from 6cdaea7 to ed0cea5 Compare April 3, 2017 15:07

milianw requested changes Apr 4, 2017

View reviewed changes

nwrogers force-pushed the TooltipsForModels branch from ed0cea5 to 0de9bdf Compare April 10, 2017 14:20

milianw requested changes Apr 10, 2017

View reviewed changes

Add tooltips for each of the models.

d8b9a36

Tooltips include one for each column header and one for each row of data. Fixes: KDAB#29

nwrogers force-pushed the TooltipsForModels branch from 0de9bdf to d8b9a36 Compare April 10, 2017 14:51

milianw approved these changes Apr 10, 2017

View reviewed changes

milianw merged commit 89d6d1f into KDAB:master Apr 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tooltips for each of the models. #41

Add tooltips for each of the models. #41

nwrogers commented Mar 30, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw Apr 3, 2017

milianw left a comment

milianw Apr 4, 2017

milianw Apr 4, 2017

nwrogers Apr 10, 2017

milianw Apr 4, 2017

milianw Apr 4, 2017

milianw Apr 4, 2017

milianw commented Apr 10, 2017

milianw Apr 10, 2017

nwrogers Apr 10, 2017

milianw Apr 10, 2017

milianw Apr 10, 2017

milianw Apr 10, 2017

nwrogers Apr 10, 2017

milianw left a comment

Add tooltips for each of the models. #41

Add tooltips for each of the models. #41

Conversation

nwrogers commented Mar 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milianw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milianw commented Apr 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milianw left a comment

Choose a reason for hiding this comment