zeke / remind-me

A command-line reminder tool that supports natural language dates and times, inspired by Slack's remind feature (UNMAINTAINED)
http://npm.im/remind-me
81 stars 11 forks source link

Accounting for single-numbered times with no date/day. #62

Open drodrig opened 7 years ago

drodrig commented 7 years ago

Hi. Thanks for a nice app. You might consider adjusting your code to account for something like this:

remind me to take a nap at 9

Currently the program barfs at:

/usr/local/lib/node_modules/remind-me/lib/parse.js:55 result.time = chrono.parse(result.time)[0].start.date() TypeError: Cannot read property 'start' of undefined

Adding the following code worked for me (line numbers for your reference):

40
41 if (result.time.length == 1) { 42 result.time = result.time + ':00'; 43 } 44 `

That partially worked, but the result can be in the past, which is no good, so I added:

61 62 if (result.time < new Date()) { 63 result.time = new Date(result.time).setMinutes(result.time.getMinutes() + 720); // 12 hours 64 } 65

Then I get what I am looking for:

Ok, I'll remind you to "take a nap" on Tuesday, November 29 at 9:00 pm

Thanks again and I hope this helps you. One more thing: It was easy to convert the command line util into a library that other scripts can call. If you want that code let me know.

drodrig commented 7 years ago

Hi again.

I just realized the result.time variable might not be initialized properly where I inserted my initially modified code, so I moved the code towards the bottom of the function. Your original code (parse.js, line 54):

if (typeof result.time === 'string') {
    result.time = chrono.parse(result.time)[0].start.date()
}

To this:

if (typeof result.time === 'string') {
    if (result.time.length == 1) {
        result.time = result.time + ':00';
    }
    result.time = chrono.parse(result.time)[0].start.date()
}

I also changed the code that sets the date based on an interval from this (parse.js line 46):

if (result.interval) {
         result.interval = dateparser(result.interval).value
        result.time = Date.now() + result.interval
}

To this:

      if (result.interval) {
        result.interval = dateparser(result.interval).value
        result.time = new Date(new Date().getTime() + result.interval);
      }

That way the result.time is still a date object, and not seconds since epoch. Haven't tested too much.

Cheers.

zeke commented 7 years ago

Hi! Thanks for opening the issue. I do not currently have time to work on fixes for this, but will happily accept pull requests and cut new releases. 👍

drodrig commented 7 years ago

Thank you. I created a pull request. First time doing one, so if I made a mistake just let me know.

Dave

-------- Original Message --------

Subject: Re: [zeke/remind-me] Accounting for single-numbered times with no date/day. (#62)

Local Time: November 30, 2016 12:29 PM

UTC Time: November 30, 2016 5:29 PM

From: notifications@github.com

To: zeke/remind-me remind-me@noreply.github.com

drodrig drodrig@magicbrain.com, Author author@noreply.github.com

Hi! Thanks for opening the issue. I do not currently have time to work on fixes for this, but will happily accept pull requests and cut new releases. 👍

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.