Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
@elextr requested changes on this pull request.
@techee a (mostly) documentation review, lots of minor tweaks, but a significant point that I have commented on several times and might be best if I summarise here:
It is confusing the way function pointers that are members of struct PluginExtension are referred to as eg "_perform() function". They are not functions, they are function pointers with specific member names that end in _perform and _provided, the functions they point to are in the plugin and can have any name. You know what you mean and after some confusion I think I now know what you mean, so it would be best to fix it to use proper terminology so future readers don't go through the same confusion.
IIUC when it says "_provided() function" it should be "the function pointed to by the XXXX_provided member". Yes its longer but one global replace should get most of them.
In doc/plugins.dox:
> @@ -1116,4 +1118,130 @@ static void proxy_cleanup(GeanyPlugin *plugin, gpointer pdata) @endcode +@page plugin_extension Plugin Extension HowTo + +@section plugin_extension_intro Introduction + +Since Geany 2.1 the PluginExtension API allows plugins to take over some of
Originally the Geany plugin API only allowed plugins to add to Geany functionality, plugins could not modify Geany built-in functionality, but since Geany 2.1 ...
In doc/plugins.dox:
> @@ -1116,4 +1118,130 @@ static void proxy_cleanup(GeanyPlugin *plugin, gpointer pdata) @endcode +@page plugin_extension Plugin Extension HowTo + +@section plugin_extension_intro Introduction + +Since Geany 2.1 the PluginExtension API allows plugins to take over some of +the core Geany functionality, such as autocopletion, calltip display, symbol goto, +typename highlighting inside document, etc.
instead of hand wavey "such as" would be better to have a proper list of all the functions available for replacement that could then be kept up to date.
In doc/plugins.dox:
> @@ -1116,4 +1118,130 @@ static void proxy_cleanup(GeanyPlugin *plugin, gpointer pdata) @endcode +@page plugin_extension Plugin Extension HowTo + +@section plugin_extension_intro Introduction + +Since Geany 2.1 the PluginExtension API allows plugins to take over some of +the core Geany functionality, such as autocopletion, calltip display, symbol goto, +typename highlighting inside document, etc. + +@section plugin_extension_init Initialization and cleanup +
Not sure if doxygen will allow an actual link, but otherwise a reference to the plugin docs "Extension plugins are just normal plugins and behave as described in [link to plugin docs]." That provides the docs about what an init() function of a plugin is and then leads into extensions also having to register.
In doc/plugins.dox:
> + +Since Geany 2.1 the PluginExtension API allows plugins to take over some of +the core Geany functionality, such as autocopletion, calltip display, symbol goto, +typename highlighting inside document, etc. + +@section plugin_extension_init Initialization and cleanup + +First, any plugin interested in using this interface has to register its +@c PluginExtension instance using @c plugin_extension_register(). This typically +happens in the @c init() function of the plugin. Registered extensions have to be +unregistered before the plugin is unloaded using @c plugin_extension_unregister(), +typically inside the @c cleanup() function of the plugin. + +@section plugin_extension_impl Implementing extensions + +Inside the @c PluginExtension instance, the plugin fills-in the pointers to
whats a plugin extension instance?
In doc/plugins.dox:
> +the core Geany functionality, such as autocopletion, calltip display, symbol goto, +typename highlighting inside document, etc. + +@section plugin_extension_init Initialization and cleanup + +First, any plugin interested in using this interface has to register its +@c PluginExtension instance using @c plugin_extension_register(). This typically +happens in the @c init() function of the plugin. Registered extensions have to be +unregistered before the plugin is unloaded using @c plugin_extension_unregister(), +typically inside the @c cleanup() function of the plugin. + +@section plugin_extension_impl Implementing extensions + +Inside the @c PluginExtension instance, the plugin fills-in the pointers to +the functions it wishes to implement. Typically, these functions come in pairs: + - @c _provided() functions are used to query the plugin whether it implements
... used by Geany to query ...
Hmmm, its not until I got to the example below that I realised that _provided() is prefixed with the function name, maybe something like "XXXX_provided() where XXXX is the name of the function being replaced"
(cultural aside, XXXX is a local brand of beer, some say because the locals can't spell beer 😀 )
In doc/plugins.dox:
> + +@section plugin_extension_init Initialization and cleanup + +First, any plugin interested in using this interface has to register its +@c PluginExtension instance using @c plugin_extension_register(). This typically +happens in the @c init() function of the plugin. Registered extensions have to be +unregistered before the plugin is unloaded using @c plugin_extension_unregister(), +typically inside the @c cleanup() function of the plugin. + +@section plugin_extension_impl Implementing extensions + +Inside the @c PluginExtension instance, the plugin fills-in the pointers to +the functions it wishes to implement. Typically, these functions come in pairs: + - @c _provided() functions are used to query the plugin whether it implements + the particular feature for the passed document + - @c _perform() passes control to the plugin to perform the feature
... perform() used by Geany to pass control ...
...perform the feature instead of performing the Geany built-in functionality.
In doc/plugins.dox:
> +@c PluginExtension instance using @c plugin_extension_register(). This typically +happens in the @c init() function of the plugin. Registered extensions have to be +unregistered before the plugin is unloaded using @c plugin_extension_unregister(), +typically inside the @c cleanup() function of the plugin. + +@section plugin_extension_impl Implementing extensions + +Inside the @c PluginExtension instance, the plugin fills-in the pointers to +the functions it wishes to implement. Typically, these functions come in pairs: + - @c _provided() functions are used to query the plugin whether it implements + the particular feature for the passed document + - @c _perform() passes control to the plugin to perform the feature + +When the plugin returns @c TRUE from the @c _provided() function, it indicates +it wants to take control of the particular feature and disable Geany's +implementation. However, returing @c TRUE does not automatically guarantee that
returning
In doc/plugins.dox:
> +@section plugin_extension_impl Implementing extensions + +Inside the @c PluginExtension instance, the plugin fills-in the pointers to +the functions it wishes to implement. Typically, these functions come in pairs: + - @c _provided() functions are used to query the plugin whether it implements + the particular feature for the passed document + - @c _perform() passes control to the plugin to perform the feature + +When the plugin returns @c TRUE from the @c _provided() function, it indicates +it wants to take control of the particular feature and disable Geany's +implementation. However, returing @c TRUE does not automatically guarantee that +the plugin's implementation is executed - if there are multiple plugins competing +for implementing a feature, the extension with the highest priority +passed into the @c plugin_extension_register() function gets executed. + +Plugins can perform a check whether it is them who gets executed for the
A plugin can perform a check if it gets executed ...
A plugin isn't a people (at least until the AI plugin) so its "it" not "them"
In doc/plugins.dox:
> + - @c _provided() functions are used to query the plugin whether it implements + the particular feature for the passed document + - @c _perform() passes control to the plugin to perform the feature + +When the plugin returns @c TRUE from the @c _provided() function, it indicates +it wants to take control of the particular feature and disable Geany's +implementation. However, returing @c TRUE does not automatically guarantee that +the plugin's implementation is executed - if there are multiple plugins competing +for implementing a feature, the extension with the highest priority +passed into the @c plugin_extension_register() function gets executed. + +Plugins can perform a check whether it is them who gets executed for the +particular feature; e.g. for autocompletion the plugin can use +@c plugin_extension_autocomplete_provided() which returns @c TRUE if the +passed extension is executed, taking into account all registered extension +priorities and their interest in providing the particular feature.
I'd end the sentence at "priorities". Not sure what their "interest" is and again a plugin isn't a people 😁
In doc/plugins.dox:
> +static gboolean autocomplete_provided(GeanyDocument *doc, gpointer data)
+{
+ /* Check whether the plugin provides the feature for the passed document */
+ return doc->file_type->id == GEANY_FILETYPES_PYTHON;
+}
+
+
+static void autocomplete_perform(GeanyDocument *doc, gboolean force, gpointer data)
+{
+ /* The autocompletion logic comes here, including the autocompletion UI
+ * display (either using some custom widget or using Scintilla's
+ * SCI_AUTOCSHOW) */
+}
+
+
+/* The PluginExtension instance - we only implement autocompletion here. */
...here so only fill in the pointers for that function.
So in fact it doesn't matter what the functions are called, its what entry in the struct its passed to that matters:
.autocomplete_provided = foo_bar,
.autocomplete_perform = foo_bletch
Would be just as legal, thats not really clear pre-example, but maybe when you explain what an "instance" is (see comment above) that can be made clear.
> + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +/** + * Demo plugin extension - example of a plugin using the PluginExtension API + * to provide simple autocompletion of Python keywords.
"This is the plugin example used in the documentation."
> +
+
+/* sort higher priorities first */
+static gint sort_extension_entries(gconstpointer a, gconstpointer b)
+{
+ const PluginExtensionEntry *entry_a = a;
+ const PluginExtensionEntry *entry_b = b;
+
+ return entry_b->priority - entry_a->priority;
+}
+
+
+/**
+ * Registers the provided extension in Geany. There can be multiple extensions
+ * registered in Geany - these are stored in a list sorted by the priporty
+ * parameter. When executing @c _perform() functions, Geany goes through
See comments above on the overview docs.
> + +/** + * Registers the provided extension in Geany. There can be multiple extensions + * registered in Geany - these are stored in a list sorted by the priporty + * parameter. When executing @c _perform() functions, Geany goes through + * the list and executes the @c _perform() function of the first extension in + * the list whose @c _provided() function returns @c TRUE. + * + * This function is typically called in the plugin @c init() function. + * + * Plugins wishing to re-register themselves, e.g. with a different priority, + * should first unregister themselves using @c plugin_extension_unregister() + * and call @c plugin_extension_register() afterwards. + * + * @param extension The @c PluginExtension structure instance provided by the + * plugin.
Would be better to describe what the actual parameter does, we know its provided by the plugin, its calling this function. 😁
"The @c PluginExtension structure initialised with the function pointers for the functionality the plugin wishes to replace."
> + * the list and executes the @c _perform() function of the first extension in + * the list whose @c _provided() function returns @c TRUE. + * + * This function is typically called in the plugin @c init() function. + * + * Plugins wishing to re-register themselves, e.g. with a different priority, + * should first unregister themselves using @c plugin_extension_unregister() + * and call @c plugin_extension_register() afterwards. + * + * @param extension The @c PluginExtension structure instance provided by the + * plugin. + * @param ext_name Human-readable name of the extension that can appear in + * the user interface. The string should be reasonably unique so extensions can + * be distinguished from each other. + * @param priority The higher number, the earlier in the extension list the + * plugin is and the higher chance it becomes called when multiple extensions
don't think we need the bit about the list, unless I am missing something thats implementation and is irrelevant to the user, so "The higher the number the higher the chance ...."
And what if two have the same priority? Do we want to define it, or my preference is to state its implementation defined which wins and that may change.
> + * Plugins wishing to re-register themselves, e.g. with a different priority, + * should first unregister themselves using @c plugin_extension_unregister() + * and call @c plugin_extension_register() afterwards. + * + * @param extension The @c PluginExtension structure instance provided by the + * plugin. + * @param ext_name Human-readable name of the extension that can appear in + * the user interface. The string should be reasonably unique so extensions can + * be distinguished from each other. + * @param priority The higher number, the earlier in the extension list the + * plugin is and the higher chance it becomes called when multiple extensions + * compete for the same feature. + * @param data User data passed to the functions from the @c PluginExtension + * struct. + * @warning Plugins are responsible for calling @c plugin_extension_unregister() + * when they no longer provide the extension.
... and when the plugin is unloaded.
> + g_return_if_fail(ext_name != NULL); + + entry = g_malloc(sizeof *entry); + entry->extension = extension; + entry->data = data; + entry->priority = priority; + + all_extensions = g_list_insert_sorted(all_extensions, entry, sort_extension_entries); +} + + +/** + * Plugins are responsible for calling this function when they no longer + * provide the extension, at the latest in the plugin @c cleanup() function. + * + * @param extension The @c PluginExtension structure that is being unregistered.
Question, does this need to be the actual same chunk of memory, or just one with the same contents, and what if its not?
> + return entry->extension->f_perform args; \ + break; \ + } \ + } \ + return defret; \ + } G_STMT_END + + +/* + * Geany itself can also pass NULL as the extension parameter to determine + * whether there is any extension implementing autocompletion. Plugins should + * not use this and rely on Geany's extension list and the priorities so + * this feature is not documented in the plugin API. + */ +/** + * Plugins can call this function to check whether, based on the extension list
"extension list" S/B "extensions registered" as above its implementation if its a list
> + */
+#define CALL_PROVIDED(f, doc, ext) \
+ G_STMT_START { \
+ for (GList *node = all_extensions; node; node = node->next) \
+ { \
+ PluginExtensionEntry *entry = node->data; \
+ \
+ if (entry->extension->f && entry->extension->f(doc, entry->data)) \
+ return (ext) ? entry->extension == (ext) : TRUE; \
+ \
+ if ((ext) && entry->extension == (ext)) \
+ return FALSE; \
+ } \
+ return FALSE; \
+ } G_STMT_END
+
I hate to reopen the implementation in what is mostly a documentation review, but will 😈. Instead of one list of extensions could there not be a separate list per extension point of extensions offering that functionality sorted by priority as they are registered, so each _PROVIDED and _PERFORM just need to access the first on the list not search every time the extension is accessed?
> + * Geany itself can also pass NULL as the extension parameter to determine + * whether there is any extension implementing autocompletion. Plugins should + * not use this and rely on Geany's extension list and the priorities so + * this feature is not documented in the plugin API. + */ +/** + * Plugins can call this function to check whether, based on the extension list + * and the provided extension priorities, the extension passed in the @c ext + * parameter is used for autocompletion. + * + * Plugins will typically call this function with their own @c PluginExtension + * to check, if it is them who gets (or got) executed for autocompletion of the + * provided document. This is useful for various auxiliary functions such as + * cleanups after the @c _perform() function is completed so plugins know it was + * them who executed the @c _perform() function and do not have to store this + * information by some other means.
reminder all the "them"s S/B "it"
> + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +/** + * @file pluginextension.h + * This file defines an interface allowing plugins to take over some of the + * core functionality provided by Geany, such as autocompletion, calltips, + * symbol goto, types highlighting inside document, etc.
again proper complete list, not just "such as" which implies its not a complete list
> + * This file defines an interface allowing plugins to take over some of the + * core functionality provided by Geany, such as autocompletion, calltips, + * symbol goto, types highlighting inside document, etc. + **/ + +#ifndef GEANY_PLUGIN_EXTENSION_H +#define GEANY_PLUGIN_EXTENSION_H 1 + +#include "document.h" + + +G_BEGIN_DECLS + +/** + * Structure serving as an interface between plugins and Geany allowing + * plugins inform Geany about what features they provide and allowing
... to inform ...
> + +#ifndef GEANY_PLUGIN_EXTENSION_H +#define GEANY_PLUGIN_EXTENSION_H 1 + +#include "document.h" + + +G_BEGIN_DECLS + +/** + * Structure serving as an interface between plugins and Geany allowing + * plugins inform Geany about what features they provide and allowing + * Geany to delegate its functionality to the plugins. + * + * Plugins should allocate this structure on the heap - either statically, + * or dynamically. When allocated dynamically, plugins should always
statically on the heap?
what is the required lifetime of the object? when can it be freed?
In the G* terminology who "owns" it if the pointer is also cached in the callee, and is that handled for non-C plugins?
> +#include "document.h" + + +G_BEGIN_DECLS + +/** + * Structure serving as an interface between plugins and Geany allowing + * plugins inform Geany about what features they provide and allowing + * Geany to delegate its functionality to the plugins. + * + * Plugins should allocate this structure on the heap - either statically, + * or dynamically. When allocated dynamically, plugins should always + * zero-fill the structure before use. + * + * Depending on the functionality they provide, plugins should assign the + * pointers from the structure to the functions implementing this interface.
assign pointers to the functions implementing this interface to the appropriate members of the structure.
Actually its pairs isn't it? What if only one of the pair is assigned, UB?
> + +G_BEGIN_DECLS + +/** + * Structure serving as an interface between plugins and Geany allowing + * plugins inform Geany about what features they provide and allowing + * Geany to delegate its functionality to the plugins. + * + * Plugins should allocate this structure on the heap - either statically, + * or dynamically. When allocated dynamically, plugins should always + * zero-fill the structure before use. + * + * Depending on the functionality they provide, plugins should assign the + * pointers from the structure to the functions implementing this interface. + * Not all of the functions have to be implemented by the plugin - unimplemented + * functions can be left to contain the NULL pointer.
member pointers for unimplemented ...
> + +/** + * Structure serving as an interface between plugins and Geany allowing + * plugins inform Geany about what features they provide and allowing + * Geany to delegate its functionality to the plugins. + * + * Plugins should allocate this structure on the heap - either statically, + * or dynamically. When allocated dynamically, plugins should always + * zero-fill the structure before use. + * + * Depending on the functionality they provide, plugins should assign the + * pointers from the structure to the functions implementing this interface. + * Not all of the functions have to be implemented by the plugin - unimplemented + * functions can be left to contain the NULL pointer. + * + * Typically, functions from this interface come in pairs. Functins ending with
Ok, so question above about what if only one S/B answered here
As noted above its not the function names that end in _provided() or _perform() its the structure members that have those names.
typo "Functions"
> + * or dynamically. When allocated dynamically, plugins should always + * zero-fill the structure before use. + * + * Depending on the functionality they provide, plugins should assign the + * pointers from the structure to the functions implementing this interface. + * Not all of the functions have to be implemented by the plugin - unimplemented + * functions can be left to contain the NULL pointer. + * + * Typically, functions from this interface come in pairs. Functins ending with + * @c _provided() inform Geany whether the plugin implements the given feature + * for the passed document. Functions ending with @c _perform() are called by + * Geany at appropriate moments to inform the plugin when to perform the given + * feature. + * + * Instances of the @c PluginExtension structures are registered in Geany using + * the @c plugin_extension_register() function.
This is just a data structure passed to the register function to provide a bunch of pointers, it shouldn't be elevated to the status of "Instances of the ... structures are registered in Geany" as if its the extension, the plugin is the extension.
> + * functions can be left to contain the NULL pointer. + * + * Typically, functions from this interface come in pairs. Functins ending with + * @c _provided() inform Geany whether the plugin implements the given feature + * for the passed document. Functions ending with @c _perform() are called by + * Geany at appropriate moments to inform the plugin when to perform the given + * feature. + * + * Instances of the @c PluginExtension structures are registered in Geany using + * the @c plugin_extension_register() function. + * + * @warning The API provided by this file is subject to change and should not be + * considered stable at this point. That said, it is highly probable that if + * a change of this API is required in the future, it will not be of a major + * nature and should not require major modifications of the affected plugins + * (e.g. added/modified parameter of a function and similar changes).
is it going to be stabilised in the future? Probably @b4n question.
> + * Geany at appropriate moments to inform the plugin when to perform the given
+ * feature.
+ *
+ * Instances of the @c PluginExtension structures are registered in Geany using
+ * the @c plugin_extension_register() function.
+ *
+ * @warning The API provided by this file is subject to change and should not be
+ * considered stable at this point. That said, it is highly probable that if
+ * a change of this API is required in the future, it will not be of a major
+ * nature and should not require major modifications of the affected plugins
+ * (e.g. added/modified parameter of a function and similar changes).
+ **/
+typedef struct
+{
+ /**
+ * Called by Geany to check whether the plugin implements autocompletion
Ok, its understandable, but technically it should be "Pointer to function called by Geany ..."
Same for all below.
> +static GList *all_extensions = NULL;
+
+
+/* sort higher priorities first */
+static gint sort_extension_entries(gconstpointer a, gconstpointer b)
+{
+ const PluginExtensionEntry *entry_a = a;
+ const PluginExtensionEntry *entry_b = b;
+
+ return entry_b->priority - entry_a->priority;
+}
+
+
+/**
+ * Registers the provided extension in Geany. There can be multiple extensions
+ * registered in Geany - these are stored in a list sorted by the priporty
priporty
> + * Instances of the @c PluginExtension structures are registered in Geany using
+ * the @c plugin_extension_register() function.
+ *
+ * @warning The API provided by this file is subject to change and should not be
+ * considered stable at this point. That said, it is highly probable that if
+ * a change of this API is required in the future, it will not be of a major
+ * nature and should not require major modifications of the affected plugins
+ * (e.g. added/modified parameter of a function and similar changes).
+ **/
+typedef struct
+{
+ /**
+ * Called by Geany to check whether the plugin implements autocompletion
+ * for the provided document.
+ *
+ * @param doc The document for which Geany checks whether the plugin
"checks" S/B is "is querying"
Maybe a note that this allows plugins to restrict their replacement of the function to documents of specific filetypes or other characteristics.
In src/symbols.c:
> @@ -258,6 +259,23 @@ const gchar *symbols_get_context_separator(gint ft_id) } +/** Gets the icon data corresponding to the provided TMIcon. + * @param icon TMIcon. + * + * Returns the GdkPixbuf corresponding to the provided TMIcon. + * + * @since 2.1 + */ +GEANY_API_SYMBOL
Is this really part of the extensions patches? Is it another PR that needs merging first?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.