It's useful to be able to attach arbitrary data to documents, especially for plugins. This PR adds GObject-like keyed data lists to documents so plugins can add stuff similar to `g_object_get|set_data()`. The first commit adds a simple wrapper to the document module, the second exposes a safe API for plugins.
In the future if GeanyDocument is ever converted to a GObject, the new `document_get|set_data()` functions could become simple wrappers around `g_object_get|set_data()` rather than using keyed data lists directly.
Aside from conventional plugins, this will also be useful for planned "filetype plugins" from #1195 and possibly for certain proxy plugin implementations, and could even prove useful in the core.
A made a very simple test plugin to tinker with this in https://github.com/codebrainz/geany/commit/b662ba3ca15969429ee2f9cf4c00e8402.... You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1203
-- Commit Summary --
* Add support for Keyed Data Lists for documents * Add plugin API functions to get/set document data
-- File Changes --
M src/document.c (23) M src/document.h (7) M src/documentprivate.h (2) M src/plugins.c (39) M src/pluginutils.c (153) M src/pluginutils.h (11)
-- Patch Links --
https://github.com/geany/geany/pull/1203.patch https://github.com/geany/geany/pull/1203.diff
@kugel- can you tell me (or provide a patch of) what annotations to add to the Doxygen comments, as mentioned commit message for 7b2b9609e473bdc591010a68794df46dac52e1be, if any.
LGBCI (C == conceptual)
As a later optimisation the Quark could be returned and a quark based query function made available if its ever found to be needed.
What's the advantage over a hash table inside the plugin, using the document pointers as key?
What's the advantage over a hash table inside the plugin, using the document pointers as key?
I'm sure you mean document IDs since pointers can be re-used.
What's the advantage over a hash table inside the plugin, using the document pointers as key?
That it doesn't need to be implemented inside each plugin. Also, in theory core code could use it.
I'm sure you mean document IDs since pointers can be re-used.
even better
That it doesn't need to be implemented inside each plugin. Also, in theory core code could use it.
That's true, but it seems that the convenience has some cost (the cleanup management you implemented). On the other hand, setting up hash tables is trivial and only a few lines, so I'm not sure there is so much gained with this API.
I'm not against, just unsure if it's worth it.
I'm not against, just unsure if it's worth it.
I don't have much opinion on implementation (I chose the similar underlying GObject mechanism obviously), but I have a need to have my plugin attach data/free_func to each document, and I would like it not to clash with any other plugin. Otherwise, I don't care about the implementation, I think I have encapsulated it enough to upgrade to various better implementations if this way doesn't work.
I kind of see where @kugel- is coming from, a plugin can have a totally internal table indexed by `<document_id>/key` and it is easy to clean up nicely when the plugin goes away.
Not sure if I have missed something?
Not sure if I have missed something?
It's what this PR tries to avoid plugins having to re-implement each time they need to associate some pointer with each document. It's not terribly difficult to implement, but it requires connecting to several signals (`document-new`, `document-open`, `document-close`) and maintaining own lookup table when a document is opened and closed.
I see there's a few nice conveniences, but how useful is this over setting data on the `ScintillaWidget`? e.g. [*Git ChangeBar* does this fairly fine](https://github.com/geany/geany-plugins/blob/master/git-changebar/src/gcb-plu...).
but how useful is this over setting data on the ScintillaWidget
I've done that before too. With Geany's current architecture where the model and view are so tightly bound, it's effectively the same. The main difference is the `plugin_` functions in this PR which provide the helper API to namespace the data and to remove it when the plugin is unloaded.
As previously mentioned, I'm not fixed on the particular implementation in the PR, I'm only really interested in the `plugin_` API functions, which I think allow the implementation to be done differently without affecting anything.
So yeah, until Geany is re-architected in a way that there's no 1:1 relationship between documents and Scintilla widgets, doing it that way would work. It would make the tiny ce6c0fad9481549c62c308a42ccfe774bacf3b55 commit not required, and require re-working 7b2b9609e473bdc591010a68794df46dac52e1be to operate on the Scintilla widgets' keyed data lists (assuming it's possible to access them in the same way).
Using scintillawidget for this is just hacky and we shouldnt recommend or even suggest this :-/
For the change, I don't think it's truly necessary but OK with me if you like the added convenience.
Merged #1203.
As a comparison, [the code here](https://gist.github.com/codebrainz/78a23f54970b975e0898ae544c2f5bbd) which used to be required in each plugin that needed it is reduced to:
```c plugin_document_set_data_full(plugin, doc, g_strdup("Hi"), g_free); ```
github-comments@lists.geany.org