Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ICS 'categories' field #414

Merged
merged 3 commits into from
May 25, 2016
Merged

Support ICS 'categories' field #414

merged 3 commits into from
May 25, 2016

Conversation

pdav
Copy link
Contributor

@pdav pdav commented May 9, 2016

  • Add a categories field to the event class
  • Add the categories field to editor
  • Add the -g flag to ikhal new

pdav added 3 commits May 9, 2016 12:52
- Add a `categories` field to the `event` class
- Add the `categories` field to editor
- Add the `-g` flag to `ikhal new`
@untitaker
Copy link
Member

Implementation looks straightfoward, 👍

@@ -451,6 +458,8 @@ def relative_to(self, day, full=False):
body += ', ' + self.description.strip()
if self.location.strip() != '':
body += ', ' + self.location.strip()
if self.categories.strip() != '':
body += ', ' + self.categories.strip()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to group the calendar selector and the categories textfield together.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a matter of personal taste.

The right solution would be to have a configuration item for the order of fields. For example

order_print_normal = Time, Title
order_print_full = Title, Title, Description, Categories
order_interactive = Title, Organizer, Location!, Date, Calendar, Categories
order_editor = Title, Calendar, Location, Categories, Description, Date, Repeat, Alarm

With a !-mark if an item must be displayed even if it is empty. Would be useful to also mask "Alarm" in the editor, which I never use.

@geier
Copy link
Member

geier commented May 17, 2016

Thanks for the PR!

First let me say that I'm going to merge this PR, because categories are
probably useful for some people.

But...
adding any element between the title and start/end times of an event
means that I need to press tab one more time than before, therefore I
believe ordering or even en-/disabling of editor interface elements (as
suggested by Pierre) is a very good idea and I'll see if I can make some
time for that (but only after I finished the timezone support in the
editor and the new daywalker)!

On Mon, May 09, 2016 at 03:55:32AM -0700, Pierre DAVID wrote:

  • Add a categories field to the event class

  • Add the categories field to editor

  • Add the -g flag to ikhal new
    You can view, comment on, or merge this pull request online at:

    Support ICS 'categories' field #414

-- Commit Summary --

  • Support ICS categories

-- File Changes --

M CHANGELOG.rst (1)
M doc/source/usage.rst (5)
M khal/aux.py (6)
M khal/cli.py (5)
M khal/controllers.py (3)
M khal/khalendar/event.py (14)
M khal/ui/__init__.py (10)
M tests/aux_test.py (10)
M tests/event_test.py (1)

-- Patch Links --

https://github.com/pimutils/khal/pull/414.patch
https://github.com/pimutils/khal/pull/414.diff


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#414

@geier
Copy link
Member

geier commented May 21, 2016

In the RFC the example for multiple categories (CATEGORIES:APPOINTMENT,EDUCATION) is comma separated, while the current implementation here escapes the comma with a backslash (example: CATEGORIES:CatA\,CatB), is this an oversight or deliberately done this way (and if so, why?)?

@pdav
Copy link
Contributor Author

pdav commented May 22, 2016

This could be related to: collective/icalendar#127

@geier
Copy link
Member

geier commented May 24, 2016

Asking for your opinions: is this feature usable with that bug in icalendar?

@untitaker
Copy link
Member

Only with one category entered I suspect.

@pdav
Copy link
Contributor Author

pdav commented May 24, 2016

I intensively use categories with SOGo server, which supports only one category by event (UI design limit). So, I agree with @untitaker, this feature is usable as is, provided one does not try to use more than one category.

@geier geier merged commit 11ad0be into pimutils:master May 25, 2016
@geier
Copy link
Member

geier commented May 25, 2016

Thank you, @pdav!

@gpl34
Copy link

gpl34 commented Jun 29, 2016

ikhal, version 0.8.2.dev118+ng4c1562e

I use fastmail. If I modify an event in ikhal, I get:

debug: Sending request...
debug: 403
debug: {'Date': 'Wed, 29 Jun 2016 18:34:12 GMT', 'Content-Encoding': 'gzip', 'Transfer-Encoding': 'chunked', 'Cache-Control': 'no-cache', 'Vary': 'Accept-Encoding', 'Accept-Patch': 'application/calendar-patch; charset=utf-8', 'Server': 'nginx', 'Content-Type': 'application/xml; charset=utf-8', 'Connection': 'keep-alive'}
debug: b'<?xml version="1.0" encoding="utf-8"?>\n<D:error xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">\n  <C:valid-calendar-data/>\n  <D:responsedescription>No value for CATEGORIES property. Removing entire property:</D:responsedescription>\n</D:error>\n'

ikhal is adding a CATEGORIES even if empty (I checked the .ics file) and fastmail does not like that. If I add a category, there's no sync error.

@WhyNotHugo
Copy link
Member

WhyNotHugo commented Jun 30, 2016

I've been seeing this for a while too (though I'd been thinking it one was of my many broken, semi-experimental events).

All khal-created events are not uploading to fastmail (which is quite strict in it's validations).

I've created #453 to properly track this.

geier pushed a commit that referenced this pull request Jul 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants