jspricke / python-remind

Python library to convert between Remind and iCalendar
GNU General Public License v3.0
13 stars 6 forks source link

Special tag for event status #23

Closed walderich closed 4 months ago

walderich commented 4 months ago

So, I guess I finally managed to create my first pull request 😄. According to your suggestion in remind-caldav I added the new status tag values TENTATIVE, CONFIRMED and CANCELLED, which are translated to the vevent object status.

jspricke commented 4 months ago

This looks good, thanks!

Do you also want to add the reverse like here? https://github.com/jspricke/python-remind/blob/master/remind.py#L524

walderich commented 4 months ago

Oh, I totally forgot about the reverse, as I don't use it. Unfortunately, I wasn't able to test it initially, as I got an error in the rdate attribute:

if hasattr(vevent, "rdate"):
    rdates = {rdate.date() for rdate in vevent.rdate.value}
    rdates.add(dtstart.date())
    remind.append(Remind._parse_rdate(list(rdates))) 

I had to remove both of the .date() calls, as the rdate.value seems to be a datetime.date object already.

Afterwards the status tag tests work correctly for all three possible values. The results are added to the Pull request.

jspricke commented 4 months ago

Can you attach the file that you used for testing and that triggered the rdate problem?

walderich commented 4 months ago

The issue can be reproduced with this reminder:

REM 1 OMIT Sat Sun AFTER MSG message

I guess that due to the event not defining a start time the rdate is basically a date (not datetime), and due to the OMIT values no simple repetition can be detected.

jspricke commented 4 months ago

I don't see a problem with the version from master, can you paste your complete youtput?

> echo "REM 1 OMIT Sat Sun AFTER MSG message" | rem2ics -
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//PYVOBJECT//NONSGML Version 1//EN
BEGIN:VEVENT
UID:0db3abb764f2ca68b631b974172b2772@fenchel
DTSTART;VALUE=DATE:20240101
DTEND;VALUE=DATE:20240102
DTSTAMP:19700101T010000
RDATE;VALUE=DATE:20240101,20240201,20240301,20240401,20240501,20240603,2024
 0701,20240801,20240902,20241001,20241101,20241202,20250101,20250203,202503
 03
SUMMARY:message
END:VEVENT
END:VCALENDAR
walderich commented 4 months ago

Actually, using your command works on my side as well. But I was importing data via dav2rem. It seems that the two commands are handling the import differently. rem2ics uses _gen_dtend_rrule for conversion, where date and datetime are handled differently:

rset = rrule.rruleset()
if isinstance(dtstarts[0], datetime):
    for dat in dtstarts:
        rset.rdate(dat)
else:
    for dat in dtstarts:
        rset.rdate(datetime(dat.year, dat.month, dat.day))

By using dav2rem the code uses to_remind for the import, where the above mentioned code is called:

if hasattr(vevent, "rdate"):
    rdates = {rdate.date() for rdate in vevent.rdate.value}
    rdates.add(dtstart.date())
    remind.append(Remind._parse_rdate(list(rdates)))

Here datetime and date is not handled differently.

jspricke commented 4 months ago

I see your bug now:

> echo "REM 1 OMIT Sat Sun AFTER MSG message" | rem2ics - | ics2rem
Traceback (most recent call last):
  File "/home/jspricke/.local/bin/ics2rem", line 8, in <module>
    sys.exit(ics2rem())
             ^^^^^^^^^
  File "/home/jspricke/src/python/remind/remind.py", line 770, in ics2rem
    rem = Remind(localtz=zone).to_reminders(
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jspricke/src/python/remind/remind.py", line 558, in to_reminders
    reminders = [
                ^
  File "/home/jspricke/src/python/remind/remind.py", line 559, in <listcomp>
    self.to_remind(vevent, label, priority, tags, tail, sep, postdate, posttime, locations)
  File "/home/jspricke/src/python/remind/remind.py", line 520, in to_remind
    rdates = {rdate.date() for rdate in vevent.rdate.value}
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jspricke/src/python/remind/remind.py", line 520, in <setcomp>
    rdates = {rdate.date() for rdate in vevent.rdate.value}
              ^^^^^^^^^^
AttributeError: 'datetime.date' object has no attribute 'date'

Will look into it, thanks for pointing out!

Anyway, looks unrelated to this PR so merging. Thanks for your contribution!

jspricke commented 4 months ago

Fixed in 0ba3f2c. Thanks again for pointing out!