search

dengste / org-caldav

Caldav sync for Emacs orgmode
GNU General Public License v3.0
718 stars 105 forks source link

Add support for syncing TODOs #218

Closed whirm closed 1 year ago

whirm commented 4 years ago

This PR brings in some commits from @grauschnabel's fork I've adapted to your master plus some fixes on top of it.

Edit: Original PR from the author of most of the work here: #118

lhindir commented 4 years ago

This is sweet! Just here to mention #118 for future lurkers.

Would love to see this merged @dengste

whirm commented 4 years ago

This is sweet! Just here to mention #118 for future lurkers.

Would love to see this merged @dengste

I've added the reference on the PR comment, thanks for noting it!

whirm commented 3 years ago

@dengste are you planning on reviewing this? Thanks!

ethancedwards8 commented 3 years ago

Ah man, this would be great!

olekenneth commented 3 years ago

I really would like to see this merged.

grauschnabel commented 3 years ago

What happend to @dengste? That's a bit confusing that he is not even answering here.. .

florhizome commented 3 years ago

thanks for all your effort :) this is a great and important feature since nextcloud is spreading :) next up would be async support ... ;P

sorry for the bad joke, but is there a selfoperable branch at one of yours one could already use until all is merged ? when I try to use "sync-todos" of @whirm all the usual variables are not available anymore.

f.e. "Symbol's function definition is void: org-caldav-calendars"

It would be nice since I think @dengste shouldn't need to rush the commit but still could really use the features right now ^^^

whirm commented 3 years ago

sorry for the bad joke, but is there a selfoperable branch at one of yours one could already use until all is merged ? when I try to use "sync-todos" of @whirm all the usual variables are not available anymore.

