Re: Category support in adsressbook

[Matt Davey <mcdavey mrao cam ac uk>]

Hi,
just a quick comment on a couple of points you raise:

On Wed, 2007-01-10 at 15:08 +0100, Tom Billiet wrote:
[....]
> Now for the rest I do have some remarks and questions.
> Although this patch works here (for me), I don't think the way of
> implementation is very good. There is actually too much code duplication
> between the todo/memos and calendar conduits regarding the category's. 
> Todo and memo both contain the function "add_category_if_possible" (I
> even thought it was not allowed to use exactly the same function name
> twice in plain C?) which the calendar conduit also needs.

The add_category_if_possible() functions are marked with the 'static'
keyword, so they are not visible outside their source file.

> The rest of the category syncing mainly is written in the 2 functions
> "comp_from_remote_record" and "local_record_from_comp" . The code that's
> written there for the category support can actually be the same for the
> three conduits I think.  The code there is actually (almost) exact the
> same for the todo and the memo's conduit. It differs a bit from the
> calendar conduit as I made the sync only adapt the first category in
> evolution if there is more than 1 assigned, thus keeping the evolution
> structure more intact.

Sounds good.  Presumably the other conduits should be modified to
conform to this behaviour too.

> What I want to say is I think it would be a better Idea to move the
> "add_category_if_possible" somewhere to the gnome-pilot code, and also
> add 2 more functions there: "remote_category_to_local" and
> "local_category_to_remote" which would contain the category code from
> the functions "comp_from_remote_record" and "local_record_from_comp"
> respectively. 

Sounds sensible.  Code duplication should be reduced wherever possible. 
The question is, should the common code be put into pilot-link,
gnome-pilot or evolution?

"add_category_if_possible" could live anywhere: it is modifying a
pilot-link structure, the CategoryAppInfo.  However, the code from
"remote_category_to_local" and "local_category_to_remote" requires
knowledge of evolution structures, AFAICS, and therefore can't be put
into pilot-link or gnome-pilot.

It's probably easier to move all the common code to the existing
"e-util/e-pilot-util.c"

Matt

Matt Davey          A woman walks into a Cocktail Bar and asks the Barman
mcdavey mrao cam ac uk     for a Double Entendre.  So he gives her one.