Sherlock icon indicating copy to clipboard operation
Sherlock copied to clipboard

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

Open clach04 opened this issue 3 years ago • 1 comments

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)

clach04 avatar Aug 13 '21 01:08 clach04

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!

neilgupta avatar Aug 13 '21 16:08 neilgupta