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

[Thomas Friedrichsmeier]

Hi,

omitting some parts, to keep the mail from growing too long.

On Wed, 15 Jan 2020 13:17:51 +0100
Christoph Cullmann <christoph at cullmann.io> wrote:
> On 2020-01-14 10:11, Thomas Friedrichsmeier wrote:
> > - 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.

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 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 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)

> > - 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?

> > 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!

Regards
Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20200115/81fac30b/attachment.sig>