todoman icon indicating copy to clipboard operation
todoman copied to clipboard

WIP: Add support for categories. To fix #10

Open christf opened this issue 5 years ago • 8 comments

This adds categories to todoman.

  • [x] implement setting --category multiple times
  • [x] implement test for multiple categories
  • [x] adjust table layout
  • [x] allow to write to the new table structure
  • [x] allow to edit the structure. Currently "aaaaa,bbbb" will be written as "a,a,a,a,a,,,b,b,b,b" when entered in the interactive editor. However it will be fine when specifying via clia.
  • [x] allow to read from the new table structure
  • [ ] handle the strings, forming a list for the UI.
  • [x] allow to filter tasks having one category
  • [ ] allow to filter tasks having multiple categories

christf avatar Sep 17 '18 22:09 christf

TodoWriter.set_field calls add on the vtodo. This is icalendar.Component.add, with the relevant part there: https://github.com/collective/icalendar/blob/master/src/icalendar/cal.py#L186

It seems that the categories list is not recognized as such (matches the remove of ",".join(categories) in the commit, because it put a comma after each character – sign that a string was handed over.

Possible solutions:

  • put the comma separated list in a vText object
  • fix the issue that the categories field is not recognized as list. Fixing the join issue could be a milestone here.

penguineer avatar Sep 18 '18 09:09 penguineer

I've just had a look into the proposed implementation and I vote for removing the lv:easy tag from #10. While it may seem easy to implement categories similar to location, there is the major difference that each TOOD has only one location with no further format, while there can be multiple categories.

We can take the easy route and store the categories comma-separated in a database text field. This would mostly mean:

  • put categories together when the parameter is repeated
  • initialize the Todo.categorie with None instead of [] However, with this implementation it is hard to filter by a specific category, which is the ultimate goal.

Proper database design would suggest that there is a table with tuples (category, UID), where proper clean-up, aka remove all UID entries from this mapping when a UID is removed from the cache, is necessary.

Unless we go with the less complex, but also less efficient approach:

  • Use a text field to store a comma-separated list of categories
  • Filter categories by loading all Todos with a category and then check for each item if it matches the filter. I guess the only necessary fix for this solution is to convert the category string to vText before adding the field to the icalendar.Component.

penguineer avatar Sep 18 '18 19:09 penguineer

ok, let's just lay out the tasks then.

christf avatar Sep 19 '18 18:09 christf

unfortunately the model ties the name of the command line options and the name of the field in the ics file. So the option will be called --categories.

christf avatar Sep 19 '18 23:09 christf

Regarding this comment:

unfortunately the model ties the name of the command line options and the name of the field in the ics file. So the option will be called --categories.

I believe it's possible to make the parameter name and flag name different, see here.

I know this documentation change is relatively recent, but I believe click>=6 has the same behaviour (as seen in 9b111c5297b0b6b308a1dba6c6b8fc7f566c5548)

WhyNotHugo avatar Sep 24 '18 16:09 WhyNotHugo

icalendar had some API changes around the whole categories part, so this PR might need some updating. I've also had to update master to deal with those upstream changes, so there'll be new conflicts.

WhyNotHugo avatar Oct 18 '18 04:10 WhyNotHugo

icalendar had some API changes around the whole categories part, so this PR might need some updating.

Shouldn't todoman specify the icalendar version in the requirements?

penguineer avatar Oct 18 '18 08:10 penguineer

The latest version specifies a minimum version, due to this API change being backwards incompatible.

WhyNotHugo avatar Oct 18 '18 18:10 WhyNotHugo