Re: [Geany-Devel] Plugin API design question/change proposal
On 26 May 2014 09:10, Matthew Brush wrote: > Hi, > > If nobody's opposed to this, then I'll start working on it shortly. At worst > we'll end up with some new `*private.h` files in the `src` directory and > maybe find some buggy plugins and/or unintentionally-public API. > > I'll try to keep the private headers changes separate from the removal of > the `geanyfunctions.h`/function pointer macros stuff, since in theory they > are independent of each other. To be clear, I support us cleaning up the API headers and moving to a "single include" model like Glib/GTK. Cheers Lex > > Cheers, > Matthew Brush > > > On 14-05-20 02:29 AM, Matthew Brush wrote: >> >> Hi, >> >> Does anyone know why the plugin API was designed to use a bunch of >> structures containing function pointers, hidden behind macros in >> geanyfunctions.h? I found the commit where this stuff was added >> initially (ie. plugin ABI 2-3) but it doesn't mention why it was done >> like this and I tried to search the mailing list archives but Gmane >> won't let me search and the other mailing list archive doesn't go back >> that far. >> >> Somebody mentioned it might be because Windows doesn't export symbols by >> default, but it still doesn't explain why this way chosen over >> explicitly exporting the symbols using >> __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this. >> >> As mentioned in the "Proxy Plugins" thread I'm looking into making the >> headers scanned by GObject-introspection to automate binding the plugin >> API to non-C plugins, but with all of the private stuff and public stuff >> mixed together in public headers, it will be hard to do this. >> >> Assuming there isn't actually much of a reason for the chosen existing >> function pointer/structure/macro mechanism, is anybody opposed to just >> making the API available in the normal C way where the public functions >> goes in one header that plugins (and core) can use, and one header where >> the private stuff goes, that doesn't get installed? >> >> Just to see the work involved, I tried to do this with the build, >> document and editor functions. It makes the public/private more >> explicit, removes lots of extra code, makes only one place to update >> when adding new functions to the API, doesn't force plugins to import a >> bunch of private stuff indirectly by #including , still >> makes the symbols available/exported on Windows, and does it without >> breaking the official (ie. doxygen-commented) parts of the API (but it >> would need an ABI bump). The experimental changes to build, document, >> and editor functions are here in my header-cleanup-private branch, based >> ontop of my "header-cleanup" branch that I have an open PR for: >> >> build.h/buildprivate.h >> >> https://github.com/codebrainz/geany/commit/0b1221ce85905a066adfaae62fc73470b34af4d5 >> >> >> document.h/documentprivate.h >> >> https://github.com/codebrainz/geany/commit/f5e03bbee2c4edc8fe66c8e0762ef86e1f130857 >> >> >> editor.h/editorprivate.h >> >> https://github.com/codebrainz/geany/commit/1534f196d626494b57d408f780dfde022f18dd07 >> >> >> What we could do for commonly used existing private stuff: >> >> https://github.com/codebrainz/geany/commit/ac02d5330af2bd2cfcff45609f0e5b02a8b9d72a >> >> >> (Sorry, I just linked the relevant commits instead of the branch so >> people don't have to figure out which specific ones I'm talking about >> amidst all the other unrelated ones. It's mostly just to give an idea of >> what I'm talking about.) >> >> I think this would be a fairly big improvement overall and it would >> finally allow us to sort out what's really private and what's really >> public, which will make bindings generated by scanning the headers much >> easier. >> >> Some common stuff that wasn't public before (ie. doxygen-commented) but >> that is still used in plugins could be added as "deprecated" to the >> public header or if useful could be properly added with a doxygen >> comment. This will avoid excessive breakage where plugins were using >> private stuff. >> >> Any opinions, suggestions, reasons about the original design welcome. >> >> Cheers, >> Matthew Brush >> ___ >> Devel mailing list >> Devel@lists.geany.org >> https://lists.geany.org/cgi-bin/mailman/listinfo/devel > > > ___ > Devel mailing list > Devel@lists.geany.org > https://lists.geany.org/cgi-bin/mailman/listinfo/devel ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Another set of Plugin API questions
On 14-05-25 05:16 PM, Lex Trotman wrote: On 26 May 2014 09:38, Matthew Brush wrote: [snip] Another example is `filetype_id` which is the enum type in `filetypes.h` that holds the various filetype IDs (ex. GEANY_FILETYPES_C, GEANY_FILETYPES_HTML, etc.). It's completely undocumented, but is used (as gint type) in functions such as `filetypes_index()`. Is it fair to say this whole enum is private, or should it rather be considered public since it was exposed to plugins via the include of `filetypes.h` in `geanyplugin.h`? Again sounds like its a mistake, since the example for `filetypes_index()` uses `GEANY_FILETYPES_C`. At least the enum should be documented. A lazy person like me would say "the enum members are part of the API, even though they are not individually documented since their use is obvious" in that documentation. IMO, we could just stick some empty/dummy `/**! */` comments after the enumerators if they are indeed public and their meaning is obvious (or whatever trick to get Doxygen to recognize them). Cheers, Matthew Brush ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Another set of Plugin API questions
On 26 May 2014 10:19, Matthew Brush wrote: > On 14-05-25 04:38 PM, Matthew Brush wrote: >> >> [snip] >> >> And my final question: do we support individual includes of Geany's >> headers that were available? For example, if someone, for some crazy >> reason wanted to move `struct GeanyDocument` to `document-blah.h` (fake >> example), is it a plugin bug if they directly included "document.h" >> where this type was previously, or is it an API bug because we moved the >> type to another file, even though we added an and include for the new >> header inside the central include file `geanyplugin.h`? >> > > For a real life example of this, consider this comment from the recent > `header-cleanup` merge: > > https://github.com/geany/geany/commit/4efcbab33234d13a7c6d1dea2901535d9317e4e1#diff-b0d9f5f851974f08ac7c7bd620163687R28 > > And the likewise patch/PR for the plugin in GP: > > https://github.com/codebrainz/geany-plugins/commit/307880c0778e27d191305ec6c68355af2512d1d4 > > In this case I moved `GeanyApp` which was in `geany.h` (which was exposed to > plugins via `geanyplugin.h`) to `app.h` of its own, but it broke GeanyLua > that included all the stuff that `geanyplugin.h` would've provided it, > individually. > > Is this a plugin bug or a public API break/bug? Since everything is currently documented in its header (in the files tab) then moving which header its in is an API break (but not an ABI break). See previous comment supporting "single include" paradigm. Cheers Lex > > > Cheers, > Matthew Brush > > ___ > Devel mailing list > Devel@lists.geany.org > https://lists.geany.org/cgi-bin/mailman/listinfo/devel ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] [RFC]: Public API comments in headers
On 14-05-25 05:23 PM, Lex Trotman wrote: On 26 May 2014 09:50, Matthew Brush wrote: [snip] If we moved to having public headers that just included actual public symbols, I think it would be advantageous to have those headers totally commented/documented rather than requiring the user to download Geany's source code and cross-reference functions in it to access the comments/docs, or getting hold of the generated Doxygen API documentation. Its neater to have them in the public .h file sure, but I suggest that they are going to be less likely to be kept up to date that way. Most of the editing happens in the .c files (I don't even open the .h much of the time) so the doxygen comments in headers become out of sight, out of mind. If, in future changes, we used the `G_MODULE_EXPORT` stuff, and kept it at the site of the definition (in the C file), it might provide a queue for anyone changing it that it has related boilerplate, being an exported/public function. It might help a bit here at least. Cheers, Matthew Brush ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] [RFC]: Public API comments in headers
On 26 May 2014 09:50, Matthew Brush wrote: > Hi, > > As part of working on cleaning up the exposed API to plugins I got to > thinking about where our comments are located. While it's nice to keep the > API-documentation-comments right at the definitions of the functions in > their respective .c source files, since we only install the headers as a > public reference (not even the plugin API docs IIUC), should we not move the > API-documentation-comments, that we already diligently ensure exist and are > fairly complete, into the headers where they are accessible to plugin > developers? They are intended to be "accessible" via the generated doxygen documentation, its not necessary to read the source. > > If we moved to having public headers that just included actual public > symbols, I think it would be advantageous to have those headers totally > commented/documented rather than requiring the user to download Geany's > source code and cross-reference functions in it to access the comments/docs, > or getting hold of the generated Doxygen API documentation. Its neater to have them in the public .h file sure, but I suggest that they are going to be less likely to be kept up to date that way. Most of the editing happens in the .c files (I don't even open the .h much of the time) so the doxygen comments in headers become out of sight, out of mind. > > Does anyone feel really strongly about this? No, but if you do move them, leave a comment in the .c file on any API visible function as a note that its in the API, and should not be changed without changing the API docs and bumping the API/ABI. Cheers Lex > > Cheers, > Matthew Brush > ___ > Devel mailing list > Devel@lists.geany.org > https://lists.geany.org/cgi-bin/mailman/listinfo/devel ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Another set of Plugin API questions
On 14-05-25 04:38 PM, Matthew Brush wrote: [snip] And my final question: do we support individual includes of Geany's headers that were available? For example, if someone, for some crazy reason wanted to move `struct GeanyDocument` to `document-blah.h` (fake example), is it a plugin bug if they directly included "document.h" where this type was previously, or is it an API bug because we moved the type to another file, even though we added an and include for the new header inside the central include file `geanyplugin.h`? For a real life example of this, consider this comment from the recent `header-cleanup` merge: https://github.com/geany/geany/commit/4efcbab33234d13a7c6d1dea2901535d9317e4e1#diff-b0d9f5f851974f08ac7c7bd620163687R28 And the likewise patch/PR for the plugin in GP: https://github.com/codebrainz/geany-plugins/commit/307880c0778e27d191305ec6c68355af2512d1d4 In this case I moved `GeanyApp` which was in `geany.h` (which was exposed to plugins via `geanyplugin.h`) to `app.h` of its own, but it broke GeanyLua that included all the stuff that `geanyplugin.h` would've provided it, individually. Is this a plugin bug or a public API break/bug? Cheers, Matthew Brush ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Another set of Plugin API questions
On 26 May 2014 09:38, Matthew Brush wrote: > Hi, > > Is it fair to say that any function that is not represented in > `geanyfunctions.h`/`plugindata.h` and any types that are not documented with > a `/**` or other Doxygen comment are to be considered "private" with respect > to the plugin API? My understanding, and the documentation (as you mention below says) that this is the case. But don't presume that there are no mistakes in the construction of the API public functions as you point out below. It might need adjustment. > > For example there are some types/enums in `build.h` that have comments that > start with `/* *` (note the space in-between asterisks). I presume that's to > keep them from being made "public" by not allowing Doxygen to scan them. Is > that accurate? If that's the case, how do we handle that some of the public > functions in `build.h` require these enums in order to be useful (ie. use > parameters of those types)? It sounds like its a mistake, remember that build.h additions to the API were removed, then put back, and those items seem to have been missed in the process. Anything (typedef, struct, enum etc) that is needed to use public functions should be public and documented. Where a struct/typedef may be public but validly not have the members documented is the case where it is returned from an API function to be passed to another API function without its contents being used. The struct etc should be documented as being "opaque" in this case to make it clear that its not a mistake that the members were not part of the API. > > Another example is `filetype_id` which is the enum type in `filetypes.h` > that holds the various filetype IDs (ex. GEANY_FILETYPES_C, > GEANY_FILETYPES_HTML, etc.). It's completely undocumented, but is used (as > gint type) in functions such as `filetypes_index()`. Is it fair to say this > whole enum is private, or should it rather be considered public since it was > exposed to plugins via the include of `filetypes.h` in `geanyplugin.h`? Again sounds like its a mistake, since the example for `filetypes_index()` uses `GEANY_FILETYPES_C`. At least the enum should be documented. A lazy person like me would say "the enum members are part of the API, even though they are not individually documented since their use is obvious" in that documentation. > > Along these lines, in `geanyplugin.h` there are includes of many common > headers, with a comment about only including headers that expose types, not > just functions. Having had these headers exposed to plugins (and also being > documented in Doxygen API docs), does that mean that every > symbol/function/type in these headers is part of the public API and should > not be hidden inside of private headers, for example (see other thread about > private headers). Is it a plugin bug if they used any undocumented > symbols/functions/signals/types? Functions that are not doxygened in *either* the .h or .c file are not meant to be in the API. So they can safely be made private (from the point of view of the API). IIUC the tradition was for functions to be documented in the .c files rather than the .h files since functions in the API did not need to be in the header, since the macro trick was intended to make them visible, but types only exist in the headers, so they need documenting there. > > FWIW, I noticed the plugin API docs says right on the main page: > >> Warning >> Do not use any symbol not in the documentation - it may change. > > And my final question: do we support individual includes of Geany's headers > that were available? For example, if someone, for some crazy reason wanted > to move `struct GeanyDocument` to `document-blah.h` (fake example), is it a > plugin bug if they directly included "document.h" where this type was > previously, or is it an API bug because we moved the type to another file, > even though we added an and include for the new header inside the central > include file `geanyplugin.h`? If the type was documented as being in document.h then its a bug or API break in Geany for it to be moved because it no longer conforms to the documented API. So that suggests that the "single include" approach is the best, then so long as the type is included it doesn't matter from where. Geany is then free to move stuff around as it requires. So, I would agree with a "single plugin" approach. That should be done once (being an API break). It isn't a big API break though, only plugin files that don't include "geanyplugin.h" will be affected. Cheers Lex PS thanks for the effort to clean up the API. > > Cheers, > Matthew Brush > ___ > Devel mailing list > Devel@lists.geany.org > https://lists.geany.org/cgi-bin/mailman/listinfo/devel ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
[Geany-Devel] [RFC]: Public API comments in headers
Hi, As part of working on cleaning up the exposed API to plugins I got to thinking about where our comments are located. While it's nice to keep the API-documentation-comments right at the definitions of the functions in their respective .c source files, since we only install the headers as a public reference (not even the plugin API docs IIUC), should we not move the API-documentation-comments, that we already diligently ensure exist and are fairly complete, into the headers where they are accessible to plugin developers? If we moved to having public headers that just included actual public symbols, I think it would be advantageous to have those headers totally commented/documented rather than requiring the user to download Geany's source code and cross-reference functions in it to access the comments/docs, or getting hold of the generated Doxygen API documentation. Does anyone feel really strongly about this? Cheers, Matthew Brush ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
[Geany-Devel] Another set of Plugin API questions
Hi, Is it fair to say that any function that is not represented in `geanyfunctions.h`/`plugindata.h` and any types that are not documented with a `/**` or other Doxygen comment are to be considered "private" with respect to the plugin API? For example there are some types/enums in `build.h` that have comments that start with `/* *` (note the space in-between asterisks). I presume that's to keep them from being made "public" by not allowing Doxygen to scan them. Is that accurate? If that's the case, how do we handle that some of the public functions in `build.h` require these enums in order to be useful (ie. use parameters of those types)? Another example is `filetype_id` which is the enum type in `filetypes.h` that holds the various filetype IDs (ex. GEANY_FILETYPES_C, GEANY_FILETYPES_HTML, etc.). It's completely undocumented, but is used (as gint type) in functions such as `filetypes_index()`. Is it fair to say this whole enum is private, or should it rather be considered public since it was exposed to plugins via the include of `filetypes.h` in `geanyplugin.h`? Along these lines, in `geanyplugin.h` there are includes of many common headers, with a comment about only including headers that expose types, not just functions. Having had these headers exposed to plugins (and also being documented in Doxygen API docs), does that mean that every symbol/function/type in these headers is part of the public API and should not be hidden inside of private headers, for example (see other thread about private headers). Is it a plugin bug if they used any undocumented symbols/functions/signals/types? FWIW, I noticed the plugin API docs says right on the main page: > Warning > Do not use any symbol not in the documentation - it may change. And my final question: do we support individual includes of Geany's headers that were available? For example, if someone, for some crazy reason wanted to move `struct GeanyDocument` to `document-blah.h` (fake example), is it a plugin bug if they directly included "document.h" where this type was previously, or is it an API bug because we moved the type to another file, even though we added an and include for the new header inside the central include file `geanyplugin.h`? Cheers, Matthew Brush ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Plugin API design question/change proposal
Hi, If nobody's opposed to this, then I'll start working on it shortly. At worst we'll end up with some new `*private.h` files in the `src` directory and maybe find some buggy plugins and/or unintentionally-public API. I'll try to keep the private headers changes separate from the removal of the `geanyfunctions.h`/function pointer macros stuff, since in theory they are independent of each other. Cheers, Matthew Brush On 14-05-20 02:29 AM, Matthew Brush wrote: Hi, Does anyone know why the plugin API was designed to use a bunch of structures containing function pointers, hidden behind macros in geanyfunctions.h? I found the commit where this stuff was added initially (ie. plugin ABI 2-3) but it doesn't mention why it was done like this and I tried to search the mailing list archives but Gmane won't let me search and the other mailing list archive doesn't go back that far. Somebody mentioned it might be because Windows doesn't export symbols by default, but it still doesn't explain why this way chosen over explicitly exporting the symbols using __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this. As mentioned in the "Proxy Plugins" thread I'm looking into making the headers scanned by GObject-introspection to automate binding the plugin API to non-C plugins, but with all of the private stuff and public stuff mixed together in public headers, it will be hard to do this. Assuming there isn't actually much of a reason for the chosen existing function pointer/structure/macro mechanism, is anybody opposed to just making the API available in the normal C way where the public functions goes in one header that plugins (and core) can use, and one header where the private stuff goes, that doesn't get installed? Just to see the work involved, I tried to do this with the build, document and editor functions. It makes the public/private more explicit, removes lots of extra code, makes only one place to update when adding new functions to the API, doesn't force plugins to import a bunch of private stuff indirectly by #including , still makes the symbols available/exported on Windows, and does it without breaking the official (ie. doxygen-commented) parts of the API (but it would need an ABI bump). The experimental changes to build, document, and editor functions are here in my header-cleanup-private branch, based ontop of my "header-cleanup" branch that I have an open PR for: build.h/buildprivate.h https://github.com/codebrainz/geany/commit/0b1221ce85905a066adfaae62fc73470b34af4d5 document.h/documentprivate.h https://github.com/codebrainz/geany/commit/f5e03bbee2c4edc8fe66c8e0762ef86e1f130857 editor.h/editorprivate.h https://github.com/codebrainz/geany/commit/1534f196d626494b57d408f780dfde022f18dd07 What we could do for commonly used existing private stuff: https://github.com/codebrainz/geany/commit/ac02d5330af2bd2cfcff45609f0e5b02a8b9d72a (Sorry, I just linked the relevant commits instead of the branch so people don't have to figure out which specific ones I'm talking about amidst all the other unrelated ones. It's mostly just to give an idea of what I'm talking about.) I think this would be a fairly big improvement overall and it would finally allow us to sort out what's really private and what's really public, which will make bindings generated by scanning the headers much easier. Some common stuff that wasn't public before (ie. doxygen-commented) but that is still used in plugins could be added as "deprecated" to the public header or if useful could be properly added with a doxygen comment. This will avoid excessive breakage where plugins were using private stuff. Any opinions, suggestions, reasons about the original design welcome. Cheers, Matthew Brush ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Geany/plugins win32 waf build/install
On Sat, 24 May 2014 11:55:47 +0200 Enrico Tröger wrote: > On 18/05/14 19:08, Dimitar Zhekov wrote: > > On Sun, 18 May 2014 15:30:21 +0200 > > Enrico Tröger wrote: > > > >> On 16/05/14 18:41, Dimitar Zhekov wrote: > >>> > >>> The prefix problem is workarounded. Since geany.pc is installed in > >>> \geany\path\lib\pkgconfig now, win32 pkg-config automatically > >>> uses /geany/path, replacing \ with / and ignoring the prefix setting. > >>> [...] > >> > >> Not sure whether I completely understand the problem here. > > > > With a prefix with backshahes in geany.pc (unlike the other variables, > > and any other .pc files), my gcc decides than c:\geany\path/include is > > c:geanypath/include. The same goes for c:\geany\path/lib et cetera, > > Ah got it, finally :). > Should be better in 1bacf869e076ef37caa768196d21d941a657d1e7, Broke it completely. :) > I changed the code from hard-coded forward slashes to os.path.join(). > Still, there are tons of forward slashes in the build system. If these > cause problems at any stage, let me know. Again: the problem is that the _backward_ slashes in the prefix (and now the other variables) in geany.pc are treated as escape characters by cpp, gcc or whatever. With geany.pc in "c:\what\ever\lib\pkgconfig" (in 1.24+), win32 pkg-config ignores $prefix and uses "c:/what/ever". That doesn't any more, because $includedir became "c:/what/ever\include", processed as "c:/what/everinclude" ("\i" does not stand for anything particular, thus simply "i"). The fix is to revert 1bacf869e076ef37caa768196d21d941a657d1e7. It would nice to have the prefix in geany.pc with forward slashes, but not really important - all glib/gtk+ .pc files contain prefixes like c:/devel/target/059c48de6b739307c37648aba3005b29, and pkg-config ignores them as described above. > Some applications can handle the forward slashes even on Windows, > others not. And IIRC also Windows sometimes handle the forward > slashes properly, sometimes not. The Windows API-s handle forward slashes as separators since DOS 5. The only thing I'm not sure is whether server names ("//server/path" instead of "X:/path") work, and that may cause problems. > >>> The plugin installation, however, > >>> creates a \lib directory on the installation drive, with all > >>> lib.dll.a files > > > Yeah but I don't know yet where this path (\lib) is set in Waf and how > to change it. I didn't consider it a real problem yet apart from the > fact that it sort of clutters the file system. > I might will have a look at it if time permits. I tried to find that as well, but couldn't. The entire installation of lib*.dll.a is useless, nobody will link against our plugin DLL-s. -- E-gards: Jimmy ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel