[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwrite-devel
Subject:    Re: Notes / Observations while embedding KTextEditor::Plugin(s) into a new host application
From:       Christoph Cullmann <christoph () cullmann ! io>
Date:       2020-01-15 12:17:51
Message-ID: d7fd4bc278b6d5a2acadd3b0b20b8e43 () cullmann ! io
[Download RAW message or body]

On 2020-01-14 10:11, Thomas Friedrichsmeier wrote:
> Hi!
> 
> I'm quite happy to see that the plugins written for kate seem to work
> reasonably well in RKWard, now, and most interface differences are in
> fact abstracted away quite nicely. Kudos to the developers who
> designed this!

Hi,

nice to hear ;=)

> 
> However, here's a list of the troubles I ran into during the process of
> getting the plugins to work in RKWard, and some ideas where applicable.
> This is more a brainstorming list, rather than any sort of well thought
> our proposal, but I hope some of this can be addressed for KF6 (in
> general, what would be the best place to collect ideas for / discuss
> KF6 plans)?
> 
> Ok, here's what I found:
> - Overall the process is pretty tedious. For one thing documentation
>   on writing a plugin host is still a bit lacking (importantly a
>   well-defined starting point in the API docs). For another thing, the
>   initial barrier is fairly high, already, with quite a heavy list of
>   methods to be implemented.

Yes, that is unfortunately try, help in that areas is highly welcome.

> 
> - Could some of the functionality of KatePluginManger be brought into
>   the KTextEditor-API? For RKWard I'm duplicating a whole lot of that.
>   I know for KDevelop the case is somewhat different, because that
>   supports different types of plugin. However, some basics might still
>   be offered, such as:
> 	- getting a list of available plugins
> 	- loading / unloading plugins by their id
> 	- some of the bureaucracy around creating/discarding plugin
>   views: Reading their config, emitting signals, keeping track of which
>   plugins are loaded, and their created (tool) views

It would make perhaps sense to move the plugin interface again back to 
Kate
and just let e.g. other applications depend on a library we install 
there.

Then we could relax the binary compatibility rules, too.

> 
> - Some plugins appear to violate the return value rules stated in
>   KTextEditor::MainWindow. For instance at a few places, the search
>   plugin assumes that activeView() cannot be 0. Patch submitted,
>   accpeted, and pushed, however I'm quite afraid this is not the only
>   instance of this and similar problems (as kate *always* has an active
>   view). Therefore, I also resorted to creating a hidden dummy view on
>   the fly, if and when asked for it. Now that is not so cool, either,
>   as it means well behaved plugins have  not chance to detect whether
>   or not there is an active view. Perhaps adding a new function "bool
>   isEmpty()" to the MainWindow interface? (Note: Same issue for
>   KTextEditor::Application::documents())

I am not sure if some "isEmpty()" function will help.
Either we require plugins to check for nullptr and empty lists or not.
If I don't misunderstand the proposal, thought ;=)

> 
> - There are also - lesser - nuisances, such as some plugins
>   malfunctioning, until readSessionConfig() has been called on them
>   (noteably search-and-replace: Search-box gets cleared "as you type").
>   Reading config is a good thing to do at any rate, but debugging this
>   has cost me an hour of frustration, while I was implementing things
>   step-by-step.

Guess that should be better documented, too, like you mention above, our
docs suck in that regard :/

> 
> - I suppose the reason for using Qt-slot-dispatch, instead of virtual
>   functions is to avoid incompatibility nightmares between host and
>   plugins compiled against different versions of ktexteditor. However,
>   having no compile-time sanity check does make the hair on the back of
>   my neck stand up. Would it make sense to add an intermediate layer in
>   the form of - say - a KTextEditor::MainWindowImplementation that has
>   virtual slots for all the dispatch functions, and require the parent
>   of KTextEditor::MainWindow to be an instance of this class? I imagine
>   this might also make it easier to provide default implementations,
>   and related API (such as plugin manager stuff, see above), where
>   applicable. "Grandfathered" interface functions could even be
>   converted to plain virtual function calls from MainWindow to
>   MainWindowImplementation, while any new additions during a major
>   release would continue to be implemented as slot invocations for
>   compatibility.

