GNOME Bugzilla – Bug 640601
Patch: make document-loader independent of GSettings
Last modified: 2014-01-03 15:19:08 UTC
Created attachment 179342 [details] [review] Patch v1 document-loader needs the "encodings-auto-detected" setting located in "org.gnome.gedit.preferences.encodings". The document-loader is created only in gedit-document, and document-loader has a reference to the document. The property "encodings-auto-detected" has been added in gedit-document. The document-loader gets the value with the reference. When a new document is created, the property has to be bind with GSettings (or it could also be bind indirectly). A new document is created in several places: - one time in gedit-view-frame - and at some other places in the tests So to avoid duplicated code, a new function has been added, which create a new document and bind the setting and finally returns the doc. The problem was that I didn't know where to place it, so it is in gedit-utils temporarily. Another problem will come when we will try to place gedit-view-frame in the lib. A solution would be to create another property which contains the same value. If we always apply this solution, we will see a lot of hierarchies of the same properties. To me it doesn't look nice. And if the hierarchy is big, the execution could be slow (a signal is generally slow). Another solution is to create an interface with a getter and/or a setter for each setting. This interface would be implemented in the apps by using GSettings or whatever. This centralize the settings. A drawback is that if an app want to use only a few widgets, it is difficult to know which settings has to be implemented (we can also give good documentation).
I forgot to say that the property is used only with streams, so we can test like this: $ echo "blabla" | jhbuild run gedit
For the solution with the getters and setters, we can provide default implementation for each setting. The getter would always return the default value, and the setter would do nothing. By doing this, if the app doesn't care about some settings, the functions don't need to be overridden. Another advantage is that if the app use only a few widgets, the developer looks which setting must be overridden and takes only those that he wants to be able to change the default value. This class would be a singleton. To be able to subclass this singleton, here is what I proposed some days ago for GeditApp: > And since GeditApp is a singleton, GeditApp must be instantiated with > the subclass. For this I've thought adding a static function in GeditApp > like gedit_app_set_instance (). We call gedit_sub_app_get_default () > first, which call gedit_app_set_instance () with the instance of > GeditSubApp. It works but there is maybe a better approach.
For the subclassable singleton thing, you should have a look at how this is done in libpeas for the PeasEngine class.
Thanks. But I don't think it'll be needed, maybe for GeditApp in the future, because there is an easier way to centralize the settings (see [1]): simply create a singleton class with a property for each setting. The apps can bind these properties with GSettings (only those that are needed and useful for the app), and the lib use directly these properties instead of GSettings (at some places it will be only a get, and at some other places we have to connect the notify signal). With this approach ExoBinding from XFCE [2] could be useful to bind two properties together. [1] http://mbarnes.livejournal.com/1899.html [2] http://docs.xfce.org/api/exo/exo-Binding-Properties-Functions.html
Just for the record, you don't need exobinding, since latest version of glib you can use GObject for this. See: http://library.gnome.org/devel/gobject/unstable/GBinding.html
Review of attachment 179342 [details] [review]: As said by irc, let's go step by step and let's get the data from gsettings directly in gedit-document.
Instead of using a special interface with properties, you can also just have a requirement on GSettings. The application can set a scheme and provide the gsettings to the document, or loader, or anything else? I would prefer not introducing another object again with properties (like GeditPreferences) now that we just got rid of that :)
(In reply to comment #7) > Instead of using a special interface with properties, you can also just have a > requirement on GSettings. The application can set a scheme and provide the > gsettings to the document, or loader, or anything else? I would prefer not > introducing another object again with properties (like GeditPreferences) now > that we just got rid of that :) Well, my first idea was that the app provides the string "org.gnome.gedit" (for instance), but it's not flexible enough. Instead the app can provides strings with the full path for each setting, is that what you mean?
Created attachment 179408 [details] [review] Patch v2 Now the binding is done in gedit_document_new(). I tried to put this in gedit_document_init() but it doesn't work, I don't know why.
Yeah that's true because on init properties of the same object are still not built. For this you have to use the constructed method.
Obsolete, see bug #721016.