On Thu, May 07, 2009 at 05:38:10PM +0100, Dr J A Gow wrote:
> Folks,
>
> Attached is a patch that provides reasonable (but not yet fully tested)
> 2-way syncing of recurrent events in the opensync-0.4x plugin. I based
> this on the recurrence handling stuff I wrote for the SynCE project
> although it is not a direct port.
Thanks very much for this patch. I worked through it and have a few
comments.
> It also fixes up a problem with some addresses not syncing and a
> case-issue in Evo.
I noticed that the patch includes changes like this:
> // add all applicable phone numbers... there can be multiple
> // TEL fields, even with the same TYPE value... therefore, the
> // second TEL field with a TYPE=work, will be stored in WorkPhone2
> - AddPhoneCond("pref", con.Phone);
> - AddPhoneCond("fax", con.Fax);
> - AddPhoneCond("work", con.WorkPhone);
> - AddPhoneCond("work", con.WorkPhone2);
> - AddPhoneCond("home", con.HomePhone);
> - AddPhoneCond("home", con.HomePhone2);
> - AddPhoneCond("cell", con.MobilePhone);
> - AddPhoneCond("pager", con.Pager);
> + AddPhoneCond("PREF", con.Phone);
> + AddPhoneCond("FAX", con.Fax);
> + AddPhoneCond("WORK", con.WorkPhone);
> + AddPhoneCond("WORK", con.WorkPhone2);
> + AddPhoneCond("HOME", con.HomePhone);
> + AddPhoneCond("HOME", con.HomePhone2);
> + AddPhoneCond("CELL", con.MobilePhone);
> + AddPhoneCond("PAGER", con.Pager);
> AddPhoneCond(con.OtherPhone);
This was for a bug in SynCE, as I recall.
http://www.mail-archive.com/barry-devel@lists.sourceforge.net/msg01109.html
I don't recall that Evolution had this problem.
As you might expect, I have a general resistance to including workarounds
in my code for bugs in other software. :-) So far I have not included
this, although I might add it as a contrib/ patch for those needing to
work with SynCE. I'd much rather see this fixed in SynCE.
I would even accept a patch that added a --with-synce option to Barry's
configure script that automatically applied this patch. I don't want
to make it painless for synce users, since they should be fixing their
own code too. Note that any change to configure and friends needs to pass
the test/buildtest.sh script.
> +unsigned short vCalendar::GetMonthWeekNumFromBYDAY(const std::string& ByDay)
> +{
> + return atoi(ByDay.substr(0,ByDay.length()-2).c_str());
> +}
I'm confused by this... BYDAY can contain strings like this, according
to the RFC:
BYDAY=MO,TU,WE,TH,FR
Which doesn't work with an atoi() call.
What kind of data are you seeing in your tests?
> case Calendar::Day: // eg. every day
> - AddParam(attr, "FREQ", "DAILY");
> + //AddParam(attr, "FREQ", "DAILY");
> + AddValue(attr,"FREQ=DAILY");
> break;
Looks like I was hoping for better support in vformat.c... Thanks. :-)
I need to do more testing on this, but your change looks better than
my code on first glance.
> + // we do have COUNT. This means we won't have UNTIL. So
> we need to
> + // process the RecurringEndTime from the current start
> date. Set the count level to
> + // something other than zero to indicate we need to
> process it as the exact end date will
> + // depend upon the frequency.
> + count=atoi(args["COUNT"].c_str());
> + }
> + }
I added a check here so that an exception would be thrown if count == 0.
This seems to match the RFC requirements too, and makes your if(count)
tests below safer.
> + // we need these if COUNT is true, or if we are a yearly job.
> +
> + time_t time = cal.StartTime;
Code needs to be clearer... this introduces a prerequisite dependency on
the order of initialization of the cal class... if the code is moved
someday to fill in StartTime after this function is called, things will
break.
I changed it to use a function argument for starttime, to make this
requirement clearer.
> + if(args.find(string("FREQ"))!=args.end()) {
> + if(args["FREQ"]==string("DAILY")) {
> + cal.RecurringType=Calendar::Day;
> + } else {
> + if(args["FREQ"]==string("WEEKLY")) {
I changed this into a more linear if / else if / else if structure for
readability, and fixed a bracket bug on the YEARLY count at the bottom.
> + // convert to struct tm, then
> simply add to the year.
> + struct tm datestruct;
> + gmtime_r(&time,&datestruct);
> + datestruct.tm_year += count;
> +
> cal.RecurringEndTime=mktime(&datestruct);
There were a few places where this sequence was used: gmtime converts
time_t into a struct tm in UTC, while mktime converts a struct tm in
the local timezone into time_t. I changed gmtime() to localtime().
I still need to do more testing on this, but you put a lot of work into
this patch, and thank you very much. It has been applied with changes.
- Chris
------------------------------------------------------------------------------
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
production scanning environment may not be a perfect world - but thanks to
Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700
Series Scanner you'll get full speed at 300 dpi even with all image
processing features enabled. http://p.sf.net/sfu/kodak-com
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
