Notes / Observations while embedding KTextEditor::Plugin(s) into a new host application

[Christoph Cullmann]

Hi,

>> 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.
> 
> Hm, not sure that I get you right. For RKWard we're trying quite hard
> to support a wide range of our dependent libs, 'cause that's what our
> users have been asking for. They want the latest and greatest in R and
> RKWard, without touching anything else about their systems (as much as
> possible). So from our perspective, binary compatibility is a very good
> thing, indeed.

I see that point, but the you are stuck with some things like not being 
able
to e.g. add virtual functions.

> 
>> 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 ;=)
> 
> Like I said, this was brainstorming more than a well-thought-out
> proposal. All that isEmpty() would do is help those plugins that are
> well-behaved (while I have to lie to all other plugins by providing a
> fake view, just in case). But the gain may be little indeed.

I think it would be more clean for the plugins to just check if the
activeView returns null or the list is empty as said ;=)

But in any case, one needs to improve the plugins, as at the moment
they often rely on the Kate behavior of always having some view around
I assume.

> 
>> > - 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.
> 
> Maybe I'm missing something, here, but I was hoping to work around the
> BC problem by having both. Mockup:
> 
> /** KTextEditor::MainWindow-Interface. This is the "plugin-facing" side
> of the API */
> class MainWindow : public QObject {
> public:
> 	/** c'tor now requires more specific type of parent */
> 	MainWindow(MainWindowImpl *_parent) :
> 	QObject(_parent), parent(_parent) {};
> 
> 	/** Example of a method that has been around since before the
> 	latest major release. We can rely on it being around in the
> 	host */
> 	View* activeView() {
> 		return parent->activeView();
> 	}
> 
> 	/** Example of a method added during the major release
> 	lifetime. We continue to use slot-dispatch in this case. */
> 	void doMagic() {
> 		QMetaObject::invokeMethod(parent, "doMagic",
> 		Qt::DirectConnection);
> 	}
> private:
> 	MainWindowImpl* parent;
> };
> 
> /** KTextEditor::MainWindowImpl-"Implementation". This is the
> "host-facing" side of the API */
> class MainWindowImpl : public QObject {
> public:
> 	virtual View* activeView() {
> 		// default implementation knows no active view
> 		return nullptr;
> 	}
> public slots:
> 	virtual void doMagic() {
> 		// default implementation cannot do magic
> 	}
> public:
> 	/** Example of a function that is useful to hosting
> 	applications, but not (meant to be) called from plugins. */
> 	Plugin loadPlugin(const QString &id) {
> 		// map id to lib
> 		// create plugin
> 		// emit signal
> 		// return
> 	}
> };
> 
> KateMainWindow would inherit from KTextEditor::MainWindowImpl, and some
> of KatePluginManager could possibly be moved to MainWindowImpl. No
> further changes should be needed.
> 
> (Note: Not happy with the name "MainWindowImpl", but sticking to it for
> want of a better idea)

Perhaps I miss the point, but e.g. Kate would use 
KTextEditor::MainWindowImpl or?
That means it must stay stable BC wise and you don't win a thing with 
having it separated from
the KTextEditor::MainWindow.

You are right that the plugins will have no BC issue, but still the 
integration
will not work as the other side of the interface has BC issues.

But perhaps I misunderstand that.

> 
>> > - 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.
> 
> I'll see what I can come up with (and when). Again, where would I put
> code and ideas targetted for KF6?

I think we can either have stuff in extra files in master in some
subdir, to share ideas, or just create some branch for sharing such 
things.

> 
>> > 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.
> 
> Agreed. One getter, and one setter with QVariant can go a long way.

:=)

> 
>> > That's it for now, thanks for your time!
>> > Thomas
>> 
>> Great that you use that stuff now!
> 
> We'll have one more release without (next week or so), but I'm pretty
> eager to boast about all the new features "I" added, soon after that!

Cool!

Greetings
Christoph

-- 
Ignorance is bliss...
https://cullmann.io | https://kate-editor.org