Dennis Kasprzyk and I have been discussing some changes to how options
are initialized.
Problems with how options are currently initialized.
1. Helper functions are not used to initialize options, which means that
if we make a change to the option structure, all option initialization
code needs to be updated. Using helper functions will also reduces the
amount of duplicate code.
2. No convenient way to get the initial value of an option once it's
been modified.
3. An option can't be initialized without compiz running.
2 and 3 are of interest to configuration backends that like to be able
to do offline readout of option information and some kind of standard
default value. 3, might be hard to provide for all kind of plugins as
some might rely on a unique plugin loader to be present so I'm not sure
that I want to recommend a configuration backend to do this. However,
it's easy to provide for plugins built as shared libraries and I don't
want to prevent anyone from doing this if they wish to.
Here's our current idea for how we can solve these issues:
Add
typedef Bool (*InitPlugin(Display|Screen)OptionProc) (CompOption *o,
int index);
or
typedef Bool (*InitPlugin(Display|Screen)OptionsProc) (CompOption **o);
and the number of display and screen options to the plugin vTable. I
prefer the function prototype with the index as it's a bit more
flexible.
We can add a set of option initialization macros to compiz.h or helper
functions to a library, which will make the initPluginOption function in
each plugin minimal.
Action options will be changed to contain a string or KeySym instead of
the current key-code.
- David
Am Donnerstag, 29. M?rz 2007 11:02 schrieb David Reveman:> Dennis Kasprzyk and I have been discussing some changes to how options > are initialized. > > Problems with how options are currently initialized. > > 1. Helper functions are not used to initialize options, which means that > if we make a change to the option structure, all option initialization > code needs to be updated. Using helper functions will also reduces the > amount of duplicate code. > > 2. No convenient way to get the initial value of an option once it's > been modified. > > 3. An option can't be initialized without compiz running. > > > 2 and 3 are of interest to configuration backends that like to be able > to do offline readout of option information and some kind of standard > default value. 3, might be hard to provide for all kind of plugins as > some might rely on a unique plugin loader to be present so I'm not sure > that I want to recommend a configuration backend to do this. However, > it's easy to provide for plugins built as shared libraries and I don't > want to prevent anyone from doing this if they wish to. > > > Here's our current idea for how we can solve these issues: > > Add > > typedef Bool (*InitPlugin(Display|Screen)OptionProc) (CompOption *o, > int index); > > or > > typedef Bool (*InitPlugin(Display|Screen)OptionsProc) (CompOption **o); > > and the number of display and screen options to the plugin vTable. I > prefer the function prototype with the index as it's a bit more > flexible. > > We can add a set of option initialization macros to compiz.h or helper > functions to a library, which will make the initPluginOption function in > each plugin minimal. > > Action options will be changed to contain a string or KeySym instead of > the current key-code. > > - David > > _______________________________________________ > compiz mailing list > compiz@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/compizCurrently there are two types of configuration tools. Some with fixed functionality and some autogenerated. To improve the quality of the autogenerated tools I would like to make this proposal about additional values in the CompOption struct and the plugin vtable. Additions to the CompOption struct: char * group/char * subgroup : Ability to group sets of options and to give this sets a name. char *hints : A semicolon separated list of string that inform the configuration tool to handle this option in a special way. A string value could have a hint "image" or "file" to inform the configuration tool to open a file dialog when the user wants to set this option. We all could workout here a set of hints, that all configuration tools would understand. Additions to the plugin vTable: char* category: A plugin can define to belong to a category like "effects" or "accessibility", so that a configuration tool can group plugins together. We could workout possible categories later here. All this values would be optional and could be exposed over the dbus plugin or any other configuration system. Dennis
David Reveman wrote:> Dennis Kasprzyk and I have been discussing some changes to how options > are initialized. > > Problems with how options are currently initialized. > > 1. Helper functions are not used to initialize options, which means that > if we make a change to the option structure, all option initialization > code needs to be updated. Using helper functions will also reduces the > amount of duplicate code. >Maybe we could create a whole library of extra helper functions. That way some people could do it without the helpers, or you can use them for speed of development.> 2. No convenient way to get the initial value of an option once it's > been modified. >No, this was my problem when writing ini. The way I see it is that gconf should be able to revert to the default easily, ini might need some work.> Here's our current idea for how we can solve these issues: > > Add > > typedef Bool (*InitPlugin(Display|Screen)OptionProc) (CompOption *o, > int index); > > or > > typedef Bool (*InitPlugin(Display|Screen)OptionsProc) (CompOption **o); >Are these to replace the current getOptions items in the vtable? Or are they more of a 'reset option' function? Would it be possible to add a reference back to the plugin as well so that plugin loaders can handle options for other plugins?
Dennis Kasprzyk wrote:> Do you also want a> CompOptionTypeExecutable,Thats hardly ever needed, when it is used, it normally refers to a command rather than directly to an executable. If you wanted this then you could always create a file type and have it parse the mime type etc.> CompOptionTypeWindow, >CompOptionTypeMatch ? Surely this is the same thing (ie. it matches windows). I am not sure what a window type option would be otherwise.> CompOptionTypePng,This would be covered under the image type.> CompOptionType*.foo ? >I dont think that would compile properly ;)
Dennis Kasprzyk wrote:> Am Donnerstag, 29. M?rz 2007 18:33 schrieben Sie: > >> Dennis Kasprzyk wrote: >> >>> The problem is that we already have additional data in the plugins. So we >>> should be consistent here and remove also the long/short decriptions of >>> the option struct and or the plugin vtable and move it to an additional >>> file or have everything in the plugin. A mixture of both is simply >>> stupid. I can live with both because I can use bcop to generate what I >>> want. >>> >> To me, it would make more sense to pull out >> the descriptions rather than add extra identifiers. >> >> This might be tricky to implement. Maybe each config >> backend could also be responsible for loading these >> extra pieces of information. That way each distro can >> easily change defaults as well as descriptions if they >> choose. Dbus etc would then also have access to this >> information. >> >> Then if you need extra information for your particular >> backend you can include it in your data. >> > > Let me summarize what options do we have: > > 1. We keep the current system (lon/short decription in the plugin) and add > additional descriptions to a special file. This is stupid because all > descriptions should be a the same place. > > 2. We add everything to the plugin. This makes it easy for other plugins > (dbus) to get the information, but don't really makes sense because this > information's are not needed for the functionality of the plugin. > > 3. We move everything out of a plugin into a XML file and generate also the > options code for the plugin out of the XML file. We have BCOP that already > does it, but this would force everyone to use it, and there are some things > that cannot be done with BCOP. > > 4. We move everything that ins not needed for plugins functionality > (long/short descriptions ...) to a special file. I would prefer to do it in a > ini file style: > > [PLUGIN] > short=foo > long=lonf FOO > author> email> category> ... > > [SCREEN/"option name"] > short> long> group> subgroup> hints> ... > > To prevent that other plugins/configuration tool need to create a parser for > that files, we could provide a library that would be a part of the core, that > would read this plugin.info files. There are already plans to have the system > to readout of options without a running compiz in a library so we could also > add this to the library. With this plugins and configuration tools would have > an easy access to the additional plugin/options data. For > internationalisation we could simply create plugin.info.es_ES ... files that > would contain the translated values. The library would provide access to get > translated and original values. > > With this system there should be no problem to create configuration system > dependent files (gconf schema). > > I don't think that we should store the default values in such file. The > default value is a part of the plugin. Each configuration system should > provide a system for distribution specific defaults. This is easy for dbus, > but should be also doable for other configuration systems. > > Dennis > >We could always add an option 5 where there a getOptionMetaData function which the storage plugins could be responsible for wrapping They could then choose how to store this information. Maybe shortDesc could be kept and everything moved to a metadata category. That way you could run Compiz with any storage plugin and any external app will be able to relay whatever data it needs to get. If the backend does not support something then it can just return blank and the application can deal with that. ie. if the ini plugin does not support the group attribute then it will always return blank, this way BSM could be used with any settings plugin, but it would maybe not group things as well (for example) dbus could easily be modified to include this as part of the existing getMetadata function, it would just return a dict of extra attributes along with all the core information.
On Thu, 2007-03-29 at 16:56 +0100, Mike Dransfield wrote:> David Reveman wrote: > > Dennis Kasprzyk and I have been discussing some changes to how options > > are initialized. > > > > Problems with how options are currently initialized. > > > > 1. Helper functions are not used to initialize options, which means that > > if we make a change to the option structure, all option initialization > > code needs to be updated. Using helper functions will also reduces the > > amount of duplicate code. > > > > Maybe we could create a whole library of extra helper > functions. That way some people could do it without > the helpers, or you can use them for speed of development.Yes, it's probably a good idea to just go ahead and create a libcompiz that plugins can choose to link to.> > > 2. No convenient way to get the initial value of an option once it's > > been modified. > > > > No, this was my problem when writing ini.Yes, I put it in the list of current problems as I remembered it from the ini discussions. If we're changing things, it makes sense to make sure that this gets fixed as well.> > The way I see it is that gconf should be able to revert to > the default easily, ini might need some work. > > > Here's our current idea for how we can solve these issues: > > > > Add > > > > typedef Bool (*InitPlugin(Display|Screen)OptionProc) (CompOption *o, > > int index); > > > > or > > > > typedef Bool (*InitPlugin(Display|Screen)OptionsProc) (CompOption **o); > > > > > Are these to replace the current getOptions items > in the vtable? Or are they more of a 'reset option' function?No these are just additional functions that are used to initialize options. The existing Get/SetPluginOption functions are still used to get/set option values. However, we should also update the get/set functions to accept an index if we select the init option function prototype with an index. Example: for (i = 0; i < p->vTable->nScreenOption; i++) { CompOption op; (*p->vTable->initScreenOption) (p, i, &op); /* 'op' is is now initialized with default values for screen option 'i' */ }> > Would it be possible to add a reference back to the plugin > as well so that plugin loaders can handle options for other > plugins?Yes, it should be: typedef Bool (*InitPlugin(Display|Screen)OptionProc) (CompPlugin *plugin, CompOption *option, int index); - David
Dennis Kasprzyk wrote:> Am Freitag, 30. M?rz 2007 17:48 schrieben Sie: >> We could always add an option 5 where there a getOptionMetaData >> function which the storage plugins could be responsible for wrapping >> They could then choose how to store this information. >> >> > In this system a plugin developer would have to create metedata information > for each storage backend, what is a stupid idea to me. >I thought the idea was that this sort of thing could be generated at compile-time using whatever system necessary. I think the plugin developer should provide at least short description because that gives people a clue as to what it actually does. I think the extra attribute stuff should be added by other people to give a more human readable version.>> Maybe shortDesc could be kept and everything moved to a metadata >> category. That way you could run Compiz with any storage plugin >> and any external app will be able to relay whatever data it needs to get. >> >> > I think that there is no reason to keep short desc in the plugin if we move > other metadata out of the plugin. We should be consistent in this case. > >The short description provides at least a basic > 1 word explanation of the what the option does, otherwise people will need to work it out from just the name.>> If the backend does not support something then it can just return blank >> and the application can deal with that. ie. if the ini plugin does not >> support >> the group attribute then it will always return blank, this way BSM could be >> used with any settings plugin, but it would maybe not group things as well >> (for example) >> >> > BSM has it's own storage system and will not use the ini or the gconf plugin. >I thought you were planning a libbs plugin instead of linking core to libbs directly? This way you are free to add whatever information you like, but other plugins do not depend on them just to compile. It will also mean that people will be able to modify or extend these attributes without changing code.>> dbus could easily be modified to include this as part of the existing >> getMetadata >> function, it would just return a dict of extra attributes along with all >> the core >> information. >> > With my proposal dbus would provide the metadata that comes from libconmpiz. > >But you would need to modify core every time you wanted to add some extra information about the option. This idea could be further extended so that other plugins can read this extended information.
On Sun, 2007-04-01 at 01:39 +0200, Dennis Kasprzyk wrote:> Am Samstag, 31. M?rz 2007 10:12 schrieben Sie: > > On Thu, 2007-03-29 at 17:23 +0200, Dennis Kasprzyk wrote: > > > Am Donnerstag, 29. M?rz 2007 11:02 schrieb David Reveman: > > > > Dennis Kasprzyk and I have been discussing some changes to how options > > > > are initialized. > > > > > > > > Problems with how options are currently initialized. > > > > > > > > 1. Helper functions are not used to initialize options, which means > > > > that if we make a change to the option structure, all option > > > > initialization code needs to be updated. Using helper functions will > > > > also reduces the amount of duplicate code. > > > > > > > > 2. No convenient way to get the initial value of an option once it's > > > > been modified. > > > > > > > > 3. An option can't be initialized without compiz running. > > > > > > > > > > > > 2 and 3 are of interest to configuration backends that like to be able > > > > to do offline readout of option information and some kind of standard > > > > default value. 3, might be hard to provide for all kind of plugins as > > > > some might rely on a unique plugin loader to be present so I'm not sure > > > > that I want to recommend a configuration backend to do this. However, > > > > it's easy to provide for plugins built as shared libraries and I don't > > > > want to prevent anyone from doing this if they wish to. > > > > > > > > > > > > Here's our current idea for how we can solve these issues: > > > > > > > > Add > > > > > > > > typedef Bool (*InitPlugin(Display|Screen)OptionProc) (CompOption *o, > > > > int > > > > index); > > > > > > > > or > > > > > > > > typedef Bool (*InitPlugin(Display|Screen)OptionsProc) (CompOption **o); > > > > > > > > and the number of display and screen options to the plugin vTable. I > > > > prefer the function prototype with the index as it's a bit more > > > > flexible. > > > > > > > > We can add a set of option initialization macros to compiz.h or helper > > > > functions to a library, which will make the initPluginOption function > > > > in each plugin minimal. > > > > > > > > Action options will be changed to contain a string or KeySym instead of > > > > the current key-code. > > > > > > > > - David > > > > > > > > _______________________________________________ > > > > compiz mailing list > > > > compiz@lists.freedesktop.org > > > > http://lists.freedesktop.org/mailman/listinfo/compiz > > > > > > Currently there are two types of configuration tools. Some with fixed > > > functionality and some autogenerated. To improve the quality of the > > > autogenerated tools I would like to make this proposal about additional > > > values in the CompOption struct and the plugin vtable. > > > > > > Additions to the CompOption struct: > > > char * group/char * subgroup : Ability to group sets of options and to > > > give this sets a name. > > > > > > char *hints : A semicolon separated list of string that inform the > > > configuration tool to handle this option in a special way. A string value > > > could have a hint "image" or "file" to inform the configuration tool to > > > open a file dialog when the user wants to set this option. We all could > > > workout here a set of hints, that all configuration tools would > > > understand. > > > > > > Additions to the plugin vTable: > > > char* category: A plugin can define to belong to a category like > > > "effects" or "accessibility", so that a configuration tool can group > > > plugins together. We could workout possible categories later here. > > > > > > All this values would be optional and could be exposed over the dbus > > > plugin or any other configuration system. > > > > I've read the whole thread. To summarize, there's a desire to assign > > much more meta-data than the current short/long descriptions to plugins, > > which to me seems like a very valid request. > > > > Question is how we allow this in the best possible way. > > > > Mike made a good point about the plugin writer not always being the best > > person to be responsible for providing all meta-data. However, I think > > it will be the best solution in most cases and it's always going to be > > easy to have a table of meta-data in the configuration tool that will > > override the plugin provided data so I'm not very concerned about this. > > > > Adding new fields to the plugin vTable every time someone comes up with > > some new useful meta-data seems very inconvenient. > > > > The most obvious way to provide this meta-data is to have an xml file > > paired with each plugin. That way new tags can easily be added as > > desired. I think that this is what we want in most cases but I'm still > > very interested in allowing plugins to be fully functional without any > > external file references. Imagine compiz running on a machine without a > > writable file-system and loading plugins remotely or plugins that > > dynamically create other plugins... > > > > My suggestion is that we remove the two description fields from the > > vTable and add a "char *metadata" field that contains meta information > > as xml data. A helper function that reads meta data for an option from > > an xml file could easily be provided for those plugins that like to keep > > this in an external xml file. > > > > - David > > Great idea. Should we do the same for the options?Yes, feel free to start working on some patches for this if you like. I wont have time to do it in the next couple of days. - David