[california] Fix Quick Add bug parsing "12:30pm"



commit 3bfdd5e4f1b1cd985ddc81374f62ed4b613a674f
Author: Jim Nelson <jim yorba org>
Date:   Fri May 30 14:20:15 2014 -0700

    Fix Quick Add bug parsing "12:30pm"
    
    Internally time is represented by a 24-hour clock and the parser
    was adding 12 to 12pm to convert.  Fixed and tests added.
    
    Added conversion of "2400" to midnight.
    
    In quick parser, if no date is specified but midnight is, it
    assumes the event is midnight of the next day.  Might generalize this
    logic to always use next day if no day specified and time is in the
    current day's past.

 src/calendar/calendar-date.vala             |   38 ++++++++++++++
 src/calendar/calendar-unit.vala             |   10 ++--
 src/calendar/calendar-wall-time.vala        |   25 ++++++++-
 src/component/component-details-parser.vala |   23 +++------
 src/tests/tests-calendar-date.vala          |   52 +++++++++++++++++++
 src/tests/tests-quick-add.vala              |   72 +++++++++++++++++++++++++++
 6 files changed, 196 insertions(+), 24 deletions(-)
---
diff --git a/src/calendar/calendar-date.vala b/src/calendar/calendar-date.vala
index a8beab0..ce762f0 100644
--- a/src/calendar/calendar-date.vala
+++ b/src/calendar/calendar-date.vala
@@ -248,6 +248,44 @@ public class Date : Unit<Date>, Gee.Comparable<Date>, Gee.Hashable<Date> {
     }
     
     /**
+     * Returns the { link Date} of the upcoming (next chronological) { link DayOfWeek}.
+     *
+     * Set { link includes_this_day} to true if this Date is to be considered "upcoming", that is,
+     * if it falls on the day of the week, it is returned.
+     *
+     * @see prior
+     */
+    public Date upcoming(DayOfWeek dow, bool includes_this_day) {
+        return upcoming_prior(dow, includes_this_day, 1);
+    }
+    
+    /**
+     * Returns the { link Date} of the prior (previous chronological) { link DayOfWeek}.
+     *
+     * Set { link includes_this_day} to true if this Date is to be considered "prior", that is,
+     * if it falls on the day of the week, it is returned.
+     *
+     * @see upcoming
+     */
+    public Date prior(DayOfWeek dow, bool includes_this_day) {
+        return upcoming_prior(dow, includes_this_day, -1);
+    }
+    
+    private Date upcoming_prior(DayOfWeek dow, bool includes_this_day, int adjustment) {
+        // look for current date being the one
+        if (day_of_week.equal_to(dow) && includes_this_day)
+            return this;
+        
+        // find a Date for day of the week ... brute force isn't great, but it works
+        Date upcoming_prior = this;
+        for (;;) {
+            upcoming_prior = upcoming_prior.adjust(adjustment);
+            if (upcoming_prior.day_of_week.equal_to(dow))
+                return upcoming_prior;
+        }
+    }
+    
+    /**
      * @inheritDoc
      */
     public override int difference(Date other) {
diff --git a/src/calendar/calendar-unit.vala b/src/calendar/calendar-unit.vala
index 4717e48..25812e4 100644
--- a/src/calendar/calendar-unit.vala
+++ b/src/calendar/calendar-unit.vala
@@ -80,7 +80,8 @@ public abstract class Unit<G> : Span, Collection.SimpleIterable<Date> {
     /**
      * Returns the number of { link Unit}s between the two.
      *
-     * If the supplied Unit is earlier than this one, a negative value is returned.
+     * If the supplied Unit is earlier than this one, a negative value is returned.  (In other
+     * words, future days are positive, past days are negative.)
      */
     public abstract int difference(G other);
     
@@ -88,8 +89,7 @@ public abstract class Unit<G> : Span, Collection.SimpleIterable<Date> {
      * True if the { link Unit} contains the specified { link Date}.
      *
      * This is named to conform to Vala's rule for automatic syntax support.  This allows for the
-     * ''in'' operator to function on DiscreteUnits, but only for Dates (which is a common
-     * operation).
+     * ''in'' operator to function on Units, but only for Dates (which is a common operation).
      */
     public bool contains(Date date) {
         return has_date(date);
@@ -100,8 +100,8 @@ public abstract class Unit<G> : Span, Collection.SimpleIterable<Date> {
      * { link Unit}'s span of time.
      *
      * This is named to conform to Vala's rule for automatic iterator support.  This allows for
-     * the ''foreach'' operator to function on DiscreteUnits, but only for Dates (which is a
-     * common operation).
+     * the ''foreach'' operator to function on Units, but only for Dates (which is a common
+     * operation).
      */
     public Collection.SimpleIterator<Date> iterator() {
         return date_iterator();
diff --git a/src/calendar/calendar-wall-time.vala b/src/calendar/calendar-wall-time.vala
index 7145474..57de06d 100644
--- a/src/calendar/calendar-wall-time.vala
+++ b/src/calendar/calendar-wall-time.vala
@@ -186,20 +186,39 @@ public class WallTime : BaseObject, Gee.Comparable<WallTime>, Gee.Hashable<WallT
                 m = int.parse(token.slice(2, 4));
             }
             
-            if (!meridiem_unknown && pm)
+            // only convert 12hr -> 24hr if meridiem is known, is PM, and not 12pm (which is 12
+            // in 24-hour time as well)
+            if (!meridiem_unknown && pm && h != 12)
                 h += 12;
             
+            // accept "24:00" or "2400" as midnight
+            if (h == 24 && m == 0)
+                h = 0;
+            
+            // basic bounds checking; WallTime ctor will clamp, but for parsing prefer to be a
+            // little strict in this case
+            if (h < MIN_HOUR || h > MAX_HOUR || m < MIN_MINUTE || m > MAX_MINUTE)
+                return null;
+            
             strictly_parsed = true;
             
             return new WallTime(h, m, 0);
         }
         
         // otherwise, treat as short-form 12-hour time (even if meridiem is unknown, i.e. "8" is
-        // treated as "8:00am"
+        // treated as "8:00am" ... 12pm is 12 in 24-hour clock
         int h = int.parse(token);
-        if (!meridiem_unknown && pm)
+        if (!meridiem_unknown && pm && h != 12)
             h += 12;
         
+        // accept "24" as midnight
+        if (h == 24)
+            h = 0;
+        
+        // basic bounds checking to avoid WallTime ctor clamping
+        if (h < MIN_HOUR || h > MAX_HOUR)
+            return null;
+        
         strictly_parsed = !meridiem_unknown;
         
         return new WallTime(h, 0, 0);
diff --git a/src/component/component-details-parser.vala b/src/component/component-details-parser.vala
index 5bd1f20..20c42c3 100644
--- a/src/component/component-details-parser.vala
+++ b/src/component/component-details-parser.vala
@@ -179,9 +179,13 @@ public class DetailsParser : BaseObject {
             }
         }
         
-        // if no start date was described but a start time was, assume for today
-        if (start_date == null && start_time != null)
+        // if no start date was described but a start time was, assume for today *unless* midnight
+        // was specified, in which case, tomorrow
+        if (start_date == null && start_time != null) {
             start_date = Calendar.System.today;
+            if (start_time.equal_to(Calendar.WallTime.earliest))
+                start_date = start_date.next();
+        }
         
         // if no end date was describe, assume ends today as well (unless midnight was crossed
         // due to duration)
@@ -343,21 +347,8 @@ public class DetailsParser : BaseObject {
         
         // attempt to parse into day of the week
         Calendar.DayOfWeek? dow = Calendar.DayOfWeek.parse(token.casefolded);
-        if (dow == null)
-            return null;
-        
-        // find a Date for day of the week ... starting today, move forward up to one
-        // week
-        Calendar.Date upcoming = Calendar.System.today;
-        Calendar.Date next_week = upcoming.adjust_by(1, Calendar.DateUnit.WEEK);
-        do {
-            if (upcoming.day_of_week.equal_to(dow))
-                return upcoming;
-            
-            upcoming = upcoming.next();
-        } while (!upcoming.equal_to(next_week));
         
-        return null;
+        return (dow != null) ? Calendar.System.today.upcoming(dow, true) : null;
     }
     
     // Parses potential date specifiers into a specific calendar date
diff --git a/src/tests/tests-calendar-date.vala b/src/tests/tests-calendar-date.vala
index fb05d3d..0291696 100644
--- a/src/tests/tests-calendar-date.vala
+++ b/src/tests/tests-calendar-date.vala
@@ -12,6 +12,12 @@ private class CalendarDate : UnitTest.Harness {
         add_case("clamp-end", clamp_end);
         add_case("clamp-both", clamp_both);
         add_case("clamp-neither", clamp_neither);
+        add_case("difference-pos", difference_pos);
+        add_case("difference-neg", difference_neg);
+        add_case("upcoming", upcoming);
+        add_case("prior", prior);
+        add_case("upcoming-today", upcoming_today);
+        add_case("upcoming-next-week", upcoming_next_week);
     }
     
     protected override void setup() throws Error {
@@ -61,6 +67,52 @@ private class CalendarDate : UnitTest.Harness {
         
         return adj.start_date.equal_to(span.start_date) && adj.end_date.equal_to(span.end_date);
     }
+    
+    private bool difference_pos() throws Error {
+        Calendar.Date today = Calendar.System.today;
+        Calendar.Date tomorrow = today.next();
+        
+        return today.difference(tomorrow) == 1;
+    }
+    
+    private bool difference_neg() throws Error {
+        Calendar.Date today = Calendar.System.today;
+        Calendar.Date day_before_yesterday = today.previous().previous();
+        
+        return today.difference(day_before_yesterday) == -2;
+    }
+    
+    private bool upcoming() throws Error {
+        Calendar.Date today = Calendar.System.today;
+        Calendar.Date upcoming_fri = today.upcoming(Calendar.DayOfWeek.FRI, false);
+        int diff = today.difference(upcoming_fri);
+        
+        return diff > 0 && diff <= 7;
+    }
+    
+    private bool prior() throws Error {
+        Calendar.Date today = Calendar.System.today;
+        Calendar.Date prior_tue = today.prior(Calendar.DayOfWeek.TUE, false);
+        int diff = today.difference(prior_tue);
+        
+        return diff < 0 && diff >= -7;
+    }
+    
+    private bool upcoming_today() throws Error {
+        Calendar.Date today = Calendar.System.today;
+        Calendar.Date another_today = today.upcoming(today.day_of_week, true);
+        int diff = today.difference(another_today);
+        
+        return diff == 0;
+    }
+    
+    private bool upcoming_next_week() throws Error {
+        Calendar.Date today = Calendar.System.today;
+        Calendar.Date next_week = today.upcoming(today.day_of_week, false);
+        int diff = today.difference(next_week);
+        
+        return diff == 7;
+    }
 }
 
 }
diff --git a/src/tests/tests-quick-add.vala b/src/tests/tests-quick-add.vala
index 3d6bf8c..78f1147 100644
--- a/src/tests/tests-quick-add.vala
+++ b/src/tests/tests-quick-add.vala
@@ -20,6 +20,11 @@ private class QuickAdd : UnitTest.Harness {
         add_case("dialog-example", dialog_example);
         add_case("noon", noon);
         add_case("midnight", midnight);
+        add_case("pm1230", pm1230);
+        add_case("bogus-time", bogus_time);
+        add_case("zero-hour", zero_hour);
+        add_case("oh-twenty-four-hours", oh_twenty_four_hours);
+        add_case("midnight-to-one", midnight_to_one);
     }
     
     protected override void setup() throws Error {
@@ -165,6 +170,73 @@ private class QuickAdd : UnitTest.Harness {
             && parser.event.exact_time_span.start_exact_time.equal_to(start)
             && parser.event.exact_time_span.end_exact_time.equal_to(end);
     }
+    
+    private bool pm1230() throws Error {
+        Component.DetailsParser parser = new Component.DetailsParser(
+            "12:30pm Friday Lunch with Eric and Charles", null);
+        
+        Calendar.Date friday = Calendar.System.today.upcoming(Calendar.DayOfWeek.FRI, true);
+        
+        Calendar.ExactTime start = new Calendar.ExactTime(Calendar.Timezone.local, friday,
+            new Calendar.WallTime(12, 30, 0));
+        Calendar.ExactTime end = new Calendar.ExactTime(Calendar.Timezone.local, friday,
+            new Calendar.WallTime(13, 30, 0));
+        
+        return parser.event.summary == "Lunch with Eric and Charles"
+            && parser.event.exact_time_span.start_exact_time.equal_to(start)
+            && parser.event.exact_time_span.end_exact_time.equal_to(end);
+    }
+    
+    private bool bogus_time() throws Error {
+        Component.DetailsParser parser = new Component.DetailsParser(
+            "Dinner 25:00", null);
+        
+        return parser.event.summary == "Dinner 25:00"
+            && parser.event.exact_time_span == null
+            && parser.event.date_span == null;
+    }
+    
+    private bool zero_hour() throws Error {
+        Component.DetailsParser parser = new Component.DetailsParser(
+            "Dinner 00:00", null);
+        
+        Calendar.ExactTime start = new Calendar.ExactTime(Calendar.Timezone.local, 
Calendar.System.today.next(),
+            new Calendar.WallTime(0, 0, 0));
+        Calendar.ExactTime end = new Calendar.ExactTime(Calendar.Timezone.local, 
Calendar.System.today.next(),
+            new Calendar.WallTime(1, 0, 0));
+        
+        return parser.event.summary == "Dinner"
+            && parser.event.exact_time_span.start_exact_time.equal_to(start)
+            && parser.event.exact_time_span.end_exact_time.equal_to(end);
+    }
+    
+    private bool oh_twenty_four_hours() throws Error {
+        Component.DetailsParser parser = new Component.DetailsParser(
+            "Dinner 24:00", null);
+        
+        Calendar.ExactTime start = new Calendar.ExactTime(Calendar.Timezone.local, 
Calendar.System.today.next(),
+            new Calendar.WallTime(0, 0, 0));
+        Calendar.ExactTime end = new Calendar.ExactTime(Calendar.Timezone.local, 
Calendar.System.today.next(),
+            new Calendar.WallTime(1, 0, 0));
+        
+        return parser.event.summary == "Dinner"
+            && parser.event.exact_time_span.start_exact_time.equal_to(start)
+            && parser.event.exact_time_span.end_exact_time.equal_to(end);
+    }
+    
+    private bool midnight_to_one() throws Error {
+        Component.DetailsParser parser = new Component.DetailsParser(
+            "Dinner midnight to 1am", null);
+        
+        Calendar.ExactTime start = new Calendar.ExactTime(Calendar.Timezone.local, 
Calendar.System.today.next(),
+            new Calendar.WallTime(0, 0, 0));
+        Calendar.ExactTime end = new Calendar.ExactTime(Calendar.Timezone.local, 
Calendar.System.today.next(),
+            new Calendar.WallTime(1, 0, 0));
+        
+        return parser.event.summary == "Dinner"
+            && parser.event.exact_time_span.start_exact_time.equal_to(start)
+            && parser.event.exact_time_span.end_exact_time.equal_to(end);
+    }
 }
 
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]