Repository: pimutils/khal
Pull request #380 (2016-03-19 14:48:52)

cli option for alarms in new events
With the new alarms feature surely some people (at least me ;)) would like to be able to specify alarms when creating new events using the cli interface. Beware: This also supports negative alarm times, so the alarm can actually be _before_ the event starts.

Also python is not my native tounge, feel free to modify this PR in any way seeming sensible to you. (I'd also appreciate feedback)

untitaker: https://github.com/pimutils/vdirsyncer/issues/154 might be relevant, where we explore options for serializing/unserializing timedeltas.
untitaker: BTW @sdx23 your commit has some styling issues, see https://travis-ci.org/pimutils/khal/jobs/117118064
sdx23: Is it really iso8601-timedeltas what you want a user to write down when creating a new event? With the current interface design I followed the input accepted by unix `sleep`. Imo "1h 15m" is much clearer than "PT1H15M".
untitaker: I agree, perhaps the leading "P" could be made optional... still I see the value in standardizing day and month suffixes, especially as those are wildly varying between programming languages.
geier: IMO we should be pretty lenient in what we accept for timedeltas in khal (as long as its unambiguous). I personally won't be typing something like `PT1H15M`. `1h 15m` much more natural in my opinion, but we should also support `15 minutes` etc. This discussion is also relevant for #70.

@sdx Could you add some tests (at least for `guesstimedeltafstr()`) and add yourself to `AUTHORS.txt`?
sdx23: Fixed the type annotation (was indeed messed up), added me to authors.txt, added tests.

How do I correctly test the tests? Also, don't we need a `# coding=utf8` line or something?
geier: To run the tests you need to at least install pytest and freezegun, you can than run `py.test khal/tests` (the excecutable might be called pytest instead of pytest).

We should probably add something to the documentation about how to run the tests.
geier: 
re coding:
as khal only runs with python3, we do not need a line like that (utf-8
encoding is the default, perhaps even the only possible one, I'm not
sure).


Quoting sdx23 (2016-03-29 23:09:07)
> Fixed the type annotation (was indeed messed up), added me to authors.txt, added tests.
> 
> How do I correctly test the tests? Also, don't we need a `# coding=utf8` line or something?
> 
> ---
> You are receiving this because you commented.
> Reply to this email directly or view it on GitHub:
> https://github.com/pimutils/khal/pull/380#issuecomment-203106157

hobarrera: Actually, just running `tox` should be enough. Upsides it that it installs
everything into its own virtualenv, and will tests with multiple python
versions, if available.

On 2016-03-29 14:27, Christian Geier wrote:
> To run the tests you need to at least install pytest and freezegun, you can than run `py.test khal/tests` (the excecutable might be called pytest instead of pytest).
> 
> We should probably add something to the documentation about how to run the tests.
> 
> ---
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly or view it on GitHub:
> https://github.com/pimutils/khal/pull/380#issuecomment-203113303

-- 
Hugo Osvaldo Barrera
A: Because we read from top to bottom, left to right.
Q: Why should I start my reply below the quoted text?

sdx23: The test can right now not be adapted to fit the changes introduced in #383, as the dict does not allow for duplicate entries :cry:
geier: I'm not sure if we should accept something like `23foo10min` and interpret it as `10 minutes`, at least not longterm. 

Also, why don't you just make the TRIGGER times all negative?
sdx23: For `23foo10min` true. I'll have to look into how to fail the whole thing for invalid input (should anyways, I guess).

Regarding negative: I'd rather do it consistent with gui first and fix #386 together with this.
three-comrades: > The test can right now not be adapted to fit the changes introduced in #383, as the dict does not allow for duplicate entries :cry:

I wouldn't bother. Most test cases in `aux_test.py` are very similar and only differ in two or three lines. For those, #386 made sense. Your test case is a lot longer, so they wouldn't be reduced by much, anyway. :) 
sdx23: The times are now inverted, consistent with #386. Also, I cleaned up all that mess of commits, rebasing and squashing it into one.

Patch:
diff --git a/AUTHORS.txt b/AUTHORS.txt
index c439212..c5b2e54 100644
--- a/AUTHORS.txt
+++ b/AUTHORS.txt
@@ -22,3 +22,4 @@ Johannes Wienke - languitar [at] semipol [dot] de - https://www.semipol.de
 Laurent Arnoud - laurent [at] spkdev [dot] net - http://spkdev.net/
 Julian Mehne
 Stephan Weller