This could be avoided if we move this out of KF5 (in KF6) and relax
the BC constraints. At the moment using virtual functions will just not
allow us to extend the API without again adding extension to extension 
interface.

> 
> API suggestions:
> - I plead for the removal of KTextEditor::MainWindow::guiFactory(). See
> https://mail.kde.org/pipermail/kwrite-devel/2020-January/006002.html .
> The long term plan to make plugins KXMLGUIClient-derived sounds good to
> me. Not sure whether my proposed interim solution is worth the trouble,
> for now I have an ugly hack in place to achieve what I want.

I would go for the inheritance in KF6.

> 
> - Application::findUrl() could be implemented on top of
> Application::documents()

You mean that the KTextEditor library should just implement it that way?
Would make sense to avoid having it implemented twice.

> 
> - MainWindow::(create|hide|show|delete|addWidgetTo)ViewBar: It took me
> some time to figure out that these are not (currently?) used by any
> existing plugins, but by the view, and that it really is optional. For
> more clarity, could this be split out into an all-separate class /
> interface, and then you can simply have one
> MainWindow::(get|set)ViewBar to provide a custom implementation? (And
> then it would also become trivial to share the code for such a viewbar
> between host applications).

Sounds not unreasonable, if you have some proposal ready, we could think 
about
that for KF6.

> 
> - (close)splitView() in KTextEditor::MainWindow: Took me a while to
> figure these out, because in RKWard split views are a little different,
> in that we do not put emphasis on having the same documents in split
> parts of the view. They behave more like separate main windows, but if
> one becomes empty (due to views being closed), it is merged with the
> other split portion, automatically. Two splits can be completely
> disjunct (having no shared documents). Well, that's not much of a
> problem. What bugs me, again, is that it is not immediately obvious,
> whether (or what for) these are required when hosting plugins.
> Apparently not really (not used in plugins). The only win in
> implementing these seems to be the ability to split view from the vi
> command line. Not sure what conclusion to draw (if any). Perhaps
> documenting the fact. Or perhaps moving split-view functionality to a
> separate mini-interface for more clarity?
> 
> - MainWindow::viewsInSameSplitView(): I am not sure, what exactly it's
> supposed to achieve. (Nor what it's meant to mean: Does this mean two
> views that are visible side-by-side in a split window? Or two views
> that are never visible simultaneously, because they are stacked inside
> the same area?

For the split stuff: yes, that all is just there to e.g. have better VIM 
mode.
For the interface: I am not a fan of too much sub-interfaces.
That did lead to a lot of pain in the KDE 4 area, you needed to subclass 
X things
and to cast around the whole time.

> 
> - MainWindow::createToolView(): I guess this was designed around the
> good old KVBox. Now we need a dedicated override to insert added
> children into the parent's layout (in kate: ToolView::childEvent()),
> and again, that's a bit difficult to discover when writing a host. This
> could be simplified, if the plugin would pass a pointer to the widget
> it wants to add in createToolView(), so the host can add this to the
> tool view's layout.

Would make sense, too.

> 
> - Application::aboutToDeleteDocuments(), documentWillBeDeleted(),
> documentDeleted(), documentsDeleted(); correponding signals for
> document creation. Let's remove the single document variants of these
> for KF6, so the host does not have to emit both.

Consolidating this might make sense, too.

> 
> Plugin-specific notes:
> - For the search plugin, I'd like to be able to specify a default file
> filter

Reasonable, thought we need to think how to handle such plugin 
inter-communication.
Perhaps we should just go for some key/value QVariant based "you can set 
properties"
design with documented keys... Not sure. Having extra interfaces sounds 
like a lot work.

> 
> That's it for now, thanks for your time!
> Thomas

Great that you use that stuff now!

Actually, it would all be easier, if we could share more of our shell 
code.
Just not sure how we can do that easily.
But I think it is stupid (like you mention above) that we need to 
duplicate
that much effort.

Greetings
Christoph

-- 
Ignorance is bliss...
https://cullmann.io | https://kate-editor.org
[prev in list] [next in list] [prev in thread] [next in thread]