From: Benny M. <ben...@gm...> - 2009-10-07 13:39:58
|
Doug, few comments on the config code. 1/signal callbacks are added with notify_add. What if something that tracks a setting is destroyed? Then the function no longer exists. So there should be a way to remove a callback. So, just as callback.py, I think notify_add should return a unique identifier. Windows/classes that track a setting and can be destroyed, should store this identifier, and call a notify_remove(identifier) before they are destroyed. 2/I really find the data-views setting an ugly beast: register('interface.data-views', 'GrampletView,PersonView,RelationshipView,' 'FamilyListView,PedigreeView,EventView,SourceView,' 'PlaceView,MediaView,RepositoryView,NoteView') How to do this better? How about a views section in the ini: [views] 1=GrampletView 2=PersonView 3=RelationshipView ..... and then a convenience function to register a section as a list, and function to obtain a section as a list: get_list(....), and a way to do notify_add on a change in a section. It would simplify writing the view manager I have in mind. Or do you see simpler ways to do this? I don't think configparser can handle lists, the only thing I found was: http://forums.devshed.com/python-programming-11/configparser-to-create-a-list-529694.html 3/the way to register to a setting change is strange. parts = path.split("/") # /apps/gramps/section/key section = parts[-2] key = parts[-1] This is a left over from the gconf gnome scheme, where apps/gramps is where something is stored in gconf. I see no reason to leave this ambiguity in. Somting like notify_add(section, key, func) seems better now. In light of 2/, allowing key to be None would be nice. Otherwise I would say that for me you can commit the patch. Benny 2009/10/7 Doug Blank <dou...@gm...>: > On Wed, Oct 7, 2009 at 5:03 AM, Benny Malengier > <ben...@gm...> wrote: >> 2009/10/7 Doug Blank <dou...@gm...>: >>> I've put up a patch here: >>> >>> http://www.gramps-project.org/bugs/view.php?id=3272 >>> >>> This cleans up the config system quite a bit, and simplifies the work >>> to make it as easy as can be, while still providing the types of >>> integrity checks that one would expect. The patch: >>> >>> 1) renames all of the Config.CONSTANTS to strings >>> 2) requires a setting to be registered before use >>> 3) requires that a changed value has the same type as the default >>> 4) keeps the defaults and current values in a ConfigManager class >>> 5) uses the singleton pattern manager, ala the rest of gramps >>> 6) removed all of the unused functions >>> 7) removed all of the unused parameters on remaining functions >>> 8) cleanup all of the calling programs (this patch is 150k) >>> >>> Briefly, I removed the Config directory, along with __init__.py, >>> _GrampsConfigKeys.py, _GrampsIniKeys.py, and replaced it with a >>> single, simple Config.py. This can easily be reused in other projects, >>> too, to make reuse of gramps bits easier. >>> >>> The .ini file changed slightly, and I moved it over to ~/.gramps/gramps.ini. >> >> Perhaps we should use gramps32.ini, and have a setting if oldini has >> been imported already. >> Then, we can have gramps33.ini later and we developers can run trunk >> and branch side by side more easily (eg, different database >> directories). > > Good idea. > >> You have webcal.py in the patch, probably should be removed > > Oops. > >> A lot of work changing the Config.LARGE_CONST, but you are right, it >> is not really needed. These things are not something we need to >> change, and if we do, we can do it with a one line perl command. >> >> I'll look at your new Config.py in later. It should be config.py. We >> should think about the split over cli, gen and gui. Config was one of >> the problems there >> > > Thanks for the feedback, I'll upload a new patch when I can with these > fixes, and some unit tests. > > -Doug > >> Benny >>> I need to add some unit tests. I think that this is a pretty nice >>> balance between just using ConfigParser, and have something completely >>> custom. I also didn't add the signals to auto-save the file yet. I'm >>> still looking at that, and how it would work nicely with a series of >>> changes (such as adding a bunch of new settings all at once). This is >>> the same issue as preventing a series of screen refreshes when they >>> are unnecessary. >>> >>> Feedback welcomed, >>> >>> -Doug >>> >>> On Sat, Oct 3, 2009 at 8:17 PM, Doug Blank <dou...@gm...> wrote: >>>> Devs, >>>> >>>> I've been wrestling with the manner in which we have evolved our >>>> gramps Config code, and I'd like to propose a re-organization. Most of >>>> the cruft has accumulated as we evolved away from gconf, towards a >>>> file-based .ini solution. It is overly complicated, and unnecessarily >>>> limited. >>>> >>>> First, here are some of the problems that I see with the current config code: >>>> >>>> 1) In order to add a new configure setting, you have to give it two >>>> names. For example, in: >>>> >>>> src/Config/_GrampsConfigKeys.py >>>> >>>> you need to give it a Python constant name (such as EXPORT_NO_PRIVATE) >>>> and you also have to give it a section/setting/typecode like >>>> ('export', 'no-private', 0). Also, you give it a default, in a >>>> different location in this file. You have to know the type of the >>>> setting to find the default as it is keyed off of the (section, >>>> setting, typecode) tuple: >>>> >>>> default_value = { >>>> ... >>>> EXPORT_NO_PRIVATE : True, >>>> ...} >>>> >>>> 2) In order to look up or change a setting, you have to use the awkward syntax: >>>> >>>> Config.get(Config.EXPORT_NO_PRIVATE) >>>> Config.set(Config.FILTER, obj.get_active()) >>>> >>>> 3) Currently there is no manner for a third-party plugin to have >>>> easily accessible settings, short of writing their own config code and >>>> creating a separate file. For example, plugins have typically added >>>> their settings in the file above, or have created their own config >>>> file that the user has to find to edit. >>>> >>>> 4) If you change a config setting, sometimes you have to wait until >>>> you restart gramps for that change to take effect. Some of these >>>> changes could perhaps take effect immediately with a better designed >>>> config handler. >>>> >>>> 5) Currently, there are only 3 types of values that you can currently >>>> put in the ~/.gramps/keys.ini file: strings, ints, and booleans. >>>> >>>> The solution that I'd like to propose should fix many of these, and >>>> help lead to a solution for the others. In addition, it is less code, >>>> and is a more intuitive API: >>>> >>>> a) Create a standard way to add sections, settings, and default >>>> values. Something like: >>>> >>>> Config.init("export.no-private", True) >>>> >>>> The given name would be THE name for the setting, and would exactly >>>> match the keys.ini file. >>>> >>>> b) Create a standard way to retrieve and set the settings: >>>> >>>> Config.get("export.no-private") >>>> Config.set("export.no-private", False) >>>> >>>> and to remove them: >>>> >>>> Config.delete("test-plugin.linecount") >>>> Config.delete("test-plugin") >>>> >>>> c) Currently, all keys are stored internally as strings, and converted >>>> to a type on lookup. I'd like to create a format in the keys.ini that >>>> stores the setting type, so that you can get return the appropriate >>>> value and type (0, "0", or False). The easiest manner would be to use >>>> a very simple syntax for the parser (For example, if it starts with a >>>> quote, it is a string, else if it is True or False is is a bool, else >>>> if it has a dot in it it is a float, else an int). >>>> >>>> To give some protection from overwriting system level settings, one >>>> can require plugins to write only to specially named sections (such as >>>> [geoview-plugin]). I think we can still ensure that any settings must >>>> have the same type as the default to give some sanity checks. I would >>>> also require that you can only Config.set a name that has been >>>> Config.initialized. >>>> >>>> In addition, I think that we *could* be backwards compatible (if we >>>> wanted) but I'd advocate for a wholesale update. >>>> >>>> The only other concern I can think of is that keys.ini might grow too >>>> big, or that a third-party plugin could corrupt the file. But I think >>>> we can make sure that we have it properly formatted when you write it, >>>> and check for proper values when setting. I'd rather have a little bit >>>> larger keys.ini file with all of the settings there than having config >>>> files spread around. In fact, gramplets could use this config system >>>> too, and remove the gramplets.ini file. >>>> >>>> It could be possible in the future to have the Preferences just be a >>>> handy way to change all of the values in this file. >>>> >>>> Other comments, concerns, ideas? >>>> >>>> -Doug >>>> >>> >>> ------------------------------------------------------------------------------ >>> Come build with us! The BlackBerry(R) Developer Conference in SF, CA >>> is the only developer event you need to attend this year. Jumpstart your >>> developing skills, take BlackBerry mobile applications to market and stay >>> ahead of the curve. Join us from November 9 - 12, 2009. Register now! >>> http://p.sf.net/sfu/devconference >>> _______________________________________________ >>> Gramps-devel mailing list >>> Gra...@li... >>> https://lists.sourceforge.net/lists/listinfo/gramps-devel >>> >> > |