+Max Voit - max.voit+dvkh [at] with-eyes [dot] net
\ No newline at end of file
diff --git a/khal/aux.py b/khal/aux.py
index e8a81a2..35a61e7 100644
--- a/khal/aux.py
+++ b/khal/aux.py
@@ -26,6 +26,7 @@ from calendar import isleap
 from datetime import date, datetime, timedelta, time
 import random
 import string
+import re
 from time import strptime
 
 import icalendar
@@ -210,6 +211,43 @@ def guessdatetimefstr(dtime_list, locale, default_day=None):
     raise ValueError()
 
 
+def guesstimedeltafstr(delta_string):
+    """parses a timedelta from a string
+
+    :param delta_string: string encoding time-delta, e.g. '1h 15m'
+    :type delta_string: str
+    :rtype: datetime.timedelta
+    """
+
+    tups = re.split(r'(-?\d+)', delta_string)
+    if not re.match(r'^\s*$', tups[0]):
+        raise ValueError('Invalid beginning of timedelta string "%s": "%s"'
+                         % (delta_string, tups[0]))
+    tups = tups[1:]
+    res = timedelta()
+
+    for num, unit in zip(tups[0::2], tups[1::2]):
+        try:
+            numint = int(num)
+        except ValueError:
+            raise ValueError('Invalid number in timedelta string "%s": "%s"'
+                             % (delta_string, num))
+
+        ulower = unit.lower().strip()
+        if ulower == 'd' or ulower == 'day' or ulower == 'days':
+            res += timedelta(days=numint)
+        elif ulower == 'h' or ulower == 'hour' or ulower == 'hours':
+            res += timedelta(hours=numint)
+        elif (ulower == 'm' or ulower == 'minute' or ulower == 'minutes' or
+              ulower == 'min'):
+            res += timedelta(minutes=numint)
+        else:
+            raise ValueError('Invalid unit in timedelta string "%s": "%s"'
+                             % (delta_string, unit))
+
+    return res
+
+
 def generate_random_uid():
     """generate a random uid
 
@@ -222,7 +260,7 @@ def generate_random_uid():
 def construct_event(dtime_list, locale,
                     defaulttimelen=60, defaultdatelen=1, encoding='utf-8',
                     description=None, location=None, repeat=None, until=None,
-                    **kwargs):
+                    alarm=None, **kwargs):
     """takes a list of strings and constructs a vevent from it
 
     :param encoding: the encoding of your terminal, should be a valid encoding
@@ -359,6 +397,13 @@ def construct_event(dtime_list, locale,
             logger.fatal("Invalid value for the repeat option. \
                     Possible values are: daily, weekly, monthly or yearly")
             raise FatalError()
+    if alarm:
+        alarm_trig = -1 * guesstimedeltafstr(alarm)
+        new_alarm = icalendar.Alarm()
+        new_alarm.add('ACTION', 'DISPLAY')
+        new_alarm.add('TRIGGER', alarm_trig)
+        new_alarm.add('DESCRIPTION', description)
+        event.add_component(new_alarm)
 
     event.add('dtstart', dtstart)
     event.add('dtend', dtend)
diff --git a/khal/cli.py b/khal/cli.py
index feb7040..c527166 100644
--- a/khal/cli.py
+++ b/khal/cli.py
@@ -311,13 +311,16 @@ def _get_cli():
                   help=('Repeat event: daily, weekly, monthly or yearly.'))
     @click.option('--until', '-u',
                   help=('Stop an event repeating on this date.'))
+    @click.option('--alarm', '-m',
+                  help=('Alarm time for the new event.'))
     @click.argument('START', nargs=1, required=True)
     @click.argument('END', nargs=1, required=False)
     @click.argument('TIMEZONE', nargs=1, required=False)
     @click.argument('SUMMARY', metavar='SUMMARY', nargs=1, required=False)
     @click.argument('DESCRIPTION', metavar='[:: DESCRIPTION]', nargs=-1, required=False)
     @click.pass_context
