On Do, 2011-01-20 at 08:21 +0000, Patrick Ohly wrote:
> On Di, 2011-01-18 at 22:53 +0000, Frederik Elwert wrote:
> > I must confess that I don’t really understand what the type issue is
> > about, but renaming the GetConfig methods looks not exactly pretty. If
> > you split type into new properties, maybe you could use new names for
> > all new properties and drop the name "type" altogether. That probably
> > would make incompatible clients fail, but would leave the method names
> > intact. Don’t know if that makes sense, was just a thought.
>
> I agree that renaming the methods doesn't gain much and given the
> current use of the API in Genesis and sync-ui only causes problems.
>
> What I want to avoid is that some (unknown) client thinks it is doing
> the right thing by setting, for example, "evolutionsource" when
> syncevo-dbus-server in fact ignores it after the renaming to "database".
>
> I think I can achieve that by adding an additional check to SetConfig():
> if it contains obsolete or unknown properties, reject the whole
> operation. That's a worthwhile check anyway, and as far as I know
> currently missing.
I've added that check. org.syncevolution.InvalidCall will now be raised
for a) an unknown property and b) a wrong top-level entry in the config
hash (like "s/addressbook" instead of "source/addressbook"). Property
values are also checked in advance, before making any permanent changes
in the session or on disk. This changes the exception from
"org.syncevolution.Exception" to the same "InvalidCall".
While making these changes I noticed that our temporary config handling
was not quite according to the docs. It said that "GetConfig() and
SetConfig()" are not affected by temporary settings, while in fact
temporary settings *did* modify the result from GetConfig() - we even
had a test case for it. The implementation makes sense, so I fixed the
docs.
Furthermore I noticed that we didn't allow temporary changes to be reset
(temporary=True and update=False mutually exclusive). I don't remember
why we imposed that restriction. It is not terribly important, and with
the other changes it was trivial to implement the "temporary=True
update=False" case.
So here's the update to the docs reflecting these changes:
<arg type="b" name="temporary" direction="in">
<doc:doc><doc:summary>TRUE if configuration changes should only be
used for the duration of the session.
This is useful to run a single sync session with different
settings,
- for example with an increased logging level. "update=FALSE" and
- "temporary=TRUE" are mutually exclusive. Temporary config changes
- do not affect getting or setting permanent configurations.
+ for example with an increased logging level. "update=FALSE" removes
+ all previous temporary settings before adding the current ones.
+ Temporary config changes modify the result of GetConfig(), but
+ are ignored when making permanent changes in SetConfig().
</doc:summary></doc:doc>
</arg>
> The API change itself can be announced via a new entry in
> GetCapabilities().
Not done yet.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution
