Hi Nick,
The documented functions in build.h were intended to be added to the API after discussions with Enrico in the past. Not sure why they weren't. What needs to be done to do that?
Cheers Lex
2009/10/27 ntrel@users.sourceforge.net:
Revision: 4367 http://geany.svn.sourceforge.net/geany/?rev=4367&view=rev Author: ntrel Date: 2009-10-26 14:59:47 +0000 (Mon, 26 Oct 2009)
Log Message:
Move function doc-comments to build.c so they stay in sync. Note: these functions are still not in the API.
Modified Paths:
trunk/ChangeLog trunk/src/build.c trunk/src/build.h
Modified: trunk/ChangeLog
--- trunk/ChangeLog 2009-10-26 13:06:51 UTC (rev 4366) +++ trunk/ChangeLog 2009-10-26 14:59:47 UTC (rev 4367) @@ -8,6 +8,9 @@ sciwrappers.h, build.h. (This helps to get compiler warnings for uses of functions not in the API). Warning: any plugins that include these headers should remove them.
- src/build.c, src/build.h:
- Move function doc-comments to build.c so they stay in sync. Note:
- these functions are still not in the API.
2009-10-25 Enrico Tröger <enrico(dot)troeger(at)uvena(dot)de>
Modified: trunk/src/build.c
--- trunk/src/build.c 2009-10-26 13:06:51 UTC (rev 4366) +++ trunk/src/build.c 2009-10-26 14:59:47 UTC (rev 4367) @@ -201,7 +201,7 @@ gboolean printbuildcmds = PRINTBUILDCMDS;
static GeanyBuildCommand **cl[GEANY_GBG_COUNT][GEANY_BCS_COUNT] = {
- /* GEANY_BCS_DEF, GEANY_BCS_FT, GEANY_BCS_HOME_FT, GEANY_BCS_PREF,
- /* GEANY_BCS_DEF, GEANY_BCS_FT, GEANY_BCS_HOME_FT, GEANY_BCS_PREF,
* GEANY_BCS_FT_PROJ, GEANY_BCS_PROJ */ { &ft_def, NULL, NULL, NULL, NULL, NULL }, { &non_ft_def, NULL, NULL, &non_ft_pref, NULL, &non_ft_proj }, @@ -306,7 +306,7 @@ gint below, gint *from) { /* Note: parameter below used in macros above */
GeanyFiletype *ft = NULL; gint sink, *fr = &sink;
@@ -444,7 +444,23 @@ }
-/* remove the specified command, cmd < 0 remove whole group */ +/** Remove the specified Build menu item.
- Makes the specified menu item configuration no longer exist. This
- is different to setting fields to blank because the menu item
- will be deleted from the configuration file on saving
- (except the system filetypes settings @see Build Menu Configuration
- section of the Manual).
- @param src the source of the menu item to remove.
- @param grp the group of the command to remove.
- @param cmd the index (from 0) of the command within the group. A negative
- value will remove the whole group.
- If any parameter is out of range does nothing.
- @see build_menu_update
- **/
void build_remove_menu_item(GeanyBuildSource src, GeanyBuildGroup grp, gint cmd) { GeanyBuildCommand *bc; @@ -462,7 +478,20 @@ }
-/* get the build build command for the specified menu item */ +/** Get the @a GeanyBuildCommand structure for the specified Build menu item.
- Get the command for any menu item specified by @a src, @a grp and @a cmd even if it is
- hidden by higher priority commands.
- @param src the source of the specified menu item.
- @param grp the group of the specified menu item.
- @param cmd the index of the command within the group.
- @return a pointer to the @a GeanyBuildCommand structure or @a NULL if it doesn't exist.
- This is a pointer to an internal structure and must not be freed.
- @see build_menu_update
- **/
GeanyBuildCommand *build_get_menu_item(GeanyBuildSource src, GeanyBuildGroup grp, gint cmd) { GeanyBuildCommand *bc; @@ -475,6 +504,21 @@ }
+/** Get the @a GeanyBuildCommand structure for the menu item.
- Get the current highest priority command specified by @a grp and @a cmd. This is the one
- that the menu item will use if activated.
- @param grp the group of the specified menu item.
- @param cmd the index of the command within the group.
- @param src pointer to @a gint to return which source provided the command. Ignored if @a NULL.
- Values are one of @a GeanyBuildSource but returns a signed type not the enum.
- @return a pointer to the @a GeanyBuildCommand structure or @a NULL if it doesn't exist.
- This is a pointer to an internal structure and must not be freed.
- @see build_menu_update
- **/
/* parameter checked version of get_build_cmd for external interface */ GeanyBuildCommand *build_get_current_menu_item(GeanyBuildGroup grp, gint cmd, gint *src) { @@ -1373,8 +1417,20 @@ }
-/* Call this whenever build menu items need to be enabled/disabled.
- Uses current document (if there is one) when doc == NULL */
+/** Update the build menu to reflect changes in configuration or status.
- Sets the labels and number of visible items to match the highest
- priority configured commands. Also sets sensitivity if build commands are
- running and switches executes to stop when commands are running.
- @param doc The current document, if available, to save looking it up.
- If @c NULL it will be looked up.
- Call this after modifying any fields of a GeanyBuildCommand structure.
- @see Build Menu Configuration section of the Manual.
- **/
void build_menu_update(GeanyDocument *doc) { gint i, cmdcount, cmd, grp;
Modified: trunk/src/build.h
--- trunk/src/build.h 2009-10-26 13:06:51 UTC (rev 4366) +++ trunk/src/build.h 2009-10-26 14:59:47 UTC (rev 4367) @@ -181,79 +181,14 @@
/* build menu functions */
-/** Update the build menu to reflect changes in configuration or status.
- Sets the labels and number of visible items to match the highest
- priority configured commands. Also sets sensitivity if build commands are
- running and switches executes to stop when commands are running.
- @param doc The current document, if available, to save looking it up.
- If @c NULL it will be looked up.
- Call this after modifying any fields of a GeanyBuildCommand structure.
- @see Build Menu Configuration section of the Manual.
- **/
void build_menu_update(GeanyDocument *doc);
void build_toolbutton_build_clicked(GtkAction *action, gpointer user_data);
-/** Remove the specified Build menu item.
- Makes the specified menu item configuration no longer exist. This
- is different to setting fields to blank because the menu item
- will be deleted from the configuration file on saving
- (except the system filetypes settings @see Build Menu Configuration
- section of the Manual).
- @param src the source of the menu item to remove.
- @param grp the group of the command to remove.
- @param cmd the index (from 0) of the command within the group. A negative
- value will remove the whole group.
- If any parameter is out of range does nothing.
- @see build_menu_update
- **/
void build_remove_menu_item(GeanyBuildSource src, GeanyBuildGroup grp, gint cmd);
-/** Get the @a GeanyBuildCommand structure for the specified Build menu item.
- Get the command for any menu item specified by @a src, @a grp and @a cmd even if it is
- hidden by higher priority commands.
- @param src the source of the specified menu item.
- @param grp the group of the specified menu item.
- @param cmd the index of the command within the group.
- @return a pointer to the @a GeanyBuildCommand structure or @a NULL if it doesn't exist.
- This is a pointer to an internal structure and must not be freed.
- @see build_menu_update
- **/
GeanyBuildCommand *build_get_menu_item(GeanyBuildSource src, GeanyBuildGroup grp, gint cmd);
-/** Get the @a GeanyBuildCommand structure for the menu item.
- Get the current highest priority command specified by @a grp and @a cmd. This is the one
- that the menu item will use if activated.
- @param grp the group of the specified menu item.
- @param cmd the index of the command within the group.
- @param src pointer to @a gint to return which source provided the command. Ignored if @a NULL.
- Values are one of @a GeanyBuildSource but returns a signed type not the enum.
- @return a pointer to the @a GeanyBuildCommand structure or @a NULL if it doesn't exist.
- This is a pointer to an internal structure and must not be freed.
- @see build_menu_update
- **/
GeanyBuildCommand *build_get_current_menu_item(GeanyBuildGroup grp, gint cmd, gint *src);
BuildMenuItems *build_get_menu_items(gint filetype_idx);
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. _______________________________________________ Geany-commits mailing list Geany-commits@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-commits
On Tue, 27 Oct 2009 07:48:27 +1100 Lex Trotman elextr@gmail.com wrote:
The documented functions in build.h were intended to be added to the API after discussions with Enrico in the past. Not sure why they weren't. What needs to be done to do that?
Sorry I hadn't raised this before.
Basically there are 2 issues:
1. The proposed build API is not designed to be developed in future without breaking the ABI. See: http://geany.org/manual/dev/hacking.html#plugin-api-abi-design
BTW, I only wrote that after you designed the build API ;-)
Not breaking the ABI is a goal of any future plugin API extensions, so we don't have to rebuild all plugins so often.
2. Not sure if the proposed API functions are the best/tidiest way to do it, it would be best if there was a plugin that wanted to use these functions so we can see what is needed. We need to avoid wanting an API change in future, this is much more important than breaking the ABI. Also the build code still hasn't been tested in a release; not sure if we want to change the implementation (maybe not), some of which may be exposed to the API.
Regards, Nick
Hi,
2009/10/27 Nick Treleaven nick.treleaven@btinternet.com:
On Tue, 27 Oct 2009 07:48:27 +1100 Lex Trotman elextr@gmail.com wrote:
The documented functions in build.h were intended to be added to the API after discussions with Enrico in the past. Not sure why they weren't. What needs to be done to do that?
Sorry I hadn't raised this before.
Basically there are 2 issues:
- The proposed build API is not designed to be developed in future
without breaking the ABI.
Hmmm not sure why, the functions in the interface take enums or integers, the plugins don't index arrays directly. So long as enums and the GeanyBuildCommand structure are only extended I think it will be ABI compatible and it will be API compatible if new fields are added, the only thing you can't do is remove enums or fields or change their meaning or type and that restriction is always going to exist on anything visible in the API, like GeanyFiletype :-)
The only way of getting around this is to make all the interface via functions/macros and don't expose any enums or structures. But thats a lot of work for limited gain and would probably affect other parts of the API more than build.
See:
http://geany.org/manual/dev/hacking.html#plugin-api-abi-design
BTW, I only wrote that after you designed the build API ;-)
Not breaking the ABI is a goal of any future plugin API extensions, so we don't have to rebuild all plugins so often.
Excellent goal.
In which case you might need to add that plugins should not save and restore structures since they may miss fields added later or fail to comply with changes to structure invariants required (neither of which will trigger an ABI/API step according to the doc).
- Not sure if the proposed API functions are the best/tidiest way to do
Thats true, I only tried to provide a minimal interface to avoid the problems of backward compatibility, but not sure if its necessary or sufficient.
it, it would be best if there was a plugin that wanted to use these functions so we can see what is needed. We need to avoid wanting an API change in future, this is much more important than breaking the ABI. Also the build code still hasn't been tested in a release; not sure if we want to change the implementation (maybe not),
You must be psychotic ... ermm psychic I mean :-), to get a cleaner implementation so that the UI is cleaner I am looking at some changes to GeanyBuildCommand, but only additions (so far). Oh, and I'm sure you'll be pleased that I no longer jam two small integers into user_data. So lets leave things out of the API it until a plugin builder asks.
some of which may be
exposed to the API.
Regards, Nick _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On Wed, 28 Oct 2009 10:56:30 +1100 Lex Trotman elextr@gmail.com wrote:
The documented functions in build.h were intended to be added to the API after discussions with Enrico in the past. Not sure why they weren't. What needs to be done to do that?
Sorry I hadn't raised this before.
Basically there are 2 issues:
- The proposed build API is not designed to be developed in future
without breaking the ABI.
Hmmm not sure why, the functions in the interface take enums or integers, the plugins don't index arrays directly. So long as enums and the GeanyBuildCommand structure are only extended I think it will be ABI compatible and it will be API compatible if new fields are added, the only thing you can't do is remove enums or fields or change their meaning or type and that restriction is always going to exist on anything visible in the API, like GeanyFiletype :-)
I should have been clearer - I haven't really studied the build API, but a potential ABI problem is that GeanyBuildCommand has:
gchar *entries[GEANY_BC_CMDENTRIES_COUNT];
What if we want to increase GEANY_BC_CMDENTRIES_COUNT - we'd have to break the ABI.
The only way of getting around this is to make all the interface via functions/macros and don't expose any enums or structures. But thats a lot of work for limited gain and would probably affect other parts of the API more than build.
See:
http://geany.org/manual/dev/hacking.html#plugin-api-abi-design
BTW, I only wrote that after you designed the build API ;-)
Not breaking the ABI is a goal of any future plugin API extensions, so we don't have to rebuild all plugins so often.
Excellent goal.
In which case you might need to add that plugins should not save and restore structures since they may miss fields added later or fail to comply with changes to structure invariants required (neither of which will trigger an ABI/API step according to the doc).
Not sure if I'm with you. Doesn't this cover it: * Don't let plugins allocate any structs (stack or heap).
What do you mean by structure invariants?
- Not sure if the proposed API functions are the best/tidiest way to do
Thats true, I only tried to provide a minimal interface to avoid the problems of backward compatibility, but not sure if its necessary or sufficient.
it, it would be best if there was a plugin that wanted to use these functions so we can see what is needed. We need to avoid wanting an API change in future, this is much more important than breaking the ABI. Also the build code still hasn't been tested in a release; not sure if we want to change the implementation (maybe not),
You must be psychotic ... ermm psychic I mean :-), to get a cleaner implementation so that the UI is cleaner I am looking at some changes to GeanyBuildCommand, but only additions (so far). Oh, and I'm sure you'll be pleased that I no longer jam two small integers into user_data. So lets leave things out of the API it until a plugin builder asks.
OK, sounds good. Don't worry too much about reworking things, we can always provide getter/setter functions to avoid exposing any ugly/ABI brittle things if needed.
Regards, Nick
2009/10/28 Nick Treleaven nick.treleaven@btinternet.com:
On Wed, 28 Oct 2009 10:56:30 +1100 Lex Trotman elextr@gmail.com wrote:
The documented functions in build.h were intended to be added to the API after discussions with Enrico in the past. Not sure why they weren't. What needs to be done to do that?
Sorry I hadn't raised this before.
Basically there are 2 issues:
- The proposed build API is not designed to be developed in future
without breaking the ABI.
Hmmm not sure why, the functions in the interface take enums or integers, the plugins don't index arrays directly. So long as enums and the GeanyBuildCommand structure are only extended I think it will be ABI compatible and it will be API compatible if new fields are added, the only thing you can't do is remove enums or fields or change their meaning or type and that restriction is always going to exist on anything visible in the API, like GeanyFiletype :-)
I should have been clearer - I haven't really studied the build API, but a potential ABI problem is that GeanyBuildCommand has:
gchar *entries[GEANY_BC_CMDENTRIES_COUNT];
What if we want to increase GEANY_BC_CMDENTRIES_COUNT - we'd have to break the ABI.
Ok, true
The only way of getting around this is to make all the interface via functions/macros and don't expose any enums or structures. But thats a lot of work for limited gain and would probably affect other parts of the API more than build.
See:
http://geany.org/manual/dev/hacking.html#plugin-api-abi-design
BTW, I only wrote that after you designed the build API ;-)
Not breaking the ABI is a goal of any future plugin API extensions, so we don't have to rebuild all plugins so often.
Excellent goal.
In which case you might need to add that plugins should not save and restore structures since they may miss fields added later or fail to comply with changes to structure invariants required (neither of which will trigger an ABI/API step according to the doc).
Not sure if I'm with you. Doesn't this cover it:
- Don't let plugins allocate any structs (stack or heap).
I was actually thinking about a plugin that tried to save and restore structure contents so it could restore values after running or save them to file for restore next time. The struct might be allocated prior to the plugin being loaded.
What do you mean by structure invariants?
Required relationships between members of structures, for example if GeanyBuildCommand has a non-null label it might reasonably require a non-null command and working directory to save checking all the time. (Note actually I think I test all of them all the time but its an example :-)
The API functions I provided all checked parameters then called internal versions that didn't.
- Not sure if the proposed API functions are the best/tidiest way to do
Thats true, I only tried to provide a minimal interface to avoid the problems of backward compatibility, but not sure if its necessary or sufficient.
it, it would be best if there was a plugin that wanted to use these functions so we can see what is needed. We need to avoid wanting an API change in future, this is much more important than breaking the ABI. Also the build code still hasn't been tested in a release; not sure if we want to change the implementation (maybe not),
You must be psychotic ... ermm psychic I mean :-), to get a cleaner implementation so that the UI is cleaner I am looking at some changes to GeanyBuildCommand, but only additions (so far). Oh, and I'm sure you'll be pleased that I no longer jam two small integers into user_data. So lets leave things out of the API it until a plugin builder asks.
OK, sounds good. Don't worry too much about reworking things, we can always provide getter/setter functions to avoid exposing any ugly/ABI brittle things if needed.
Ok, and they can keep the invariants true too.
Regards, Nick _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On Thu, 29 Oct 2009 08:56:42 +1100 Lex Trotman elextr@gmail.com wrote:
Not breaking the ABI is a goal of any future plugin API extensions, so we don't have to rebuild all plugins so often.
Excellent goal.
In which case you might need to add that plugins should not save and restore structures since they may miss fields added later or fail to comply with changes to structure invariants required (neither of which will trigger an ABI/API step according to the doc).
Not sure if I'm with you. Doesn't this cover it:
- Don't let plugins allocate any structs (stack or heap).
I was actually thinking about a plugin that tried to save and restore structure contents so it could restore values after running or save them to file for restore next time. The struct might be allocated prior to the plugin being loaded.
I think plugins shouldn't try to do that. If it was necessary, we could add functions to the API for any structures that need to be synced from/to disk.
What do you mean by structure invariants?
Required relationships between members of structures, for example if GeanyBuildCommand has a non-null label it might reasonably require a non-null command and working directory to save checking all the time. (Note actually I think I test all of them all the time but its an example :-)
The API functions I provided all checked parameters then called internal versions that didn't.
Basically plugins probably shouldn't really set any of the exposed struct fields, only read them. Maybe we should mark them as const but I'm not sure if this would cause issues.
Anyway, we could improve the API docs (doxygen) to warn about these things. (The HACKING file is for working on Geany's core).
Regards, Nick
2009/10/29 Nick Treleaven nick.treleaven@btinternet.com:
On Thu, 29 Oct 2009 08:56:42 +1100 Lex Trotman elextr@gmail.com wrote:
Not breaking the ABI is a goal of any future plugin API extensions, so we don't have to rebuild all plugins so often.
Excellent goal.
In which case you might need to add that plugins should not save and restore structures since they may miss fields added later or fail to comply with changes to structure invariants required (neither of which will trigger an ABI/API step according to the doc).
Not sure if I'm with you. Doesn't this cover it:
- Don't let plugins allocate any structs (stack or heap).
I was actually thinking about a plugin that tried to save and restore structure contents so it could restore values after running or save them to file for restore next time. The struct might be allocated prior to the plugin being loaded.
I think plugins shouldn't try to do that. If it was necessary, we could add functions to the API for any structures that need to be synced from/to disk.
Thats what I was saying, this should be on the list separate to allocate :-)
What do you mean by structure invariants?
Required relationships between members of structures, for example if GeanyBuildCommand has a non-null label it might reasonably require a non-null command and working directory to save checking all the time. (Note actually I think I test all of them all the time but its an example :-)
The API functions I provided all checked parameters then called internal versions that didn't.
Basically plugins probably shouldn't really set any of the exposed struct fields, only read them. Maybe we should mark them as const but I'm not sure if this would cause issues.
Yes if all the API functions returned const pointers. That doesn't stop some conniving plugin writer from casting them non-const but that is deliberate rather than accidental misuse so bugs are firmly the plugin writers problem.
Anyway, we could improve the API docs (doxygen) to warn about these things. (The HACKING file is for working on Geany's core).
Or maybe both so there's a chance it will be seen, remember we're talking about programmers reading the manual :-)
Cheers Lex
Regards, Nick _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On Fri, 30 Oct 2009 09:03:37 +1100 Lex Trotman elextr@gmail.com wrote:
I was actually thinking about a plugin that tried to save and restore structure contents so it could restore values after running or save them to file for restore next time. The struct might be allocated prior to the plugin being loaded.
I think plugins shouldn't try to do that. If it was necessary, we could add functions to the API for any structures that need to be synced from/to disk.
Thats what I was saying, this should be on the list separate to allocate :-)
...
What do you mean by structure invariants?
Required relationships between members of structures, for example if GeanyBuildCommand has a non-null label it might reasonably require a non-null command and working directory to save checking all the time. (Note actually I think I test all of them all the time but its an example :-)
The API functions I provided all checked parameters then called internal versions that didn't.
Basically plugins probably shouldn't really set any of the exposed struct fields, only read them. Maybe we should mark them as const but I'm not sure if this would cause issues.
Yes if all the API functions returned const pointers. That doesn't stop some conniving plugin writer from casting them non-const but that is deliberate rather than accidental misuse so bugs are firmly the plugin writers problem.
The problem with that is that you might want to call an API function that changed the struct, but you'd have to pass it a const struct or cast the const away.
Anyway, we could improve the API docs (doxygen) to warn about these things. (The HACKING file is for working on Geany's core).
Or maybe both so there's a chance it will be seen, remember we're talking about programmers reading the manual :-)
Plugin writers have to read the API docs anyway, they don't need to read HACKING. I don't want to duplicate things really.
Regards, Nick