GNOME Bugzilla – Bug 762803
Support relocatable schemas
Last modified: 2017-12-19 12:00:55 UTC
Should support relocatable schemas! These are unnecessarily tough to deal with on the command line, dconf-editor could make it easy.
?? What do you have in mind? Relocatable schemas are supported as possible, I think.
Created attachment 322587 [details] Screenshot of dialog It treats values from relocatable schemas the same as it does for schemas that have been uninstalled. Is it not possible to do better? At least in Rhythmbox's case, all of these settings are defined in a gschema.xml, and I can reset them to default values by using /usr/bin/gsettings directly, so I would expect the same to be possible with dconf-editor?
(In reply to Michael Catafro from comment #2) > It treats values from relocatable schemas the same as it does for schemas > that have been uninstalled. Is it not possible to do better? No. There’s no way to know if a dconf db entry without fixed path has been created following a relocatable schema, a now-removed fixed schema, or by a manual request. And I don’t want to rely on guessing; that would just come with a bunch of stupid behaviors. > I can reset them to default values by using /usr/bin/gsettings directly, so > I would expect the same to be possible with dconf-editor? Reseting such a key means removing the entry from the dconf db. That’s a wanted feature, mainly for cleaning settings of uninstalled apps, but that implies you cannot edit the key value anymore from dconf-editor (the key has been removed), so the UI needs to be thought very carefully.
What about extending GSettings API with something like const gchar * g_settings_schema_key_get_schema_id (GSettingsSchemaKey *key); How feasible is that?
Actually, I see now that it won't help, since the problem is associating the path with the schema.
(In reply to Arnaud B. from comment https://bugzilla.gnome.org/show_bug.cgi?id=768275#c16) > Ah, yes, that’s a fun test. For having your opinion on things that always > conflicted in my mind: > * you’re doing a little danse with schema_already_installed_there, wouldn’t > it be easier to have a mapping of schemas to a list of paths/a path spec/a > list of path specs, instead of a mapping of a path spec to a schema? It > would be easier for users, easier for creating the UI that will manage that, > and at least schema names are known to be unique… From what I can tell, paths are unique too, right? And they make a better dictionary key because you can have multiple paths mapped to the same schema. Inverting the relation means you need lists as values. Also, the central concept of the UI is the path, not the schema, so the lookups are path-based, not schema-based. On the other hand, using schemas on the left side has the advantage of being a syntax closer to the gsettings command line. > * you cover the application of a schema to a path, to more than one > unrelated paths, to all subdirectories of a path, but do you cover also the > “/org/project/application/plugins/*/schema1/” case? it’s hard to test, and > you know your code better than I do. ^^ Should work. Wild card can be used on every segment of the path. The only restriction is that a single "*" matches against a single segment. For example, "/*/*/*/" matches against every path with depth 3. Do you know of an app that has a situation like that?
Created attachment 364303 [details] [review] Preliminary patch This patch applies on top of the patch-set Arnaud posted at [1]. [1] https://bugzilla.gnome.org/show_bug.cgi?id=768275#c17
(In reply to Davi from comment #6) > From what I can tell, paths are unique too, right? And they make a better > dictionary key because you can have multiple paths mapped to the same > schema. Inverting the relation means you need lists as values. Also, the > central concept of the UI is the path, not the schema, so the lookups are > path-based, not schema-based. > > On the other hand, using schemas on the left side has the advantage of being > a syntax closer to the gsettings command line. There are sometimes two schemas applied at the same path. I think I’d be quite more comfortable with schemas “at left” and path “at right”, and with a “a{sas}” type for convenience and uniqueness. Note that the UI might change for something more schema-based at one point. > Should work. Wild card can be used on every segment of the path. The only > restriction is that a single "*" matches against a single segment. For > example, "/*/*/*/" matches against every path with depth 3. Looks great. I remember of *something* using ‘//’, without ‘*’. Is the wildcard something suggested into dconf documentation, or something you came with? > Do you know of an app that has a situation like that? Builder had something like that for its coding language configuration, but I think it has changed. You probably know that, but for now, you’re not applying the schema at a given path (without wild card) if one of the keys is not edited. Don’t hesitate to add test case(s) under ‘/ca/desrt/dconf-editor/Demo/’. Happy hacking!
(In reply to Arnaud B. from comment #8) > There are sometimes two schemas applied at the same path. Should both be applied simultaneously? Current Directory class does not support that, if I understand it, and only has a warning flag. > I think I’d be > quite more comfortable with schemas “at left” and path “at right”, and with > a “a{sas}” type for convenience and uniqueness. Note that the UI might > change for something more schema-based at one point. Well, if it can't be avoided having a list on the right side, I don't care which is in on the left, really. I've switched to schema IDs on the left. > > Should work. Wild card can be used on every segment of the path. The only > > restriction is that a single "*" matches against a single segment. For > > example, "/*/*/*/" matches against every path with depth 3. > > Looks great. I remember of *something* using ‘//’, without ‘*’. Is the > wildcard something suggested into dconf documentation, or something you came > with? I went with "*" because I never saw a path containing that, but from the documentation on "g_settings_new_with_path" the only rule is "no consecutive slashes" //, so I'll go with that now: empty segments match anything. > > Do you know of an app that has a situation like that? > > Builder had something like that for its coding language configuration, but I > think it has changed. > > You probably know that, but for now, you’re not applying the schema at a > given path (without wild card) if one of the keys is not edited. Don’t > hesitate to add test case(s) under ‘/ca/desrt/dconf-editor/Demo/’. Happy > hacking! It is much smarter now. I think it covers every case where a path can be inferred from the path specs. It will only fail matching when there is a wild card and absolutely nothing to match against it. I'll think of some test cases to add to Demo.
Created attachment 364371 [details] [review] Preliminary patch - Schema IDs on the left - Smarter matching of specs - Small refactoring of how the flag warning_multiple_schemas is raised - Fix instantiating GLib.Settings from relocatable schemas in other places (by sharing the settings in the GSettingsKey object)
(In reply to Davi from comment #9) > (In reply to Arnaud B. from comment #8) > > There are sometimes two schemas applied at the same path. > > Should both be applied simultaneously? Current Directory class does not > support that, if I understand it, and only has a warning flag. It has to be supported, whatever UI is chosen for that. What I have in my mind for now is to group keys by schema in the list, even if two dispayed keys are in fact at the same path. That won’t work with current code. Yes, that’s one of the heads of the Hydra. Note, I see four theorical reasons why multiple schemas could be applied to the same path: * one quite theorical: the same application is installed twice (local and Flatpak?), and for whatever reason it is publishing two schemas (how would a Flatpak-aware backend work? mystery); * a case where no relocatable schemas are needed to justify the use: two applications need to share some keys (see ‘/org/gnome/desktop/a11y/mouse/’ on an usual GNOME installation, where Mousetweaks code needs to be aware of desktop settings); * an application is applying a relocatable schema from a library it is using on its main path, so it’s transparent for users (if I put the BookmarksPopover in a library –the widget is not dconf-specific but for some details–, that could happen); * a bad application wants to confuse the user. (Never forget the bad applications.) > Well, if it can't be avoided having a list on the right side, I don't care > which is in on the left, really. I've switched to schema IDs on the left. Great. I think it will be easier that way. > I went with "*" because I never saw a path containing that, but from the > documentation on "g_settings_new_with_path" the only rule is "no consecutive > slashes" //, so I'll go with that now: empty segments match anything. Great. That will probably help also for some temporarily out of scope designs. > It is much smarter now. I think it covers every case where a path can be > inferred from the path specs. It will only fail matching when there is a > wild card and absolutely nothing to match against it. I'll think of some > test cases to add to Demo. I was failing to make it work here, but I was forgetting the suffixed ‘/’ in paths. Can you add it if it’s missing? for stupid people like me. xD
(In reply to Arnaud B. from comment #11) > (In reply to Davi from comment #9) > > (In reply to Arnaud B. from comment #8) > > Well, if it can't be avoided having a list on the right side, I don't care > > which is in on the left, really. I've switched to schema IDs on the left. > > Great. I think it will be easier that way. Testing and thinking: I find the list of paths on the right side quite convenient, but looks like you were right, it was not needed and in fact can cause problem. I have in mind an UI for editing that value, and for that the “array of paths” is the easier way to do things; but as there are manual editing possible, you can finish anyway with multiple time the same schema in the “dictionary”. So an “a{ss}” type with schema at the left would be a “list of rules”, multiple rules possibly applied for one schema. Sorry for the extra work. I’d prefer not to change the format of keys after release, so all has to be carefully designed regarding all the parameters, and I finish sometimes with a brain-overflow. ^^
Created attachment 364403 [details] [review] Preliminary patch - Changed type of "manual-schemas" to a{ss}, with schema ID to the left and a single path spec to the right, and allowing multiple entries for the same schema ID. - "Fix" path specs by prefixing/suffixing with "/" if missing
Great, at least there’s a little hope to kill the hydra at one point. ^^ I’ll have to read some more times the code to completely trust the parser, but it should be ok, amazing work. :) As you can imagine, my main problem with this patch is that it’s designed with the “static tree” SettingsModel in mind; I fear a hard time coming if we try to allow updates (when “manual-schemas” is changed, for example), and I’d really prefer to calculate each time a path is requested the relevant informations (and only them). I had in mind to put all the usual mappings of relocatable schemas in this key, but starting for testing to list them all, looks like that would probably be a little to much. So, I’d like to reserve this key for “manual” schemas mappings (!), and load the proposed usual schemas mappings from another source (directly in the code), with a flags key to enable/disable loading usual mappings and/or manual mappings. Your GSettingsKey GSettings work can be done in another patch without waiting for other things, if you want. Thanks for working on this!
(In reply to Davi from comment #13) sorry to butt in, but you should be aware of this: https://bugzilla.gnome.org/show_bug.cgi?id=789969
(In reply to info-cppsp from comment #15) > (In reply to Davi from comment #13) > sorry to butt in, but you should be aware of this: > https://bugzilla.gnome.org/show_bug.cgi?id=789969 Whatever the issue of bug 789969 is (and if desrt already discarded this option —and I’d agree with that choice— that won’t be pushed in), support of mapping relocatable schemas at “random” places is a wanted feature, and being able to display multiple schemas installed at the same place also.
(Note that the idea of helping applications to add their own mappings is a good one. Not in the exact same key probably, but as long as the mechanism is done, it’s easier to add that.)
Don't get me wrong, I didn't mean to discourage Davi or to judge this solution. In fact, exactly because my patch may not be accepted, this one at least gives something to the average user for the time being. If my patch should be accepted, you / me can still extend d-e, as needed.
Created attachment 364696 [details] Preliminary patch - Split the GSettingsKey/Directory modifications into different patches.
I’d really like to have this functionality in next release (that would be another important step), but I continue to think that this patchset will make the one of bug 731750 harder to finish. So, as both are done by you, do you want the current patchset to be pushed, or do I continue to wait until you have a complete project of a dynamic model including relocatable schemas? If the “dynamic model” patchset is not finished before next release, I’ll push this one, if there’s a way to define mappings proposed by the application somewhere and a way to disable them by a simple (flags) key.
Oh, looks like I pushed at one point by mistake… so, should I revert or not?
(In reply to Arnaud B. from comment #20) > I’d really like to have this functionality in next release (that would be > another important step), but I continue to think that this patchset will > make the one of bug 731750 harder to finish. I believe it is not that much harder. I have some sketches combining the two and it seems to work. I'll try to post it soon. > So, as both are done by you, do > you want the current patchset to be pushed, or do I continue to wait until > you have a complete project of a dynamic model including relocatable > schemas? (In reply to Arnaud B. from comment #21) > Oh, looks like I pushed at one point by mistake… so, should I revert or not? Since the dynamic model will take a bit longer, it might be better to keep this one in master earlier, so that flaws may be spotted sooner. (In reply to Arnaud B. from comment #20) > If the “dynamic model” patchset is not finished before next > release, I’ll push this one, if there’s a way to define mappings proposed by > the application somewhere and a way to disable them by a simple (flags) key. I'm thinking of making the following changes: - change "manual-schemas" to "user-schema-allocations" - create a relocatable schema ca.desrt.dconf-editor.ExternalSchemaAllocations with a single "schema-allocations" key, similar to "user-schema-allocations". This relocatable schema is supposed to be allocated to any immediate subpath of /ca/desrt/dconf-editor/external/ - Add an "as"-typed key "external-apps" (or some other better name) under ca.desrt.dconf-editor.Settings that lists the active subpaths of /ca/desrt/dconf-editor/external/. An external app could register a schema allocation setting "ca.desrt.dconf-editor.ExternalSchemaAllocations:/ca/desrt/dconf-editor/external/$APP_ID/ schema-allocations" and appending '$APP_ID' to "ca.desrt.dconf-editor.Settings external" The "disable switch" would be per-app or global (or both :)?
(In reply to Davi from comment #22) > (In reply to Arnaud B. from comment #21) > > Oh, looks like I pushed at one point by mistake… so, should I revert or not? > > Since the dynamic model will take a bit longer, it might be better to keep > this one in master earlier, so that flaws may be spotted sooner. Ok, let’s keep that then. > (In reply to Arnaud B. from comment #20) > - change "manual-schemas" to "user-schema-allocations" A name that starts with “reloc” would be great. So that all keys related to this functionality could be easily findable. > - create a relocatable schema > ca.desrt.dconf-editor.ExternalSchemaAllocations with a single > "schema-allocations" key, similar to "user-schema-allocations". This > relocatable schema is supposed to be allocated to any immediate subpath of > /ca/desrt/dconf-editor/external/ > - Add an "as"-typed key "external-apps" (or some other better name) under > ca.desrt.dconf-editor.Settings that lists the active subpaths of > /ca/desrt/dconf-editor/external/. > > An external app could register a schema allocation setting > "ca.desrt.dconf-editor.ExternalSchemaAllocations:/ca/desrt/dconf-editor/ > external/$APP_ID/ schema-allocations" and appending '$APP_ID' to > "ca.desrt.dconf-editor.Settings external" Sorry, I wasn’t clear. I’d like not to go that way, at least for now. Because: * some apps won’t accept to do weird things in their code just to please dconf-editor users; * some apps won’t know about this; * adding default mappings with this mechanism is hard. But, I’d like to “just” hardcode for now some default mappings in `dconf-editor` code, for usual applications. Because that’s what most users want. > The "disable switch" would be per-app or global (or both :)? What I have in mind is a (mainly) per-mechanism switch, done with a <flags> key with four entries: * manual, for mappings done by users inside the current “manual-schemas” key; * usual, for mappings hardcoded in dconf-editor; * list-of, because in theory there’s something to automatically map some child schemas at some parent paths (to be verified before doing anything here); * and internal, for future use inside `dconf-editor`. That way, users can disable the function, but for most of them all will just be kept with default “true” values, with sometimes a “manual-schemas” dict entry added.
Regression found in 766e4d6e0f530b0b21dec982c2af2e7eae2ea935 : * dconf-editor /ca/desrt/dconf-editor/Demo/; * “boolean” is at default value; * enter /ca/desrt/dconf-editor/Demo/boolean; * set to not-default value; * go back to /ca/desrt/dconf-editor/Demo/; * “boolean” is written in bold (ok); * go back to /ca/desrt/dconf-editor/Demo/boolean; * set to default value; * go back to /ca/desrt/dconf-editor/Demo/; * “boolean” is written in bold (it shouldn’t anymore).
(In reply to Arnaud B. from comment #24) > Regression found in 766e4d6e0f530b0b21dec982c2af2e7eae2ea935 : > * dconf-editor /ca/desrt/dconf-editor/Demo/; > * “boolean” is at default value; > * enter /ca/desrt/dconf-editor/Demo/boolean; > * set to not-default value; > * go back to /ca/desrt/dconf-editor/Demo/; > * “boolean” is written in bold (ok); > * go back to /ca/desrt/dconf-editor/Demo/boolean; > * set to default value; > * go back to /ca/desrt/dconf-editor/Demo/; > * “boolean” is written in bold (it shouldn’t anymore). The problem seems to be that GSettings objects do not emit changed signals when in delay-apply mode. The commit triggered the issue because a single GSettings object is shared by the GSettingsKey and the ModificationsRevealer. Before, they were different objects, so only the ModificationsRevealer entered delay-apply, so the GSettingsKey one emitted the signal and everything was fine. I suspect this is a GLib (or DConf) bug? Shouldn't the signals be emitted after g_settings_apply?
(In reply to Davi from comment #25) > I suspect this is a GLib (or DConf) bug? Shouldn't the signals be emitted > after g_settings_apply? Maybe it could feel normal not to emit signals from the GSettings that have requested the change? I won’t say it’s a bug necessarily. In any way, we could go back to the previous way to do things, right?
(In reply to Arnaud B. from comment #26) > Maybe it could feel normal not to emit signals from the GSettings that have > requested the change? I won’t say it’s a bug necessarily. That would be very strange. Usually for GObjects it *must* be the same object. I think GSettings should not contradict that. I've tested it a bit more, and noticed that it stops emitting changed signals only for resets (i.e. normal writes still emit them), and also only after delay() has been called at least once. I've opened https://bugzilla.gnome.org/show_bug.cgi?id=791290 where I give some more details on my investigation. > In any way, we > could go back to the previous way to do things, right? I'd prefer not to, if possible, mostly because in a future "dynamic model" scenario, it will become impossible to instantiate GSettings objects without the explicit GSettingsSchema object (i.e. new GLib.Settings.full). new GLib.Settings/new GLib.Settings.with_path won't work because GSettings uses the default GSettingsSchemaSource to load schemas, and that won't recognize schema IDs discovered from a fresh GSettingsSchemaSource, crashing the app. I'm attaching a different workaround that seems to fix the issue. Please, have a look.
Created attachment 365080 [details] [review] Fix regression - Workaround for https://bugzilla.gnome.org/show_bug.cgi?id=791290
Comment on attachment 365080 [details] [review] Fix regression Pushed, thanks. :)
Created attachment 365089 [details] Post-preliminary patch-set - Rename 'manual-schemas' to 'relocatable-schemas-user-paths' - Some refactoring - Add support for built-in relocatable schema paths - Add support for disabling mappings
(In reply to Arnaud B. from comment #23) > What I have in mind is a (mainly) per-mechanism switch, done with a <flags> > key with four entries: > * manual, for mappings done by users inside the current “manual-schemas” > key; > * usual, for mappings hardcoded in dconf-editor; I think the above two are contemplated, although more built-in mappings would need to be included. > * list-of, because in theory there’s something to automatically map some > child schemas at some parent paths (to be verified before doing anything > here); By 'list-of' do you mean, for example, "org.gnome.desktop.app-folders folder-children", that lists the children of "/org/gnome/desktop/app-folders/folders/"? > * and internal, for future use inside `dconf-editor`. I've added an entry in the flags for this, but I don't know why. I probably misunderstood.
(From my phone) (In reply to Davi from comment #31) > I think the above two are contemplated, although more built-in mappings > would need to be included. Looks good from here, but I’ll reread all at home again. > > * list-of, because in theory there’s something to automatically map some > > child schemas at some parent paths (to be verified before doing anything > > here); > > By 'list-of' do you mean, for example, "org.gnome.desktop.app-folders > folder-children", that lists the children of > "/org/gnome/desktop/app-folders/folders/"? I mean the more or less not documented “list-of” function, as in the DTD. I think it is designed for such an use. > > * and internal, for future use inside `dconf-editor`. > > I've added an entry in the flags for this, but I don't know why. I probably > misunderstood. The bookmarks button could and will be made independant, using its own relocatable schema, so that at one point it could move in a library. Schema that will be mapped on /ça/desrt/dconf-editor/, of course. ^^
Patches 1 to 4 have been pushed, thanks. :) For patch 5, don’t stress with “list-of”, it can be added later to the enum. For now, concentrate on having internal/usual/manual (or whatever their name are); the “internal” use will come during this cycle.
Created attachment 365167 [details] [review] Path to disable mappings, take 2 - Remove 'Listed' from the flags and every other mention of it.
Created attachment 365184 [details] [review] Map a relocatable schema to a path from CLI. Yay, it’s pushed, thanks. :) I imagine that this patch doesn’t help for the next steps (because of the sharing of the hashtable), but the functionality would be nice to have. Do I push now so you can base your work on it, or should I delay?
(In reply to Arnaud B. from comment #35) > I imagine that this patch doesn’t help for the next steps (because of the > sharing of the hashtable), I think it won't be a problem. > but the functionality would be nice to have. Agreed. > Do I > push now so you can base your work on it, or should I delay? I don't mind.
More or less all looks good here, no? :) We can work in other reports if some bugs are found. I opened bug 791781 for thinking of the “list-of” thing, and bug 791782 for supporting multiple schemas mapped at the same path. Thanks Davi for your hard work there (also). All will be available in 3.28, to be released in March.