kate-ctags plugin: support KDevelop (WIP)
ClosedPublic

Authored by rjvbb on Nov 9 2018, 3:58 PM.

Details

Summary

This is a work-in-progress set of changes intending to make the CTags plugin available in/from KDevelop.

KDevelop does not currently have support for KTextEditor configuration dialogs so I've had to move the common_db database to a generic location (cf. the snippets plugin which does the same). I'm open to migration suggestions (common_db can take quite a while to (re)generate).

Test Plan

Currently this allows jumping to a symbol's declaration or definition in a (new) document; even the cursor is positioned correctly in my tests. Evidently this requires a companion change in KDevelop because it does not yet implement KTextEditorIntegration::showToolView() and openURL. For completeness sake:

KTextEditor::View *MainWindow::openUrl(const QUrl &url, const QString &encoding)
{
    Q_UNUSED(encoding);

    auto documentController = Core::self()->documentControllerInternal();
    auto doc = url.isEmpty() ? documentController->openDocumentFromText(QString()) : documentController->openDocument(url);
    return doc ? doc->activeTextView() : nullptr;
}

bool MainWindow::showToolView(QWidget *widget)
{
    const auto view = ToolViewFactory::fromWidget(widget);
    if (view) {
        Core::self()->uiController()->raiseToolView(widget);
        qWarning() << Q_FUNC_INFO << view << view->m_text;
        return true;
    }
    qWarning() << Q_FUNC_INFO << "no ToolViewFactory for" << widget;
    return false;
}

That showToolView() method doesn't work though, and opening the toolview manually yields a garbled plugin UI, as if it doesn't get the correct size information.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 5041
Build 5059: arc lint + arc unit
rjvbb created this revision.Nov 9 2018, 3:58 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald TranscriptNov 9 2018, 3:58 PM
rjvbb requested review of this revision.Nov 9 2018, 3:58 PM
rjvbb updated this revision to Diff 45176.Nov 9 2018, 4:26 PM
rjvbb edited the summary of this revision. (Show Details)

Merged in changes from an older attempt I forgot about (apologies!).

This should make the common_db database accessible cross-application in a transparent fashion, and adds a shortcut to update/maintain that database.

The paths to index should probably be saved in a plugin-specific settings file, however. Suggestions welcome how best to do this!

rjvbb set the repository for this revision to R40 Kate.Nov 9 2018, 4:27 PM
sars accepted this revision.Nov 9 2018, 10:15 PM
sars added a subscriber: sars.

I'm a bit confused... I thought KDevelop had much more advanced plugins/features for this? What does KDevelop need this old plugin for? Non-C/C++ languages or what?

The ctags configuration is much simpler and automatic for the user in the Kate Project plugin so I have actually been thinking about deprecating the standalone ctags plugin. Before I can do that tho, I would have to implement the missing jump to definition/declaration/back to the project plugin. There is also the global database that would need to be added to the project plugin before feature parity is achieved.

I have not started any of that yet so it is not something that definitively will come any time soon, but just making clear that I'm not super dedicated to the current plugin as it is right now.

Now to the review:

Ship it :)

I would probably add the word "global" or "common" to "Update the Database" to not confuse the user with updating the session specific database...

This revision is now accepted and ready to land.Nov 9 2018, 10:15 PM
rjvbb added a comment.Nov 9 2018, 10:32 PM
I'm a bit confused... I thought KDevelop had much more advanced plugins/features for this? What does KDevelop need this old plugin for? Non-C/C++ languages or what?

Actually, KDevelop currently only (AFAIK!) has a full-fledged parser for C-family languages, and as has been discussed on the KDevelop user ML recently, even that one isn't always reliable nor feature-complete for everyone.
A CTags plugin would complement the existing features, and provide support for a lot of other languages. It works very well for TCL, for instance.

Ship it :)

I think I'll add a few more tweaks. I just noticed that the update dialog I added is crippled, for instance (lacks the required Apply/Reset/Default buttons).

I would probably add the word "global" or "common" to "Update the Database" to not confuse the user with updating the session specific database...

Roger that.

rjvbb updated this revision to Diff 45427.Nov 13 2018, 5:29 PM

This adds a safety against a race-like condition I've seen happen once in KDevelop.
It may not be required to make m_mWin a QPointer but it seemed sensible to do.

Somehow this plugin causes duplicate context menu entries in KDevelop documents, apparently because the CTags context submenu is added via KXMLGUI. I have an initial fix for KDevelop for that; I'll wait with committing this until that issue has been addressed.

rjvbb set the repository for this revision to R40 Kate.Nov 13 2018, 5:29 PM
rjvbb updated this revision to Diff 45672.Nov 17 2018, 4:11 PM

I've made a few more usability changes, and renamed the new menu action to "Configure..." because it corresponds to what the action does.

I'm now ready to push this because the duplicate context menu issue has been addressed. I'll wait a few days to allow you to react to this, hopefully final, version.

rjvbb set the repository for this revision to R40 Kate.Nov 17 2018, 4:11 PM
rjvbb removed a subscriber: kdevelop-devel.
cullmann requested changes to this revision.Dec 2 2018, 12:36 PM
cullmann added a subscriber: cullmann.

Hmm, could you remove or comment out:

qWarning() << "Config file:" << config.config()->name();

?

This revision now requires changes to proceed.Dec 2 2018, 12:36 PM
rjvbb updated this revision to Diff 46873.Dec 4 2018, 8:52 PM

undesirable qWarning() removed

rjvbb set the repository for this revision to R40 Kate.Dec 4 2018, 8:53 PM
anthonyfieroni added inline comments.
addons/kate-ctags/kate_ctags_view.cpp
73

Dynamic_cast is pretty expensive to not check against nullptr, use static_cast instead.

86–89

Add attribute to confWin deleteOnClose and make it parent of others.

rjvbb updated this revision to Diff 46902.Dec 5 2018, 3:04 PM

Updated as requested.

If there's no need for dynamic_cast'ing then it's a waste to spend cycles on it. I guess otherwise I could have done the cast outside of the lambda (?)

rjvbb set the repository for this revision to R40 Kate.Dec 5 2018, 3:05 PM
rjvbb marked 2 inline comments as done.
cullmann accepted this revision.Dec 8 2018, 5:35 PM

> ok to have that go in now, thanks!

This revision is now accepted and ready to land.Dec 8 2018, 5:35 PM
This revision was automatically updated to reflect the committed changes.
rjvbb added a comment.Dec 10 2018, 9:43 AM

Better late than never: I also figured out how to make the plugin's toolview display correctly in KDevelop: https://commits.kde.org/kate/3cd03f408eed2d66ae008fb8349f8b5af24260e9