-    def new(ctx, calendar, start, end, timezone, summary, description, location, repeat, until):
+    def new(ctx, calendar, start, end, timezone, summary, description, location, repeat,
+            until, alarm):
         '''Create a new event from arguments.
 
         START and END can be either dates, times or datetimes, please have a
@@ -338,6 +341,7 @@ def _get_cli():
             location=location,
             repeat=repeat,
             until=until.split(' ') if until is not None else None,
+            alarm=alarm,
         )
 
     @cli.command('import')
diff --git a/khal/controllers.py b/khal/controllers.py
index 1202626..576a766 100644
--- a/khal/controllers.py
+++ b/khal/controllers.py
@@ -179,7 +179,7 @@ def agenda(collection, dates=None, encoding='utf-8', show_all_days=False, full=F
 
 
 def new_from_string(collection, calendar_name, conf, date_list, location=None, repeat=None,
-                    until=None):
+                    until=None, alarm=None):
     """construct a new event from a string and add it"""
     try:
         event = aux.construct_event(
@@ -187,6 +187,7 @@ def new_from_string(collection, calendar_name, conf, date_list, location=None, r
             location=location,
             repeat=repeat,
             until=until,
+            alarm=alarm,
             locale=conf['locale'])
     except FatalError:
         sys.exit(1)


Test patch:
diff --git a/tests/aux_test.py b/tests/aux_test.py
index c8985ea..5aa431d 100644
--- a/tests/aux_test.py
+++ b/tests/aux_test.py
@@ -7,7 +7,7 @@ import icalendar
 import pytz
 from freezegun import freeze_time
 
-from khal.aux import construct_event, guessdatetimefstr
+from khal.aux import construct_event, guessdatetimefstr, guesstimedeltafstr
 from khal import aux
 from khal.exceptions import FatalError
 import pytest
@@ -96,6 +96,35 @@ class TestGuessDatetimefstr(object):
             guessdatetimefstr('16:00'.split(), locale=locale_de, default_day=tomorrow)
 
 
+class TestGuessTimedeltafstr(object):
+
+    def test_single(self):
+        assert timedelta(minutes=10) == guesstimedeltafstr('10m')
+
+    def test_negative(self):
+        assert timedelta(minutes=-10) == guesstimedeltafstr('-10m')
+
+    def test_multi(self):
+        assert timedelta(days=1, hours=-3, minutes=10) == \
+            guesstimedeltafstr(' 1d -3H 10min ')
+
+    def test_multi_nospace(self):
+        assert timedelta(days=1, hours=-3, minutes=10) == \
+            guesstimedeltafstr('1D-3hour10m')
+
+    def test_garbage(self):
+        with pytest.raises(ValueError):
+                guesstimedeltafstr('10mbar')
+
+    def test_moregarbage(self):
+        with pytest.raises(ValueError):
+                guesstimedeltafstr('foo10m')
+
+    def test_same(self):
+        assert timedelta(minutes=20) == \
+            guesstimedeltafstr('10min 10minutes')
+
+
 test_set_format_de = _create_testcases(
     # all-day-events
     # one day only
@@ -294,6 +323,33 @@ def test_repeat():
             assert _replace_uid(event).to_ical() == vevent
 
 
+test_set_alarm = _create_testcases(
+    ('8:00 Äwesöme Event',
+     ['BEGIN:VEVENT',
+      'SUMMARY:Äwesöme Event',
+      'DTSTART;TZID=Europe/Berlin;VALUE=DATE-TIME:20140216T080000',
+      'DTEND;TZID=Europe/Berlin;VALUE=DATE-TIME:20140216T090000',
+      'DTSTAMP;VALUE=DATE-TIME:20140216T120000Z',
+      'UID:E41JRQX2DB4P1AQZI86BAT7NHPBHPRIIHQKA',
+      'DESCRIPTION:please describe the event',
+      'BEGIN:VALARM',
+      'ACTION:DISPLAY',
+      'DESCRIPTION:please describe the event',
+      'TRIGGER:-PT23M',
+      'END:VALARM',
+      'END:VEVENT']))
+
+
+def test_alarm():
+    for data_list, vevent in test_set_alarm:
+        with freeze_time('2014-02-16 12:00:00'):
+            event = construct_event(data_list.split(),
+                                    description='please describe the event',
+                                    alarm='23m',
+                                    locale=locale_de)
+            assert _replace_uid(event).to_ical() == vevent
+
+
 test_set_description_and_location = _create_testcases(
     # now events where the start date has to be inferred, too
     # today