This branch is in a working condition. (that's the version I use myself)

f.e. "Symbol's function definition is void: org-caldav-calendars"

This defcustom is at org-caldav.el:120 so my guess is that you haven't (yet) loaded org-caldav at the time you are trying to access it.

florhizome commented 3 years ago

hmm ok yeah it's maybe something I do wrong when trying to declare the fork with straight.el...

florhizome commented 3 years ago

I wanted to bring up the proposal of designing a plugin/extension package for this. @dengste still doesn't have to found the time to handle merge requests for now. Again, not here to pressure ... But this functionality is pretty important for those who want to use it, and since nextcloud is only going to expand further IMO, deserves a space for itself to be fostered. It could further be discussed if org-caldav could be designed more modular in the future, since it's a complex package and matter which I think is hard to maintain as a sole person, etc. On another thought the whole matter - Proper taskmanagement with calendar integration on the base of caldav - seems to be complicated and important enough (There doesn't seem to be much (FOSS) Software around being able to help with this) to say it could be worth to dedicate a separate base (a package defining a major mode) to this. For example, my Orgmode has been buggy for some time, and I don't want to rely on it to be fully functional to have my task management working. Of course, this is quite the separate issue on a different timeline, also needing dedicated ppl, but I think it's worthy to have written down somewhere for ppl to consider.

hny-gd commented 3 years ago

Maybe it still might be an idea to look at @girzel 's suggestion to maybe move towards core url-dav.el?

girzel commented 3 years ago

Just to be clear, that was a pretty hand-wavy suggestion! And something that, while it could potentially simplify this package, wouldn't remove the need for it altogether. But it could theoretically get more maintainer eyes on the basic caldav functionality.

florhizome commented 3 years ago

where was that mentioned? Yeah, sure, org caldav will remain, maybe it could be possible to reappoint some core functionality in the future, though.

hny-gd commented 3 years ago

It was part of the discussion in #233.

FWIW, this came through on the orgmode mailing list just now: https://github.com/theophilusx/icsorg

florhizome commented 3 years ago

i thought org-icalendar was already existent and working?

dschwilk commented 3 years ago

Just chiming in as someone who depends on org-caldav / nextcloud and cares about this. Not a great elisp programmer but am keen to see this survive/grow and can help.

jackkamm commented 2 years ago

I'm also interested in VTODO support, but had some problems with this branch that I found difficult to debug. Debugging was made harder by the fact that this branch has accumulated many changes over the years, some of which are unneeded for core VTODO support.

So, I created a smaller, barebones version of this work here: https://github.com/jackkamm/org-caldav/tree/sync-todos-clean

I think the barebones version may be easier to review and eventually merge. For example, it has a diff about 5x smaller than the current PR:

# PR #218
git diff --stat master org-caldav.el
 org-caldav.el | 1844 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------
 1 file changed, 1190 insertions(+), 654 deletions(-)

# smaller barebones version
git diff --stat master org-caldav.el
 org-caldav.el | 377 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 310 insertions(+), 67 deletions(-)

To create my new branch, I started by diff'ing this PR with master, and deleted all changes that weren't necessary for basic VTODO functionality. Deleted changes include:

Then, I squashed all the remaining changes down into a single commit (listing @whirm and @grauschnabel as coauthors).

It seems to be working now, but there may still be some bugs I missed. I'm testing it out using Radicale and Thunderbird currently.

EDIT 2022-08-01: I also created an "unsquashed" version of this branch here, to more easily see how it diverges from the current PR: https://github.com/jackkamm/org-caldav/tree/sync-todos-clean-unsquashed

grauschnabel commented 2 years ago

Great work, thanks for that @jackkamm .

For me there i no reason to keep things like days-in-past, the idea was just that we do not sync a lot of done todos. So I support to merge your work.

I couldn't see any problems with your version, but I didn't all the tests. I will use it from now and see if it works..

jackkamm commented 2 years ago

Thanks @grauschnabel -- my work is very minor compared to what you have already done.

Also, I was likely too aggressive in removing features; for example, I think I should add back a way to have multiple keywords besides TODO and DONE.

I'm continuing to push some changes to the unsquashed branch [1]. In particular, I'm cleaning up some of the code duplication, and also may revert some of the deletions I made. When it's ready, I'll re-squash and comment here, or perhaps submit an updated PR. Would be interested to hear any feedback in the meantime.

[1] https://github.com/jackkamm/org-caldav/tree/sync-todos-clean-unsquashed

jackkamm commented 2 years ago

Update: I created a branch that preserves the current functionality of this PR, while still reducing the divergence from upstream. The branch also includes several bugfixes and a unit test for the sync-todos functionality.

For now, I've opened a PR against @whirm's branch, in case he wants to include the changes into the current PR: whirm#2

Note this PR is not as small as my previous "barebones" branch, as it includes the functionality I had previously removed -- there were too many important features that I threw out before.

However the PR still achieves substantial reduction in the divergence from upstream, by removing whitespace changes, reducing code duplication, and other refactoring.

Getting closer to upstream also made it easier for me to merge in some other bugfixes (#252). And after updating the unit tests, I found a couple additional bugs which I also added fixes for.

Thaodan commented 1 year ago

Update: I created a branch that preserves the current functionality of this PR, while still reducing the divergence from upstream. The branch also includes several bugfixes and a unit test for the sync-todos functionality.

For now, I've opened a PR against @whirm's branch, in case he wants to include the changes into the current PR: whirm#2

Note this PR is not as small as my previous "barebones" branch, as it includes the functionality I had previously removed -- there were too many important features that I threw out before.

However the PR still achieves substantial reduction in the divergence from upstream, by removing whitespace changes, reducing code duplication, and other refactoring.

Getting closer to upstream also made it easier for me to merge in some other bugfixes (#252). And after updating the unit tests, I found a couple additional bugs which I also added fixes for.

Hey I'm testing the refactored/your master branch right now. How do you map the blocked state to to vtodo?

I also saw that you remove support for rrule is this intended?

Thaodan commented 1 year ago

This RFC mentions the needs-action state which could map quite nicely to a Blocked or waiting state. https://www.rfc-editor.org/rfc/rfc5545

Thaodan commented 1 year ago

I also saw that you remove support for rrule is this intended?

Restored here: https://github.com/Thaodan/org-caldav/commit/21313b7abe23a86d0283c91f1a519228a0bc2df4

Thaodan commented 1 year ago

Hey I'm testing the refactored/your master branch right now. How do you map the blocked state to to vtodo?

Answering myself: adding WAITING toorg-caldav-todo-percent-states did the trick. I'm still wondering the status field could be employed for the blocked state, however I don't know if any ui uses the needs-action state.

Thaodan commented 1 year ago

Answering myself: adding WAITING toorg-caldav-todo-percent-states did the trick. I'm still wondering the status field could be employed for the blocked state, however I don't know if any ui uses the needs-action state.

See also: https://www.kanzaki.com/docs/ical/partstat.html

edgar-vincent commented 1 year ago

@whirm @jackkamm @Thaodan @grauschnabel These changes look amazing. Since this repo is pretty much abandoned, would any of you be willing to take up maintainership? It doesn't have to the particularly active (if at all), just merging all of your great work will be good enough and would allow users to be able to benefit from these much-awaited features.

Thanks a lot!

hny-gd commented 1 year ago

Yes please. 🙂

jackkamm commented 1 year ago

I also saw that you remove support for rrule is this intended?

Don't remember if intentional, but rrule wasn't used so it had no effect. However, I saw in your branch you merged in some more work that makes use of rrule. Does it work well? I'm interested to merge it into my master, but haven't tried it yet.

I'm still wondering the status field could be employed for the blocked state, however I don't know if any ui uses the needs-action state.

I'm sympathetic to this, intuitively I'd also rather map STATUS rather than PERCENT-COMPLETE to the todo state. But I'm also hesitant to break compatibility with the older sync-todo branches, which have been battle-tested for several years now.

Thaodan commented 1 year ago

I'm still wondering the status field could be employed for the blocked state, however I don't know if any ui uses the needs-action state. I'm sympathetic to this, intuitively I'd also rather map STATUS rather than PERCENT-COMPLETE to the todo state. But I'm also hesitant to break compatibility with the older sync-todo branches, which have been battle-tested for several years now.

The issue is I think that the status field isn't used as much and percent- complete is used I would keep it.

Thaodan commented 1 year ago

Don't remember if intentional, but rrule wasn't used so it had no effect. However, I saw in your branch you merged in some more work that makes use of rrule. Does it work well? I'm interested to merge it into my master, but haven't tried it yet.

I think it used if it was added to the event since org-icalendar make use of it, I don't know about rdate but it supports rrule.

So far the conversion worked fine.

jackkamm commented 1 year ago

I've merged this into master now.

I made one backwards-incompatible change: the default value of org-caldav-todo-percent-states was changed to only use the standard built-in keywords (TODO & DONE). So you will need to set that variable if you use additional todo-keywords.

Otherwise, the functionality should be back-compatible with previous versions of this branch. Please, let me know if anything breaks for you.

Many thanks to @grauschnabel and @whirm for their work on this.

Thaodan commented 1 year ago

jackkamm @.***> writes:

I made one backwards-incompatible change: the default value of org-caldav-todo-percent-states was changed to only use the standard built-in keywords (TODO & DONE). So you will need to update that if you use additional todo-keywords.

Could you keep this optionally in? I think there's some use case for that to translate statuses like HOLD.

But maybe there are better ways to handle these.

jackkamm commented 1 year ago

Could you keep this optionally in? I think there's some use case for that to translate statuses like HOLD.

You can still accomplish this by setting org-caldav-todo-percent-states, see the docstring for an example.

I'm hesitant to include non-standard keywords in the default value -- we use org-todo to update the state after sync'ing, and I'm not sure the behavior is well defined if the state isn't present in org-todo-keywords. And everyone has their own todo keywords, so it seems better to keep the default as vanilla.

Thaodan commented 1 year ago

jackkamm @.***> writes:

I'm hesitant to include non-standard keywords in the default value -- we use org-todo to update the state after sync'ing, and I'm not sure the behavior is well defined if the state isn't present in org-todo-keywords. And everyone has their own todo keywords, so it seems better to keep the default as vanilla.

Makes sense, good.