pimutils / vdirsyncer

📇 Synchronize calendars and contacts.
https://vdirsyncer.pimutils.org/
Other
1.54k stars 160 forks source link

Enhancement: storage type or config to be used with Kontact's directory handling #881

Closed bernhardreiter closed 3 years ago

bernhardreiter commented 3 years ago

https://kontact.kde.org/ has a power calendar component called https://kontact.kde.org/components/korganizer.html

Modern versions can use CalDAV directly, but elder versions (kontact enterprise35) still in use could not, but offer saving of calendars in single files, much like the vdir storage format. It would be useful for us to use vdirsyncer to make this old kontact enterprise35 to work with CalDAV servers.

Because of the similiarity only small changes would be necessary. Two differences already found are

bernhardreiter commented 3 years ago

The old Kontact enterprise 3.5 (e35) code implementing Calendar in Local Directory seems to be in https://github.com/KDE/kdepim/blob/enterprise/e3/libkcal/resourcelocaldir.cpp

To understand how this is build, somebody probably has to read https://github.com/KDE/kdelibs/blob/enterprise/e3/kresources/README.design

bernhardreiter commented 3 years ago

Here the backup file with a trailing ~ is ignored: https://github.com/KDE/kdepim/blob/b6c2cb0a13e4fef69622e2fb1177c22836c2de1d/libkcal/resourcelocaldir.cpp#L152-L153

And here it is created https://github.com/KDE/kdepim/blob/b6c2cb0a13e4fef69622e2fb1177c22836c2de1d/libkcal/icalformat.cpp#L113

So a storage type = "filesystem" with fileext = "" should work in principle and it accepted in a simple test. (Though this violates two requirements of the vdir formats of a) having an extension and b) being otherwise ignored.).

We only need to ignore backup files just like "*.tmp" should be ignored by vdir implementations.

Implementation idea

Make setting of the backup file extension configurable, so the default .tmp could be changed to ~.

This is only a slight generalisation without drawbacks for other use cases. So it could be acceptalbe in the main line.

bernhardreiter commented 3 years ago

implemention considerations

It seems that https://github.com/pimutils/vdirsyncer/blob/master/vdirsyncer/storage/filesystem.py does not ignore files with ".tmp" like specified in the Reading from vdirs section of https://github.com/pimutils/vdirsyncer/blob/dcf5f701b7b5c21a8f4e8c80243db3e0baff1313/docs/vdir.rst

Thus implementing this would make the implementation match the specifcation better.

bernhardreiter commented 3 years ago

https://github.com/Intevation/vdirsyncer/tree/dev-issue881 has an implementation to allow setting the fileignoreext for the sotrage type filesystem.

Needs more testing now.

Regarding vdir compatibility: Using fileext='.ics' of course also ignores file with no or tmp extension. This is probably the reason why it is not in the code explicitly.

bernhardreiter commented 3 years ago

https://github.com/Intevation/vdirsyncer/tree/dev-issue881 now has more instructions now.

A first test reveals:

WhyNotHugo commented 3 years ago

Note that the vdir format is a directory with one-file-per-calendar-entry. I'm under the impression that Kontact uses a single file with all the entries, right?

Regarding vdir compatibility: Using fileext='.ics' of course also ignores file with no or tmp extension. This is probably the reason why it is not in the code explicitly.

Does ignoring *.tmp explicitly still make sense then? I believe that .ics~ should also be ignored fine, right?

bernhardreiter commented 3 years ago

The (old) kontact e35 in a legacy setting we have offers two types one is single file, which does not work, because of #818. The other calendar type is Calendar in Local Directory and uses one file per entry. So it is very close to the vdir format, it just does not use ".ics" as ending. So when using

fileext = ''
fileignorext = '~'

it works in principle with my changes now. :)

vdir compatibility of storage/filesystem

Originally I've thought that the current implementation in master would not be vdir compatible as outlined in https://github.com/pimutils/vdirsyncer/issues/881#issuecomment-833435941.

But in effect it is, once you chose either .ics or .vcs as this will also ignore files with no ending or ending in .tmp, so for general compatibility my assumption was wrong and there is no defect. (I've added a line to the description in my branch to make this more clear https://github.com/Intevation/vdirsyncer/commit/9a1582cc0faec365bab4d952c3b7f14480d16a1d#diff-f0a5822170a0cb40ccc8b537d36cac7e27a986d9911b3a08c03feab8f9a47a13R425.)

Thus yes, fileignoreext is only useful when setting fileext='' now. It could be extended to be a regexp match for more generalisation, but I wouldn't do this without usecase.