Skip to content

Todo support#113

Merged
C4ptainCrunch merged 18 commits intoics-py:masterfrom
tgamauf:master
Mar 19, 2018
Merged

Todo support#113
C4ptainCrunch merged 18 commits intoics-py:masterfrom
tgamauf:master

Conversation

@tgamauf
Copy link
Copy Markdown
Contributor

@tgamauf tgamauf commented Feb 19, 2018

Hi,

thank's for the library! Since VTODO support was missing and I needed it, I have implemented it. The code is covered by unit tests and I also have tested it with my own ics todo lists.

Beside the mandatory attributes (dtstamp, uid), I've also implemented the attributes completed, created, description, dtstart, location, percent, priority, summary, and url.
I tried to keep my implementation of Todo close to your implementation of Events, but I diverged in a few areas:

  1. Attribute DTSTAMP in Events is mapped to variable created. As the standard defines a CREATED attribute, I didn't follow you there, but added a separate variable dtstamp. created is now filled from CREATED.
  2. I thought about implementing the logical operators (xor, or, and, ...), join, and so on, but decided against it. The reason is that I just don't see how this really would make sense for todo list entries.
  3. The comparison operators (<, <=, =>, >) operate on the due time (DUE), not the begin time (DTSTART), as I believe that this makes more sense. At least this is the case for the way I use them.

In addition, I found some behaviors of the parser I wasn't sure about, but didn't investigate:

  1. A timestamp which contains seconds, e.g. 2018-02-19 12:00:05 will be stored as 2018-02-19 12:00:00. In my opinion seconds are irrelevant here, but it was surprising none-the-less.
  2. Timezone information of time fields is ignored, e.g. if COMPLETED:;TZID=Europe/Brussels:20180418T150001 is parsed, it won't store the timezone.

It would be amazing if you could have a look and if it makes sense merge it into your library!

Thanks,
Thomas

Thomas Gamauf added 16 commits February 10, 2018 20:27
feat: implement VTODO component.
e4a0820
fix: corrects implementation of dtstamp vs. created.
c1ea7b9 
The DTSTAMP attribute was set as CREATED, but this isn't
a correct implementation according to RFC5545. Now DTSTAMP
and CREATED are distinct attributes.
fix: sets COMPLETED attribute only if it is defined.
39a3921
fix: fix implementations of basic operators lt, le, gt, ge.
c69d95e
test: implements init, and basic operator tests.
aa38f80
test: implement set/get attribute tests.
336e5e6
fix: unicode representation of Todo.
bbfd9b0
fix: remove some time related methods.
68b02d9 
I see no use case where starts_within, ends_within, intersect/__xor__,
or time_equal make sense for a Todo.
fix: extend Calendar to fully support Todo.
bd20ea7
fix: percent and priority attributes.
285debc
test: add tests for percent, priority, and first cal data import.
513cf75
fix: fix invalid dtstamp output.
95508bb
fix: remove check for begin from duration output.
881e799
test: add tests for extraction and output of ics text.
4a067a4
test: add Todo related calendar test.
724bebb
test: added missing VTODO part in unfolded_cal1.
b27412b
@tgamauf
Copy link
Copy Markdown
Contributor Author

tgamauf commented Feb 19, 2018

I didn't consider Python2.7 - I will fix it and recommit as soon as possible.

@tgamauf tgamauf mentioned this pull request Feb 20, 2018
Closed
@C4ptainCrunch
Copy link
Copy Markdown
Member

C4ptainCrunch commented Mar 19, 2018

Wow, that's a pretty big PR, thanks a lot !
TODO support was long overdue (pun intended), thank you for writing it :)

It might need a little more documentation but i'll merge anyway, documentation can come later.

I thought about implementing the logical operators (xor, or, and, ...), join, and so on, but decided against it. The reason is that I just don't see how this really would make sense for todo list entries.

👍

The comparison operators (<, <=, =>, >) operate on the due time (DUE), not the begin time (DTSTART), as I believe that this makes more sense. At least this is the case for the way I use them.

It makes sense for me too.

A timestamp which contains seconds, e.g. 2018-02-19 12:00:05 will be stored as 2018-02-19 12:00:00. In my opinion seconds are irrelevant here, but it was surprising none-the-less.

It look awfully like a bug :( Could you file an issue if you have some spare time ?

Timezone information of time fields is ignored, e.g. if COMPLETED:;TZID=Europe/Brussels:20180418T150001 is parsed, it won't store the timezone.

This also looks like a bug, it might be from all the arrow mess that i have to clean. I'll look into it (but feel free to open an issue with code example if you want to)

@C4ptainCrunch C4ptainCrunch merged commit 36f1553 into ics-py:master Mar 19, 2018
This was referenced Jun 26, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tgamauf @C4ptainCrunch