Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 15:04 -0400, Matthew Barnes wrote: > On Thu, 2011-04-28 at 19:53 +0200, Milan Crha wrote: > > Only the last thing for the enum values, I would personally prefer > > something with a prefix E_CLIENT_SOURCE_TYPE_... only to not have it > > confusion-able with E_CLIENT_TYPE constant. > > Do you mean E_TYPE_CLIENT (for getting EClientClass's GType)? Heh, right, I meant E_TYPE_CLIENT. I should read-before-write more often, I'm sorry. > Regardless, I'm fine with the longer prefix if you'd prefer that. Preferred on my side, yup. > > By the way, the proposed merge of server side parts, it may also involve > > merging client side bits (for E*View) and thus finally drop all the old > > cruft. It's a benefit, I hope, even with broken backward compatibility. > > (I would prefer to break backward compatibility personally, rather than > > inventing special names for structs not intersect with old names.) > > I haven't been following the E*View changes very closely. What's > considered cruft? There was just a little problem with ECalView, which had 'client' property, which is referring to ECal, instead of ECalClient, and I was forced to "invent" different name for it. But after a bit of sleeping and small thinking I might be wrong on this point too, as it should be easy to define EBookClientView/ECalClientView and keep old EBook/CalView-s as they are, at least their public API. I see I did really too quick thinking on these. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 21:16 +0200, Patrick Ohly wrote: > Agreed, the library dependency issue is a problem. That could be solved > by an utility library on top of libecal and libebook which offers the > unified API. Or you could just write your own function in EvolutionSync. It's just a switch statement on an enum value. > What about merging libebook and libecal into one shared library instead? > Evolution 3.2 will require an soname bump and source code changes in > apps anyway, throwing a renaming of libs into the mix won't make a big > difference. > > I think it would make the overall API a lot cleaner, albeit with > slightly (?) higher memory footprint for apps which only need one or the > other. I'll have to think on that. Seems kinda drastic, but maybe I'm just not seeing how it would make the overall API cleaner. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Do, 2011-04-28 at 13:07 -0400, Matthew Barnes wrote: > It hadn't occurred to me the word "calendar" might be ambiguous. I > blame the iCalendar spec for overloading it. In the real world, one > does not make a "calendar of tasks", one makes a "list of tasks". But tasks are shown on calendars because they have deadlines. It depends whether you see a calendar as a container of something or a tool to work with date-dependent items. > Maybe this is too Evolution-centric, but to me the container/item > relationship is clear: > >an ADDRESS_BOOK contains CONTACTS > a CALENDAR contains EVENTS > a TASK_LIST contains TASKS / TODOS > a MEMO_LIST contains MEMOS / JOURNALS > a MAIL_STOREcontains MESSAGES > > The enum values should be named consistently. So if we change CALENDAR > to EVENTS, I think we should change the rest. > > FWIW, the new ESource API is already using container names. Key files > will have groups named [Address Book], [Calendar], [Task List], etc. > instead of [Contacts], [Events], [Tasks], etc. > > To me it sounds more natural to refer to containers than items, but > maybe I'm too indoctrinated in Evolution-speak. Perhaps it is really to ingrained into Evolution already to change it. Never mind, I'll cope with it either way ;-) -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Do, 2011-04-28 at 16:34 +0200, Milan Crha wrote:
> [please reply to the list only, it makes my life worse when you CC me]
>
> On Thu, 2011-04-28 at 08:17 -0400, Matthew Barnes wrote:
> > It would mean EClient has to know that ECalClient and EBookClient exist
> > in order to instantiate the right one for a given enum value. Normally
> > in class design you want to avoid that kind of knowledge seeping into
> > lower layers, but with the class hierarchy being fixed to these three
> > classes (four if we add email), I think it's a good tradeoff to have a
> > more consistent API.
> >
> > So it would be something like:
> >
> > typedef enum {
> > E_CLIENT_TYPE_ADDRESS_BOOK,
> > E_CLIENT_TYPE_CALENDAR,
> > E_CLIENT_TYPE_MEMO_LIST,
> > E_CLIENT_TYPE_TASK_LIST
> > } EClientType;
>
> You obviously face of a circular dependency, so it's not doable in a
> clean way. Also because EClient is in libedataserver, EBookClient in
> addressbook/libebook and similarly ECalClient in calendar/libecal. Both
> descendants link to libedataserver... Having another
> register_client/unregister_client function will make things only less
> clear for readers (like if one tries to follow signal handlers by
> reading the code.
Agreed, the library dependency issue is a problem. That could be solved
by an utility library on top of libecal and libebook which offers the
unified API. In that case we wouldn't get rid of the special-purpose
calls in libecal and libebook, though, would we?
What about merging libebook and libecal into one shared library instead?
Evolution 3.2 will require an soname bump and source code changes in
apps anyway, throwing a renaming of libs into the mix won't make a big
difference.
I think it would make the overall API a lot cleaner, albeit with
slightly (?) higher memory footprint for apps which only need one or the
other.
> > > Talking of capabilities, I found their usefulness a bit limited because
> > > they a) cannot change during the life time of a client and b) they only
> > > report "exists/doesn't exit".
>
> This is partly fixed, as I faced of change inability of capabilities
> too, so the EClient itself is caching capabilities until online state
> changes.
And how does it know that capability changes cannot occur at some other
point in time? That sounds like it might work for the current set of
capabilities, but not like a general solution to the problem.
> When it changes the client side capabilities cache is purged
> and the new set of capabilities is queried on the next access of them.
> I do not see any problem in an "exists/doesn't exist" (or better
> "capable/incapable") state for this particular thing. They are
> capabilities, after all.
For a capability like "how many events per calendar do you support" (can
be queried for CalDAV, if I remember correctly) a single bit is not
enough.
> For CalDAV's CTag similarity, it might worth new API, than moving it
> exposed on the capability side (I understood you are proposing something
> like that),
A dedicated API just for CTag would have the problem that I mentioned
before: can only be used if the app is compiled against an EDS version
which has that call.
> but it's usually not supported by all backends, so even
> you'll have a possibility, then I believe you'll end with a default
> behaviour of returning "something changed" and you'll check items in an
> old way, thus no benefit for it.
Even if it only works with a few backends it would be an improvement for
users of those backends and worthwhile in those cases, in particular if
the file backend supports it.
--
Bye, Patrick Ohly
--
patrick.o...@gmx.de
http://www.estamos.de/
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 19:53 +0200, Milan Crha wrote: > Only the last thing for the enum values, I would personally prefer > something with a prefix E_CLIENT_SOURCE_TYPE_... only to not have it > confusion-able with E_CLIENT_TYPE constant. Do you mean E_TYPE_CLIENT (for getting EClientClass's GType)? Regardless, I'm fine with the longer prefix if you'd prefer that. > By the way, the proposed merge of server side parts, it may also involve > merging client side bits (for E*View) and thus finally drop all the old > cruft. It's a benefit, I hope, even with broken backward compatibility. > (I would prefer to break backward compatibility personally, rather than > inventing special names for structs not intersect with old names.) I haven't been following the E*View changes very closely. What's considered cruft? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 12:35 -0400, Matthew Barnes wrote: > This may also be of benefit to Srini and his email-factory branch. I > can imagine extending the enum to include E_CLIENT_TYPE_MAIL_STORE or > something similar. Only the last thing for the enum values, I would personally prefer something with a prefix E_CLIENT_SOURCE_TYPE_... only to not have it confusion-able with E_CLIENT_TYPE constant. After all, the generic open/... functions can be added to libedataserverui, to some e-client-utils.h/c, possibly together with renaming actual e-client-authentication.h/c. Maybe? By the way, the proposed merge of server side parts, it may also involve merging client side bits (for E*View) and thus finally drop all the old cruft. It's a benefit, I hope, even with broken backward compatibility. (I would prefer to break backward compatibility personally, rather than inventing special names for structs not intersect with old names.) Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 14:49 +0200, Patrick Ohly wrote: > Please, let's avoid "E_CLIENT_TYPE_CALENDAR". For people less familiar > with Evolution terminology it is not clear whether this includes tasks. > In Evolution, it doesn't, but in other contexts it does. For example, > Nokia phones store events and tasks in the same "Calendar". In > SyncEvolution I followed the Evolution terminology and "calendar" has > caused a lot of confusion. > > It's not even obvious inside Evolution, because libe*cal*[endar] also > does include tasks and memos. > > What about E_CLIENT_TYPE_EVENTS instead? It hadn't occurred to me the word "calendar" might be ambiguous. I blame the iCalendar spec for overloading it. In the real world, one does not make a "calendar of tasks", one makes a "list of tasks". Maybe this is too Evolution-centric, but to me the container/item relationship is clear: an ADDRESS_BOOK contains CONTACTS a CALENDAR contains EVENTS a TASK_LIST contains TASKS / TODOS a MEMO_LIST contains MEMOS / JOURNALS a MAIL_STOREcontains MESSAGES The enum values should be named consistently. So if we change CALENDAR to EVENTS, I think we should change the rest. FWIW, the new ESource API is already using container names. Key files will have groups named [Address Book], [Calendar], [Task List], etc. instead of [Contacts], [Events], [Tasks], etc. To me it sounds more natural to refer to containers than items, but maybe I'm too indoctrinated in Evolution-speak. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 18:11 +0200, Milan Crha wrote: > Yes, when I was changing server side data-views for book and cal, then > if turned out that the D-Bus API is exactly the same except of the > DBUS_PROXY_NAME and book/cal strings in a file. > > Thus having type param for both factories too, though for book unused, > may clean the code very nicely, forcing us to exactly one implementation > of base EGDBusFactory, EGDBusView, EDataFactory, EDataView and subclass > these to EGDBusBookFactory, EGDBusCalFactory, ... and changing only > really minimum on them. Basically the effort which started Travis > Reitter in his eds branches. Excellent. I'm sold then. Sounds like the right thing to do. This may also be of benefit to Srini and his email-factory branch. I can imagine extending the enum to include E_CLIENT_TYPE_MAIL_STORE or something similar. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:59 -0400, Matthew Barnes wrote: > On Thu, 2011-04-28 at 16:34 +0200, Milan Crha wrote: > > You obviously face of a circular dependency, so it's not doable in a > > clean way. Also because EClient is in libedataserver, EBookClient in > > addressbook/libebook and similarly ECalClient in calendar/libecal. Both > > descendants link to libedataserver... Having another > > register_client/unregister_client function will make things only less > > clear for readers (like if one tries to follow signal handlers by > > reading the code. > > Way to douse me with a bucket of cold water there. :) > > You're right about the library dependencies. That does kinda put a > bullet in unifying the "new" function. I agree a type registration > system is overkill for a mere two GTypes. > > Still, is there any value in having such an enum defined in e-client.h? > > I cited one benefit in consolidating my "get_default_source" functions, > but that alone is not really compelling. Are there other use cases? Yes, when I was changing server side data-views for book and cal, then if turned out that the D-Bus API is exactly the same except of the DBUS_PROXY_NAME and book/cal strings in a file. Thus having type param for both factories too, though for book unused, may clean the code very nicely, forcing us to exactly one implementation of base EGDBusFactory, EGDBusView, EDataFactory, EDataView and subclass these to EGDBusBookFactory, EGDBusCalFactory, ... and changing only really minimum on them. Basically the effort which started Travis Reitter in his eds branches. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 16:34 +0200, Milan Crha wrote: > You obviously face of a circular dependency, so it's not doable in a > clean way. Also because EClient is in libedataserver, EBookClient in > addressbook/libebook and similarly ECalClient in calendar/libecal. Both > descendants link to libedataserver... Having another > register_client/unregister_client function will make things only less > clear for readers (like if one tries to follow signal handlers by > reading the code. Way to douse me with a bucket of cold water there. :) You're right about the library dependencies. That does kinda put a bullet in unifying the "new" function. I agree a type registration system is overkill for a mere two GTypes. Still, is there any value in having such an enum defined in e-client.h? I cited one benefit in consolidating my "get_default_source" functions, but that alone is not really compelling. Are there other use cases? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 16:37 +0200, Milan Crha wrote: > On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > > * "32 bit IDs" - check whether (read) or ensure that (write) IDs > > are 32 bit integers instead of strings, simplifies > > QtContacts-EDS [2]; I have a patch which reduces the size of > > IDs > > from 64 bit to 32 in the file backend, more on that in a > > separate email > > Hi, > I'm against this. It's too limiting. Or maybe elaborate more, what you > expect behind this and what exactly you want to change (really values > for UID properties of vCard and VEVENT/...?). > Bye, > Milan Never mind, I didn't notice your more detailed mail when writing this. I'm sorry. Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > * "32 bit IDs" - check whether (read) or ensure that (write) IDs > are 32 bit integers instead of strings, simplifies > QtContacts-EDS [2]; I have a patch which reduces the size of > IDs > from 64 bit to 32 in the file backend, more on that in a > separate email Hi, I'm against this. It's too limiting. Or maybe elaborate more, what you expect behind this and what exactly you want to change (really values for UID properties of vCard and VEVENT/...?). Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
[please reply to the list only, it makes my life worse when you CC me]
On Thu, 2011-04-28 at 08:17 -0400, Matthew Barnes wrote:
> It would mean EClient has to know that ECalClient and EBookClient exist
> in order to instantiate the right one for a given enum value. Normally
> in class design you want to avoid that kind of knowledge seeping into
> lower layers, but with the class hierarchy being fixed to these three
> classes (four if we add email), I think it's a good tradeoff to have a
> more consistent API.
>
> So it would be something like:
>
> typedef enum {
> E_CLIENT_TYPE_ADDRESS_BOOK,
> E_CLIENT_TYPE_CALENDAR,
> E_CLIENT_TYPE_MEMO_LIST,
> E_CLIENT_TYPE_TASK_LIST
> } EClientType;
You obviously face of a circular dependency, so it's not doable in a
clean way. Also because EClient is in libedataserver, EBookClient in
addressbook/libebook and similarly ECalClient in calendar/libecal. Both
descendants link to libedataserver... Having another
register_client/unregister_client function will make things only less
clear for readers (like if one tries to follow signal handlers by
reading the code.
> > Talking of capabilities, I found their usefulness a bit limited because
> > they a) cannot change during the life time of a client and b) they only
> > report "exists/doesn't exit".
This is partly fixed, as I faced of change inability of capabilities
too, so the EClient itself is caching capabilities until online state
changes. When it changes the client side capabilities cache is purged
and the new set of capabilities is queried on the next access of them.
I do not see any problem in an "exists/doesn't exist" (or better
"capable/incapable") state for this particular thing. They are
capabilities, after all.
For CalDAV's CTag similarity, it might worth new API, than moving it
exposed on the capability side (I understood you are proposing something
like that), but it's usually not supported by all backends, so even
you'll have a possibility, then I believe you'll end with a default
behaviour of returning "something changed" and you'll check items in an
old way, thus no benefit for it.
Bye,
Milan
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] 32 bit IDs in contact file backend
Hello!
As part of wrapping QtContacts around EDS [1] I ran into the same issue
that Nokia already encountered in their Maemo 5 [2] backend: EDS uses
strings as ID, QtContacts 32 bit integers.
Nokia solved that by setting up an in-memory hash which maps the UID
string to its CRC-16. I don't see any checks for the (possible) hash
collisions. IMHO a proper solution has to keep a permanent mapping on
disk, otherwise the 32 bit IDs won't be stable.
Overall not a nice solution. My attempt to make it nicer at least in
combination with the file backend (the main goal for QtContacts-EDS)
focused on simplifying the problem instead: with 32 bit IDs in the
storage, the mapping becomes easy.
Attached the resulting patch. Note that with the patch applied, all new
contacts in a Berkley DB get the simpler IDs, unconditionally. Older
contacts continue to use their existing IDs. Would something like this
be acceptable upstream?
Further work if you agree in principle:
* let clients query whether all contacts have the simplified ID -
could be done with the dynamic capabilities that I mentioned in
the e-client API thread; avoids reading all contact (UIDs)
* optional: migrate database to simplified IDs on request
I'm not sure whether the second point really needs to be in EDS.
Obviously it should only be done when the impact on the rest of the
system is understood. For example, in a database that is synchronized
with SyncEvolution, such a rewriting of IDs will result in removing and
re-adding all contacts in the synchronized peers because that is what
it'll look like to other EDS clients.
This is also how the migration can be implemented without changes in
EDS: just remove and re-add all contacts.
[1] https://meego.gitorious.org/meego-middleware/qtcontacts-eds
[2]
https://www.qt.gitorious.org/qt-mobility/contacts/trees/master/plugins/contacts/maemo5
--
Bye, Patrick Ohly
--
patrick.o...@gmx.de
http://www.estamos.de/
>From 65438bd3c4b706535f83304d94b3c018c8899a5d Mon Sep 17 00:00:00 2001
From: Patrick Ohly
Date: Fri, 18 Mar 2011 16:07:23 +0100
Subject: [PATCH] addressbook file backend: use 32 bit integers as IDs
The motiviation for this change is primarily that it'll simplify
wrapping EDS in QtContacts, which only supports 32 bit integers for
the storages that it wraps.
Previously the ID was generated as a combination of time and running
counter, without checking for conflicts. This patch changes this such
that the ID is only a running counter. ID conflicts are detected during
an attempt to save, leading to retries with the next counter value.
The "for future use" members of _EBookBackendFilePrivate are removed
because they serve no useful purpose in a structure which is allocated
and used only locally.
---
addressbook/backends/file/e-book-backend-file.c | 48 +++---
1 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/addressbook/backends/file/e-book-backend-file.c b/addressbook/backends/file/e-book-backend-file.c
index dc6a703..d21c527 100644
--- a/addressbook/backends/file/e-book-backend-file.c
+++ b/addressbook/backends/file/e-book-backend-file.c
@@ -77,11 +77,7 @@ struct _EBookBackendFilePrivate {
DB *file_db;
DB_ENV *env;
EBookBackendSummary *summary;
- /* for future use */
- gpointer reserved1;
- gpointer reserved2;
- gpointer reserved3;
- gpointer reserved4;
+ guint id;
};
G_LOCK_DEFINE_STATIC (global_env);
@@ -169,13 +165,13 @@ build_summary (EBookBackendFilePrivate *bfpriv)
}
static gchar *
-e_book_backend_file_create_unique_id (void)
+e_book_backend_file_create_next_id (EBookBackendFile *bf)
{
- /* use a 32 counter and the 32 bit timestamp to make an id.
- it's doubtful 2^32 id's will be created in a second, so we
- should be okay. */
- static guint c = 0;
- return g_strdup_printf (PAS_ID_PREFIX "%08lX%08X", time(NULL), c++);
+ /* use a 32 counter, avoid 0; collision checks must be done by caller */
+ ++bf->priv->id;
+ if (!bf->priv->id)
+ bf->priv->id = 1;
+ return g_strdup_printf (PAS_ID_PREFIX "%04X", bf->priv->id);
}
static void
@@ -210,7 +206,8 @@ do_create(EBookBackendFile *bf,
g_assert (vcard_req);
g_assert (contact);
- id = e_book_backend_file_create_unique_id ();
+ retry:
+ id = e_book_backend_file_create_next_id (bf);
string_to_dbt (id, &id_dbt);
@@ -224,9 +221,10 @@ do_create(EBookBackendFile *bf,
string_to_dbt (vcard, &vcard_dbt);
- db_error = db->put (db, NULL, &id_dbt, &vcard_dbt, 0);
+ db_error = db->put (db, NULL, &id_dbt, &vcard_dbt, DB_NOOVERWRITE);
g_free (vcard);
+ g_free (id);
if (0 == db_error) {
db_error = db->sync (db, 0);
@@ -234,12 +232,16 @@ do_create(EBookBackendFile *bf,
g_warning ("db->sync failed with %s", db_strerror (db_error));
}
} else {
- g_warning (G_STRLOC ": db->put failed with %s", db_strerror (db_error));
g_object_unref (*contact);
*contact = NULL;
+ if (DB_KEYEXIST == db_error) {
+ /* did not guess a new ID, try a
Re: [Evolution-hackers] New 'eclient' branch in eds
On Do, 2011-04-28 at 08:17 -0400, Matthew Barnes wrote:
> On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote:
> > First of all, +1 for rethinking the API. I'd like to suggest that
> > besides modernizing the API we also take this opportunity to move more
> > of EBook/ECal into a common core.
> >
> > Opening and listing databases don't have to be separate. Turn
> > ECalClientSourceType into EClientSourceType and all of the _new,
> > _new_from_uri, _new_system, _new_default, _get_sources functions can be
> > moved into e-client.h.
> >
> > The advantage would be that clients (like SyncEvolution) which work with
> > both only need to implement the common operations once. Right now I have
> > a lot of duplicate code in the address book and calendar backends.
>
> That's not a bad idea, actually.
>
> It would mean EClient has to know that ECalClient and EBookClient exist
> in order to instantiate the right one for a given enum value. Normally
> in class design you want to avoid that kind of knowledge seeping into
> lower layers, but with the class hierarchy being fixed to these three
> classes (four if we add email), I think it's a good tradeoff to have a
> more consistent API.
I agree, it breaks layering, but overall I'd prefer such a unified API.
> So it would be something like:
>
> typedef enum {
> E_CLIENT_TYPE_ADDRESS_BOOK,
> E_CLIENT_TYPE_CALENDAR,
> E_CLIENT_TYPE_MEMO_LIST,
> E_CLIENT_TYPE_TASK_LIST
> } EClientType;
>
> I'd prefer *our* terminology in the enum over iCalendar terminology. I
> always have to stop and think "okay a memo list is called a VJOURNAL"
> and so on.
Please, let's avoid "E_CLIENT_TYPE_CALENDAR". For people less familiar
with Evolution terminology it is not clear whether this includes tasks.
In Evolution, it doesn't, but in other contexts it does. For example,
Nokia phones store events and tasks in the same "Calendar". In
SyncEvolution I followed the Evolution terminology and "calendar" has
caused a lot of confusion.
It's not even obvious inside Evolution, because libe*cal*[endar] also
does include tasks and memos.
What about E_CLIENT_TYPE_EVENTS instead?
> > BTW, in this particular example, wouldn't
> > e_source_registry_get_default_calendar() need such an enum anyway to
> > distinguish between default events, tasks and memos?
>
> I also wrote:
>
>e_source_registry_get_default_task_list()
>e_source_registry_get_default_memo_list()
>
> http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/ESourceRegistry.html
>
> So there's four "get_default" functions all together. Hence why having
> a single enum definition to cover them all would be nice.
Here we have the first misunderstanding because of "calendar" - it
wasn't obvious to me that this was exclusively for events ;-}
--
Bye, Patrick Ohly
--
patrick.o...@gmx.de
http://www.estamos.de/
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 13:12 +0100, David Woodhouse wrote: > As I understand it, an soname bump (from libfoo.so.5 to libfoo.so.6) > should *only* be used for backwards-incompatible changes. Correct. One such example of a backwards-incompatible change is increasing the size of a public GObject class structure for which there exist or may exist subclasses. Hence the practice of reserving X number of pointers at the end of the class struct so X new methods can be added over time without altering the struct size. > If old clients can continue to use the newer library, then shouldn't it > be just a change of *minor* version from libfoo.so.5.2 to libfoo.so.5.3, > and the soname remains the same (libfoo.so.5). Yeah, we don't even do that much right. We usually just leave the soname alone from release to release unless we break compatibility. Blame me for not really groking libtool versioning practices, which just seems unnecessarily complex and confusing to my poor little mind. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote:
> First of all, +1 for rethinking the API. I'd like to suggest that
> besides modernizing the API we also take this opportunity to move more
> of EBook/ECal into a common core.
>
> Opening and listing databases don't have to be separate. Turn
> ECalClientSourceType into EClientSourceType and all of the _new,
> _new_from_uri, _new_system, _new_default, _get_sources functions can be
> moved into e-client.h.
>
> The advantage would be that clients (like SyncEvolution) which work with
> both only need to implement the common operations once. Right now I have
> a lot of duplicate code in the address book and calendar backends.
That's not a bad idea, actually.
It would mean EClient has to know that ECalClient and EBookClient exist
in order to instantiate the right one for a given enum value. Normally
in class design you want to avoid that kind of knowledge seeping into
lower layers, but with the class hierarchy being fixed to these three
classes (four if we add email), I think it's a good tradeoff to have a
more consistent API.
So it would be something like:
typedef enum {
E_CLIENT_TYPE_ADDRESS_BOOK,
E_CLIENT_TYPE_CALENDAR,
E_CLIENT_TYPE_MEMO_LIST,
E_CLIENT_TYPE_TASK_LIST
} EClientType;
I'd prefer *our* terminology in the enum over iCalendar terminology. I
always have to stop and think "okay a memo list is called a VJOURNAL"
and so on.
> Matthew suggested to replace some of these with
> e_source_registry_get_default_calendar/address_book:
>
> e_cal_client_new_default()
>
> Instead do:
>
> source = e_source_registry_get_default_calendar (registry);
> client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT,
> error);
>
> The same logic would apply here: instead of having separate calls for
> calendar and address book, have one call with an enum.
Yeah, having one "get_default" function would be desirable.
The functions would have to be moved to e-client.h in order to use the
enum, but now that I think about it, it kinda makes sense to move them
to e-client.h regardless. The default values only come into play when
you have to create a client connection to *something*.
> BTW, in this particular example, wouldn't
> e_source_registry_get_default_calendar() need such an enum anyway to
> distinguish between default events, tasks and memos?
I also wrote:
e_source_registry_get_default_task_list()
e_source_registry_get_default_memo_list()
http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/ESourceRegistry.html
So there's four "get_default" functions all together. Hence why having
a single enum definition to cover them all would be nice.
> Talking of capabilities, I found their usefulness a bit limited because
> they a) cannot change during the life time of a client and b) they only
> report "exists/doesn't exit".
I'll leave it to Milan to address the capabilities stuff. He knows more
about it than I.
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > Are you saying that a soname version bump can and should be done in a > backwards-incompatible way (= code compiled against older version no > longer works with new lib)? That's a major pain for people trying to > support multiple distros. Please avoid it whenever possible. As I understand it, an soname bump (from libfoo.so.5 to libfoo.so.6) should *only* be used for backwards-incompatible changes. If old clients can continue to use the newer library, then shouldn't it be just a change of *minor* version from libfoo.so.5.2 to libfoo.so.5.3, and the soname remains the same (libfoo.so.5). -- dwmw2 ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Di, 2011-04-19 at 10:57 -0400, Matthew Barnes wrote: > On Tue, 2011-04-19 at 16:03 +0200, Milan Crha wrote: > > As icaltimezone is the main timezone for calendar in > > evolution-data-server, and will be as long as libical will be used > > there, then what about having e_cal_client_set_default_gtimezone as a > > possible alternative for e_cal_client_set_default_timezone? On the other > > hand, wouldn't e-cal-time-util.h/.c fit better here? > > The fact that we use libical is an implementation detail and the fact > its data types are exposed in the public API is a shame, but we can't do > anything about that right now. To some extend I agree, but I would like to point out that libical, for better or worse, also is the common core in Linux for calendar. This has saved my arse while passing events from libecal to KCal in the (still on-going) "put C++ MeeGo APIs on top of EDS" work [1]. I rely on this implementation detail, please don't take it away. [1] https://meego.gitorious.org/meego-middleware/kcal-eds -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Di, 2011-04-19 at 16:03 +0200, Milan Crha wrote:
> On Tue, 2011-04-19 at 09:27 -0400, Matthew Barnes wrote:
> > There's a couple other simple things I forgot to mention yesterday.
> >
> > * We should add some padding to all the public class structs so we can
> > add new class methods in the future without breaking ABI. Something
> > like this:
> >
> > struct _ECalClientClass {
> >
> > ... methods and stuff ...
> >
> > gpointer reserved[16];
> > };
>
> I never understood a need for this, neither used it when changing
> structs. There was done a soname version bump when changing public
> "class" structures, which, from my point of view, always involves also
> API changes, and freezes on these both are tight together.
Are you saying that a soname version bump can and should be done in a
backwards-incompatible way (= code compiled against older version no
longer works with new lib)? That's a major pain for people trying to
support multiple distros. Please avoid it whenever possible.
--
Bye, Patrick Ohly
--
patrick.o...@gmx.de
http://www.estamos.de/
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mo, 2011-04-18 at 15:57 +0200, Milan Crha wrote: > I just created a new branch 'eclient' in eds [1] where is added my work > on the new API which will deprecate EBook/ECal. It is following glib/gio > async pattern and, I believe, makes things more coherent. First of all, +1 for rethinking the API. I'd like to suggest that besides modernizing the API we also take this opportunity to move more of EBook/ECal into a common core. Opening and listing databases don't have to be separate. Turn ECalClientSourceType into EClientSourceType and all of the _new, _new_from_uri, _new_system, _new_default, _get_sources functions can be moved into e-client.h. The advantage would be that clients (like SyncEvolution) which work with both only need to implement the common operations once. Right now I have a lot of duplicate code in the address book and calendar backends. Matthew suggested to replace some of these with e_source_registry_get_default_calendar/address_book: e_cal_client_new_default() Instead do: source = e_source_registry_get_default_calendar (registry); client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, error); The same logic would apply here: instead of having separate calls for calendar and address book, have one call with an enum. BTW, in this particular example, wouldn't e_source_registry_get_default_calendar() need such an enum anyway to distinguish between default events, tasks and memos? Matthew already made a similar comment about error codes and "get_capabilities". I agree that these should be common, too. I don't see why get_capabilities needs to be in ecal resp. ebook, except for the convenience wrapper functions which query specific capabilities that only apply to one or the other. Talking of capabilities, I found their usefulness a bit limited because they a) cannot change during the life time of a client and b) they only report "exists/doesn't exit". Their static nature IMHO only has one benefit, which is that they can be safely cached on the client side. I don't see that as very useful, because for those capabilities which are known to be static, the client is likely to only query them once at startup and then set some internal state accordingly. Even for that the API must be asynchronous because of the underlying D-Bus call, as Matthew said. What I'd prefer is a generic key/value mechanism, with value being a string. I prefer a string because it is easy to handle and more complex types (if ever needed) can be mapped to it on a case by case basis. Setting values should also be allowed. That gives us a way how a client can configure the storage to behave in certain ways. This can be per-database (tweak the actual on-disk storage) or per EClient (modify behavior as part of current session). Future use cases for such a key/value mechanism: * "change tag", read-only - a string which changes each time the database changes (same as the CTag in CalDAV), would be used to make change detection in SyncEvolution more efficient [1] * "32 bit IDs" - check whether (read) or ensure that (write) IDs are 32 bit integers instead of strings, simplifies QtContacts-EDS [2]; I have a patch which reduces the size of IDs from 64 bit to 32 in the file backend, more on that in a separate email There can also be convenience functions for this, if they are considered important enough. Only adding such functions and not the generic API would have the downside that code cannot be compiled against an old API implementation and still use the new features if they happen to be available at runtime. At the D-Bus level this could be mapped to properties. [1] http://wiki.meego.com/Architecture/planning/evolution-data-server/eds-improvements#quick_check_for_.22data_changed.22 [2] http://lists.meego.com/pipermail/meego-dev/2011-April/482731.html -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
