Handle watching/tooltips of C++ editor tokens consistently.

For editor tooltips and the editor context menu
'Watch expression', always try to find a local variable first
and use its expression.

Change the tooltip manager/widgets not to rely on the debugger
model enum and obscure expression, filter by complete iname
instead. Remove obsolete enumeration.

Change gdb's handling of tooltips such that local variables
are displayed immediately without creating additional tooltip
items.

Change-Id: I9b55823428029ba50d84d3a8cab55eb58942e72b
Reviewed-by: hjk <qthjk@ovi.com>

src/plugins/debugger/cdb/cdbengine.cpp

@@ -404,6 +404,7 @@ void CdbEngine::init()
     m_sourceStepInto = false;
     m_watchPointX = m_watchPointY = 0;
     m_ignoreCdbOutput = false;
+    m_watchInameToName.clear();
 
     m_outputBuffer.clear();
     m_builtinCommandQueue.clear();
@@ -470,23 +471,13 @@ bool CdbEngine::setToolTipExpression(const QPoint &mousePos,
     // Are we in the current stack frame
     if (context.function.isEmpty() || exp.isEmpty() || context.function != stackHandler()->currentFrame().function)
         return false;
-    // No numerical or any other expressions [yet]
-    if (!(exp.at(0).isLetter() || exp.at(0) == QLatin1Char('_')))
+    // Show tooltips of local variables only. Anything else can slow debugging down.
+    const WatchData *localVariable = watchHandler()->findCppLocalVariable(exp);
+    if (!localVariable)
         return false;
-    // Can this be found as a local variable?
-    const QByteArray localsPrefix(localsPrefixC);
-    QByteArray iname = localsPrefix + exp.toAscii();
-    if (!watchHandler()->hasItem(iname)) {
-        // Nope, try a 'local.this.m_foo'.
-        exp.prepend(QLatin1String("this."));
-        iname.insert(localsPrefix.size(), "this.");
-        if (!watchHandler()->hasItem(iname))
-            return false;
-    }
     DebuggerToolTipWidget *tw = new DebuggerToolTipWidget;
     tw->setContext(context);
-    tw->setDebuggerModel(LocalsType);
-    tw->setExpression(exp);
+    tw->setIname(localVariable->iname);
     tw->acquireEngine(this);
     DebuggerToolTipManager::instance()->showToolTip(mousePos, editor, tw);
     return true;
@@ -995,6 +986,10 @@ void CdbEngine::updateWatchData(const WatchData &dataIn,
         QByteArray args;
         ByteArrayInputStream str(args);
         str << dataIn.iname << " \"" << dataIn.exp << '"';
+        // Store the name since the CDB extension library
+        // does not maintain the names of watches.
+        if (!dataIn.name.isEmpty() && dataIn.name != QLatin1String(dataIn.exp))
+            m_watchInameToName.insert(dataIn.iname, dataIn.name);
         postExtensionCommand("addwatch", args, 0,
                              &CdbEngine::handleAddWatch, 0,
                              qVariantFromValue(dataIn));
@@ -1916,6 +1911,15 @@ void CdbEngine::handleLocals(const CdbExtensionCommandPtr &reply)
             dummy.name = QLatin1String(child.findChild("name").data());
             parseWatchData(watchHandler()->expandedINames(), dummy, child, &watchData);
         }
+        // Fix the names of watch data.
+        for (int i =0; i < watchData.size(); ++i) {
+            if (watchData.at(i).iname.startsWith('w')) {
+                const QHash<QByteArray, QString>::const_iterator it
+                    = m_watchInameToName.find(watchData.at(i).iname);
+                if (it != m_watchInameToName.constEnd())
+                    watchData[i].name = it.value();
+            }
+        }
         watchHandler()->insertData(watchData);
         if (debugLocals) {
             QDebug nsp = qDebug().nospace();

src/plugins/debugger/cdb/cdbengine.h

@@ -275,6 +275,7 @@ private:
     PendingBreakPointMap m_pendingBreakpointMap;
     QHash<QString, QString> m_fileNameModuleHash;
     QMultiHash<QString, quint64> m_symbolAddressCache;
+    QHash<QByteArray, QString> m_watchInameToName;
     bool m_ignoreCdbOutput;
     QVariantList m_customSpecialStopData;
     QList<SourcePathMapping> m_sourcePathMappings;

src/plugins/debugger/debuggerplugin.cpp

@@ -1143,7 +1143,12 @@ public slots:
         exp = fixCppExpression(exp);
         if (exp.isEmpty())
             return;
-        currentEngine()->watchHandler()->watchExpression(exp);
+        const QString name = exp;
+        // Prefer to watch an existing local variable by its expression (address) if it can be found.
+        WatchHandler *watchHandler = currentEngine()->watchHandler();
+        if (const WatchData *localVariable = watchHandler->findCppLocalVariable(exp))
+            exp = QLatin1String(localVariable->exp);
+        watchHandler->watchExpression(exp, name);
     }
 
     void handleExecExit()

src/plugins/debugger/debuggertooltipmanager.cpp

@@ -98,8 +98,8 @@ static const char offsetYAttributeC[] = "offset_y";
 static const char engineTypeAttributeC[] = "engine";
 static const char dateAttributeC[] = "date";
 static const char treeElementC[] = "tree";
-static const char treeModelAttributeC[] = "model"; // Locals/Watches
-static const char treeExpressionAttributeC[] = "expression"; // Locals/Watches
+static const char treeExpressionAttributeC[] = "expression";
+static const char treeInameAttributeC[] = "iname";
 static const char modelElementC[] = "model";
 static const char modelColumnCountAttributeC[] = "columncount";
 static const char modelRowElementC[] = "row";
@@ -615,7 +615,6 @@ DebuggerToolTipWidget::DebuggerToolTipWidget(QWidget *parent) :
     m_titleLabel(new DraggableLabel),
     m_engineAcquired(false),
     m_creationDate(QDate::currentDate()),
-    m_debuggerModel(TooltipType),
     m_treeView(new DebuggerToolTipTreeView),
     m_defaultModel(new QStandardItemModel(this))
 {
@@ -836,11 +835,7 @@ void DebuggerToolTipWidget::saveSessionData(QXmlStreamWriter &w) const
 /*!
     \class Debugger::Internal::TooltipFilterModel
 
-    \brief Model for tooltips filtering a local variable using the locals or tooltip model,
-    matching on the name.
-
-    Expressions/names can either be flat ('foo' will match at the root level)
-    or nested ('this.m_foo' will match 'this' at root level and 'm_foo' at level 1).
+    \brief Model for tooltips filtering an item on the watchhandler matching its tree on the iname.
 
     In addition, suppress the model's tooltip data to avoid a tooltip on a tooltip.
 */
@@ -848,9 +843,8 @@ void DebuggerToolTipWidget::saveSessionData(QXmlStreamWriter &w) const
 class TooltipFilterModel : public QSortFilterProxyModel
 {
 public:
-    TooltipFilterModel(QAbstractItemModel *model, const QString &exp, int debuggerModel) :
-        m_expressions(exp.split(QLatin1Char('.'))),
-        m_debuggerModel(debuggerModel)
+    TooltipFilterModel(QAbstractItemModel *model, const QByteArray &iname)
+        : m_iname(iname)
     {
         setSourceModel(model);
     }
@@ -864,27 +858,21 @@ public:
     bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const;
 
 private:
-    const QStringList m_expressions;
-    int m_debuggerModel;
+    const QByteArray m_iname;
 };
 
+static bool isSubIname(const QByteArray &haystack, const QByteArray &needle)
+{
+    return haystack.size() > needle.size()
+           && haystack.startsWith(needle)
+           && haystack.at(needle.size()) == '.';
+}
+
 bool TooltipFilterModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const
 {
     const QModelIndex nameIndex = sourceModel()->index(sourceRow, 0, sourceParent);
-    QByteArray iname = nameIndex.data(LocalsINameRole).toByteArray();
-    if (m_debuggerModel == LocalsType && !iname.startsWith("local"))
-        return false;
-    if (m_debuggerModel == TooltipType && !iname.startsWith("tooltip"))
-        return false;
-    // Match on expression for top level, else pass through.
-    const int depth = iname.count('.');
-    if (depth == 0)
-        return true;
-    if (depth > m_expressions.size())
-        return true;
-    const QString name = nameIndex.data().toString();
-    //const QString exp = nameIndex.data(LocalsExpressionRole).toString();
-    return name == m_expressions.at(depth - 1);
+    const QByteArray iname = nameIndex.data(LocalsINameRole).toByteArray();
+    return iname == m_iname || isSubIname(iname, m_iname) || isSubIname(m_iname, iname);
 }
 
 /*!
@@ -991,7 +979,7 @@ void DebuggerToolTipWidget::doAcquireEngine(DebuggerEngine *engine)
     // Create a filter model on the debugger's model and switch to it.
     QAbstractItemModel *model = engine->watchModel();
     TooltipFilterModel *filterModel =
-            new TooltipFilterModel(model, m_expression, m_debuggerModel);
+            new TooltipFilterModel(model, m_iname);
     swapModel(filterModel);
 }
 
@@ -1000,7 +988,7 @@ QAbstractItemModel *DebuggerToolTipWidget::swapModel(QAbstractItemModel *newMode
     QAbstractItemModel *oldModel = m_treeView->swapModel(newModel);
     // When looking at some 'this.m_foo.x', expand all items
     if (newModel) {
-        if (const int level = m_expression.count(QLatin1Char('.')) + 1) {
+        if (const int level = m_iname.count('.')) {
             QModelIndex index = newModel->index(0, 0);
             for (int i = 0; i < level && index.isValid(); i++, index = index.child(0, 0))
                 m_treeView->setExpanded(index, true);
@@ -1062,8 +1050,9 @@ void DebuggerToolTipWidget::doSaveSessionData(QXmlStreamWriter &w) const
 {
     w.writeStartElement(QLatin1String(treeElementC));
     QXmlStreamAttributes attributes;
-    attributes.append(QLatin1String(treeModelAttributeC), QString::number(m_debuggerModel));
-    attributes.append(QLatin1String(treeExpressionAttributeC), m_expression);
+    if (!m_expression.isEmpty())
+        attributes.append(QLatin1String(treeExpressionAttributeC), m_expression);
+    attributes.append(QLatin1String(treeInameAttributeC), QLatin1String(m_iname));
     w.writeAttributes(attributes);
     if (QAbstractItemModel *model = m_treeView->model()) {
         XmlWriterTreeModelVisitor v(model, w);
@@ -1078,11 +1067,11 @@ void DebuggerToolTipWidget::doLoadSessionData(QXmlStreamReader &r)
         return;
     // Restore data to default model and show that.
     const QXmlStreamAttributes attributes = r.attributes();
-    m_debuggerModel = attributes.value(QLatin1String(treeModelAttributeC)).toString().toInt();
+    m_iname = attributes.value(QLatin1String(treeInameAttributeC)).toString().toLatin1();
     m_expression = attributes.value(QLatin1String(treeExpressionAttributeC)).toString();
     if (debugToolTips)
-        qDebug() << "DebuggerTreeViewToolTipWidget::doLoadSessionData() " << m_debuggerModel << m_expression;
-    setObjectName(QLatin1String("DebuggerTreeViewToolTipWidget: ") + m_expression);
+        qDebug() << "DebuggerTreeViewToolTipWidget::doLoadSessionData() " << m_debuggerModel << m_iname;
+    setObjectName(QLatin1String("DebuggerTreeViewToolTipWidget: ") + QLatin1String(m_iname));
     restoreTreeModel(r, m_defaultModel);
     r.readNext(); // Skip </tree>
     m_treeView->swapModel(m_defaultModel);
@@ -1480,14 +1469,16 @@ void DebuggerToolTipManager::slotTooltipOverrideRequested(ITextEditor *editor,
         qDebug() << "<slotTooltipOverrideRequested() " << currentEngine << *handled;
 }
 
-QStringList DebuggerToolTipManager::treeWidgetExpressions(const QString &fileName,
-                                                          const QString &engineType,
-                                                          const QString &function) const
+
+DebuggerToolTipManager::ExpressionInamePairs
+    DebuggerToolTipManager::treeWidgetExpressions(const QString &fileName,
+                                                  const QString &engineType,
+                                                  const QString &function) const
 {
-    QStringList rc;
+    ExpressionInamePairs rc;
     foreach (const QPointer<DebuggerToolTipWidget> &tw, m_tooltips) {
         if (!tw.isNull() && tw->matches(fileName, engineType, function))
-            rc.push_back(tw->expression());
+            rc.push_back(ExpressionInamePair(tw->expression(), tw->iname()));
     }
     if (debugToolTips)
         qDebug() << "DebuggerToolTipManager::treeWidgetExpressions"

src/plugins/debugger/debuggertooltipmanager.h

@@ -119,8 +119,9 @@ public:
 
     static DebuggerToolTipWidget *loadSessionData(QXmlStreamReader &r);
 
-    int debuggerModel() const { return m_debuggerModel; }
-    void setDebuggerModel(int m) { m_debuggerModel = m; }
+    QByteArray iname() const { return m_iname; }
+    void setIname(const QByteArray &e) { m_iname = e; }
+
     QString expression() const { return m_expression; }
     void setExpression(const QString &e) { m_expression = e; }
 
@@ -166,6 +167,7 @@ private:
 
     int m_debuggerModel;
     QString m_expression;
+    QByteArray m_iname;
 
     DebuggerToolTipTreeView *m_treeView;
     QStandardItemModel *m_defaultModel;
@@ -196,6 +198,9 @@ class DebuggerToolTipManager : public QObject
     Q_OBJECT
 
 public:
+    typedef QPair<QString, QByteArray> ExpressionInamePair;
+    typedef QList<ExpressionInamePair> ExpressionInamePairs;
+
     explicit DebuggerToolTipManager(QObject *parent = 0);
     virtual ~DebuggerToolTipManager();
 
@@ -204,9 +209,9 @@ public:
     bool hasToolTips() const { return !m_tooltips.isEmpty(); }
 
     // Collect all expressions of DebuggerTreeViewToolTipWidget
-    QStringList treeWidgetExpressions(const QString &fileName,
-                                      const QString &engineType = QString(),
-                                      const QString &function= QString()) const;
+    ExpressionInamePairs treeWidgetExpressions(const QString &fileName,
+                                               const QString &engineType = QString(),
+                                               const QString &function= QString()) const;
 
     void showToolTip(const QPoint &p, Core::IEditor *editor, DebuggerToolTipWidget *);
 

src/plugins/debugger/gdb/gdbengine.cpp

@@ -117,6 +117,7 @@ public:
 
     QPoint mousePosition;
     QString expression;
+    QByteArray iname;
     Core::IEditor *editor;
 };
 
@@ -3844,22 +3845,19 @@ void GdbEngine::showToolTip()
     if (m_toolTipContext.isNull())
         return;
     const QString expression = m_toolTipContext->expression;
-    const QByteArray iname = tooltipIName(m_toolTipContext->expression);
     if (DebuggerToolTipManager::debug())
-        qDebug() << "GdbEngine::showToolTip " << expression << iname << (*m_toolTipContext);
+        qDebug() << "GdbEngine::showToolTip " << expression << m_toolTipContext->iname << (*m_toolTipContext);
 
-    if (!debuggerCore()->boolSetting(UseToolTipsInMainEditor)) {
-        watchHandler()->removeData(iname);
+    if (m_toolTipContext->iname.startsWith("tooltip")
+        && (!debuggerCore()->boolSetting(UseToolTipsInMainEditor)
+            || !watchHandler()->isValidToolTip(m_toolTipContext->iname))) {
+        watchHandler()->removeData(m_toolTipContext->iname);
         return;
     }
 
-    if (!watchHandler()->isValidToolTip(iname)) {
-        watchHandler()->removeData(iname);
-        return;
-    }
     DebuggerToolTipWidget *tw = new DebuggerToolTipWidget;
-    tw->setDebuggerModel(TooltipType);
-    tw->setExpression(expression);
+    tw->setIname(m_toolTipContext->iname);
+    tw->setExpression(m_toolTipContext->expression);
     tw->setContext(*m_toolTipContext);
     tw->acquireEngine(this);
     DebuggerToolTipManager::instance()->showToolTip(m_toolTipContext->mousePosition,
@@ -3890,12 +3888,22 @@ bool GdbEngine::setToolTipExpression(const QPoint &mousePos,
 
     DebuggerToolTipContext context = contextIn;
     int line, column;
-    const QString exp = fixCppExpression(cppExpressionAt(editor, context.position, &line, &column, &context.function));
-    if (DebuggerToolTipManager::debug())
-        qDebug() << "GdbEngine::setToolTipExpression1 " << exp << context;
+    QString exp = fixCppExpression(cppExpressionAt(editor, context.position, &line, &column, &context.function));
     if (exp.isEmpty())
         return false;
+    // Prefer a filter on an existing local variable if it can be found.
+    QByteArray iname;
+    if (const WatchData *localVariable = watchHandler()->findCppLocalVariable(exp)) {
+        exp = QLatin1String(localVariable->exp);
+        iname = localVariable->iname;
+    } else {
+        iname = tooltipIName(exp);
+    }
 
+    if (DebuggerToolTipManager::debug())
+        qDebug() << "GdbEngine::setToolTipExpression1 " << exp << iname << context;
+
+    // Same expression: Display synchronously.
     if (!m_toolTipContext.isNull() && m_toolTipContext->expression == exp) {
         showToolTip();
         return true;
@@ -3904,7 +3912,14 @@ bool GdbEngine::setToolTipExpression(const QPoint &mousePos,
     m_toolTipContext.reset(new GdbToolTipContext(context));
     m_toolTipContext->mousePosition = mousePos;
     m_toolTipContext->expression = exp;
+    m_toolTipContext->iname = iname;
     m_toolTipContext->editor = editor;
+    // Local variable: Display synchronously.
+    if (iname.startsWith("local")) {
+        showToolTip();
+        return true;
+    }
+
     if (DebuggerToolTipManager::debug())
         qDebug() << "GdbEngine::setToolTipExpression2 " << exp << (*m_toolTipContext);
 
@@ -3912,13 +3927,13 @@ bool GdbEngine::setToolTipExpression(const QPoint &mousePos,
         UpdateParameters params;
         params.tryPartial = true;
         params.tooltipOnly = true;
-        params.varList = tooltipIName(exp);
+        params.varList = iname;
         updateLocalsPython(params);
     } else {
         WatchData toolTip;
         toolTip.exp = exp.toLatin1();
         toolTip.name = exp;
-        toolTip.iname = tooltipIName(exp);
+        toolTip.iname = iname;
         watchHandler()->insertData(toolTip);
     }
     return true;

src/plugins/debugger/gdb/pythongdbengine.cpp

@@ -64,17 +64,35 @@ void GdbEngine::updateLocalsPython(const UpdateParameters &params)
     const QString fileName = stackHandler()->currentFrame().file;
     const QString function = stackHandler()->currentFrame().function;
     if (!fileName.isEmpty()) {
-        QStringList expressions = DebuggerToolTipManager::instance()
+        typedef DebuggerToolTipManager::ExpressionInamePair ExpressionInamePair;
+        typedef DebuggerToolTipManager::ExpressionInamePairs ExpressionInamePairs;
+
+        // Re-create tooltip items that are not filters on existing local variables in
+        // the tooltip model.
+        ExpressionInamePairs toolTips = DebuggerToolTipManager::instance()
             ->treeWidgetExpressions(fileName, objectName(), function);
+
         const QString currentExpression = tooltipExpression();
-        if (!currentExpression.isEmpty() && !expressions.contains(currentExpression))
-            expressions.push_back(currentExpression);
-        foreach (const QString &te, expressions) {
-            if (!watchers.isEmpty())
-                watchers += "##";
-            watchers += te.toLatin1();
-            watchers += '#';
-            watchers += tooltipIName(te);
+        if (!currentExpression.isEmpty()) {
+            int currentIndex = -1;
+            for (int i = 0; i < toolTips.size(); ++i) {
+                if (toolTips.at(i).first == currentExpression) {
+                    currentIndex = i;
+                    break;
+                }
+            }
+            if (currentIndex < 0)
+                toolTips.push_back(ExpressionInamePair(currentExpression, tooltipIName(currentExpression)));
+        }
+
+        foreach (const ExpressionInamePair &p, toolTips) {
+            if (p.second.startsWith("tooltip")) {
+                if (!watchers.isEmpty())
+                    watchers += "##";
+                watchers += p.first.toLatin1();
+                watchers += '#';
+                watchers += p.second;
+            }
         }
     }
 

src/plugins/debugger/watchhandler.cpp

@@ -1550,7 +1550,7 @@ QByteArray WatchHandler::watcherName(const QByteArray &exp)
     return "watch." + QByteArray::number(theWatcherNames[exp]);
 }
 
-void WatchHandler::watchExpression(const QString &exp)
+void WatchHandler::watchExpression(const QString &exp, const QString &name)
 {
     QTC_ASSERT(m_engine, return);
     // Do not insert the same entry more then once.
@@ -1560,7 +1560,7 @@ void WatchHandler::watchExpression(const QString &exp)
     // FIXME: 'exp' can contain illegal characters
     WatchData data;
     data.exp = exp.toLatin1();
-    data.name = exp;
+    data.name = name.isEmpty() ? exp : name;
     theWatcherNames[data.exp] = m_watcherCounter++;
     saveWatchers();
 
@@ -1794,6 +1794,20 @@ const WatchData *WatchHandler::findData(const QByteArray &iname) const
     return m_model->findItem(iname);
 }
 
+const WatchData *WatchHandler::findCppLocalVariable(const QString &name) const
+{
+    // Can this be found as a local variable?
+    const QByteArray localsPrefix("local.");
+    QByteArray iname = localsPrefix + name.toLatin1();
+    if (const WatchData *wd = findData(iname))
+        return wd;
+    // Nope, try a 'local.this.m_foo'.
+    iname.insert(localsPrefix.size(), "this.");
+    if (const WatchData *wd = findData(iname))
+        return wd;
+    return 0;
+}
+
 QString WatchHandler::displayForAutoTest(const QByteArray &iname) const
 {
     return m_model->displayForAutoTest(iname);

src/plugins/debugger/watchhandler.h

@@ -58,15 +58,6 @@ public:
 
 typedef QHash<QString, QStringList> TypeFormats;
 
-enum WatchType
-{
-    LocalsType,
-    InspectType,
-    WatchersType,
-    ReturnType,
-    TooltipType
-};
-
 enum IntegerFormat
 {
     DecimalFormat = 0, // Keep that at 0 as default.
@@ -86,7 +77,7 @@ public:
     QAbstractItemModel *model() const;
 
     void cleanup();
-    void watchExpression(const QString &exp);
+    void watchExpression(const QString &exp, const QString &name = QString());
     Q_SLOT void clearWatches();
 
     void updateWatchers(); // Called after locals are fetched
@@ -95,6 +86,7 @@ public:
 
     const WatchData *watchData(const QModelIndex &) const;
     const WatchData *findData(const QByteArray &iname) const;
+    const WatchData *findCppLocalVariable(const QString &name) const;
     QString displayForAutoTest(const QByteArray &iname) const;
     bool hasItem(const QByteArray &iname) const;
 

src/plugins/debugger/watchwindow.cpp

@@ -920,7 +920,7 @@ void WatchTreeView::contextMenuEvent(QContextMenuEvent *ev)
         grabMouse(Qt::CrossCursor);
         m_grabbing = true;
     } else if (act == actWatchExpression) {
-        watchExpression(exp);
+        watchExpression(exp, name);
     } else if (act == actRemoveWatchExpression) {
         handler->removeData(p.data(LocalsINameRole).toByteArray());
     } else if (act == actCopy) {
@@ -1036,7 +1036,12 @@ void WatchTreeView::reset()
 
 void WatchTreeView::watchExpression(const QString &exp)
 {
-    currentEngine()->watchHandler()->watchExpression(exp);
+    watchExpression(exp, QString());
+}
+
+void WatchTreeView::watchExpression(const QString &exp, const QString &name)
+{
+    currentEngine()->watchHandler()->watchExpression(exp, name);
 }
 
 void WatchTreeView::setModelData

src/plugins/debugger/watchwindow.h

@@ -56,6 +56,7 @@ public:
 
 public slots:
     void watchExpression(const QString &exp);
+    void watchExpression(const QString &exp, const QString &name);
     void handleItemIsExpanded(const QModelIndex &idx);
 
 private: