neilgupta / Sherlock

Natural-language event parser for Javascript
https://sherlock.neil.gg
MIT License
532 stars 32 forks source link

Enhancement support times with periods/dots/fullstops, e.g. 7.15pm #35

Open clach04 opened 3 years ago

clach04 commented 3 years ago

Whilst I don't personally use times of the form 7.15pm (with a . rather than :), I have come across these in the wild and the people using this form thought it was standard.

I have a change and a test that adds support, but it fails 2 date tests:

          (function() {
            var start = getNow();

            if (start.getHours() >= 19)
              start.setDate(start.getDate() + 1);
            start.setHours(19, 15, 0, 0);
/*
diff --git a/sherlock.js b/sherlock.js
index f6265ef..1323e06 100644
--- a/sherlock.js
+++ b/sherlock.js
@@ -33,9 +33,9 @@ var Sherlock = (function() {
     inMilliTime: /\b(\d+) ?(s(?:ec(?:ond)?)?|ms|millisecond)s? ?(ago|old)?\b/,
     midtime: /(?:@ ?)?\b(?:at )?(dawn|morn(?:ing)?|noon|afternoon|evening|night|midnight)\b/,
     // 23:50, 0700, 1900
-    internationalTime: /\b(?:(0[0-9]|1[3-9]|2[0-3]):?([0-5]\d))\b/,
+    internationalTime: /\b(?:(0[0-9]|1[3-9]|2[0-3])[:\.]?([0-5]\d))\b/,
     // 5, 12pm, 5:00, 5:00pm, at 5pm, @3a
-    explicitTime: /(?:@ ?)?\b(?:at |from )?(1[0-2]|[0-2]?[1-9])(?::?([0-5]\d))? ?([ap]\.?m?\.?)?(?:o'clock)?\b/,
+    explicitTime: /(?:@ ?)?\b(?:at |from )?(1[0-2]|[0-2]?[1-9])(?:[:\.]?([0-5]\d))? ?([ap]\.?m?\.?)?(?:o'clock)?\b/,

// NOTE this breaks date tests; "12.03" and "12.12"
*/
            return test("Let's do the consult at 7.15pm this evening then.", "let's do the consult this evening then.", start, null, false);
          })(),

Is this something you would consider supporting? I'm not sure what the heuristic would be but perhaps if there is am/pm present treat it as a time otherwise treat it as a date as it does today?

(EDITED for formatting)

neilgupta commented 3 years ago

Hmm interesting. I've never seen anyone write 7.15pm but I can believe that people do. I'm open to supporting it if you can fix the root cause for the failing tests. Treating it as a date if no am/pm is provided sounds like a reasonable approach.

Off the top of my head, you could either accomplish that by adding a lookahead for am/pm to the period pattern, or add a hack in the matcher logic to prefer date over time if the match length is equivalent and the divider is a .. The latter feels hackier so I'd probably prefer the lookahead, but not sure yet if that will add other problems. Looking forward to the PR!