time-related reminders

[anarcat]

I am working on a restricted set of the functionalities described in #337252. I just want to have simple reminders "tell me foo in X minutes"...

[anarcat (anarcat)]

So unfortunately, I can't figure out bzr. Trying to commit gives me this silly:

anarcat@angela:plugins$ bzr commit -m"implement reminders" fun.py
bzr: ERROR: Cannot lock LockDir(lp-80483920:///~ibid-core/ibid/trunk/.bzr/branchlock): Transport operation not possible: readonly transport

So I'll just post a patch here.

[anarcat (anarcat)]

It just crossed my mind that this may be better as an extension to the memo plugin. I'd like to hear what people think of this idea here.

[anarcat (anarcat)]

Oh, and people that want to interact with this plugin can talk with kibit on irc.indymedia.org.

[Stefano Rivera (stefanor)]

First, some bzr help: http://doc.bazaar.canonical.com/latest/en/mini-tutorial/ etc.

If you commit it your change, you should be able to "bzr push lp:~anarcat/ibid/remind-648463" (or something like that). Then you can do a merge proposal.

Now review:
We generally don't show every possible usage in the help string, just enough for the user to see how to use it. If it accepts synonyms for one of the words, we assume that user won't remember exactly which one is right, and will be pleasantly surprised when it "just works".

I tend to go with a column limit of 80 chars per line, although not *strictly*, and some other Ibid developers don't limit their lines at all.

When presenting dates to the user, you should use ibid.utils.format_date, which knows about timezone preferences. For deltas, you can use ibid.utils.ago.

The actual reminder message has quite a lot of boilerplate text in it. It's nice that it's funny, but the english grammar is a bit off "back on" should be "back at" or even better "at".

If I'd written it, the response to be something like "You asked me to remind you to get the washing in, 5 hours ago."

[anarcat (anarcat)]

Hi everybody!

I tried very hard to make bzr work, even with people on IRC: no luck, it just doesn't like me and I get the same error message when I commit. I can't commit my change, so I can't push, and I'm a bit annoyed at learning bzr right now so I'll keep on submitting good old patches for now if you don't mind. :) (Note that I have read this tutorial but there must be something I did during the original clone of this repository where i screwed up bad, as it seems to be trying to commit directly upstream instead of to my local branch, even though I did bzr branch lp:ibid remind-648463. Oh well.)

So first off, thank you very much for your feedback, it's exactly the kind of feedback I was hoping for! Very constructive!

I am not a native english speaker and messing around with code and english at the same time sometimes confuses my brain, especially with placeholders instead of real phrases in there. :) I think this new version much improves upon the grammar, especially since we now carry the "prefix", the "how" ("to", "about", "of", any other i'm missing here?) in the reminder, so the output looks much more natural:

21:10:43 <anarcat> ibit-cat: ping in 1 second of the blue sky
21:10:43 * ibit-cat will remind anarcat of the blue sky in 1 second
21:10:44 <ibit-cat> anarcat: you asked me to remind you of the blue sky, 1 second ago.

21:12:56 <anarcat> ibit-cat: remind comar in 1 second about how life is so beautiful
21:12:56 * ibit-cat will remind comar about how life is so beautiful in 1 second
21:12:57 <ibit-cat> comar: anarcat asked me to remind you about how life is so beautiful, 1 second ago.

20:56:45 <anarcat> ibit-cat: ping in 5 seconds to eat something
20:56:45 * ibit-cat will remind you toeat something in 5 seconds
20:56:50 <ibit-cat> anarcat: you asked me to remind you to eat something, 5 seconds ago.

Notice how I am also using exclusively ago() which makes the whole thing much more human-readable.

I have also restricted the usage to the most common use case, but if you omit the "about", it will just ping you:

21:20:01 <anarcat> ibit-cat: remind me in 1 second
21:20:01 * ibit-cat will ping you in 1 second
21:20:02 <ibit-cat> anarcat: you asked me to ping you, 1 second ago.

... which will teach the user he can probably use ping too. :)

Also, I rephrased the response to be more logically organised, as suggested.

I'm not sure what the 80 chars limit refers to, is this the help text or the indentation? I'm willing to conform to whatever standard you choose. :) Coffee didn't seem to wrap so that's what i stuck with...

Finally, I'm reconsidering the idea of just slapping this in fun.py. It seems to me this really belongs to memo.py: it's just like a memo, but it's defered to later. It would be especially interesting to have the reminder be postponed if the user actually logged-off, because the way things stand, if the user goes away before the timeout, he will miss the reminder, while if we plug this properly in memo, it could keep the reminder for later.

It would also make the whole user experience much more consistent.

How does that sound for you guys?

Sorry again my feeble brain can't grok bzr now. :)

[Max Rabkin (max-rabkin)]

If there's a user called "me", should this ping them or the original user? Memo requires you to use your own name.

Other things to consider for a full implementation, especially if this is to be useful for memos:
* Remind users on other channels
* Save reminders in the database, so they survive bot-restarts

You also need to take timezones into account when *reading* times from the user (I don't know if we have helper functions for this).

[Stefano Rivera (stefanor)]

> it just doesn't like me and I get the same error message when I commit

You have a "bound" branch, so it tries to commit to trunk when you commit. Do this:

$ bzr unbind

[Stefano Rivera (stefanor)]

> If there's a user called "me", should this ping them or the original user?

"remind tumbleweed in 20 mins to check the oven" just doesn't sound right to me. But yes, if there is someone in the channel called me, then that probably takes precedence.

Second round of review:
Firstly: Add your name to the header of the file, and the bottom of AUTHORS.

> Maybe this should be merged with the memo plugin.
reminders and memos are handled very differently. Have you read memo?

> usage = u'remind (me|someone else) in <delta> about something'

Not mad about "<delta>", it should be "<time>", "<someone else>" should be "<person>", "something" should be "<something>"

> addresponse

Addresponse knows how to do substitution. Use dict-substitution (look at other plugins).

> now.replace(now.year,now.month,now.day,0,0,0,0)

Needs some whitespace

> elif at == "at" or at == "on":
elif at in ("at", "on"):

> event.addresponse(u"will remind %s %s in %s" % (who, what, ago(delta)), action=True)

I wouldn't repeat the message back, although repeating the time is useful, it'll let the user know when the time-parsing was wrong. Also, it probably shouldn't be an action, responses are usually not actions.

>

[anarcat (anarcat)]

Alright, I have done all this! *And* I have learned the unbind command, so now this is a real branch here. :)

So in response to the memo comments: I have read parts of memo.py and have tested it, read the help, and so on. I understand they work differently, but my point here is that a reminder that would fail to be delivered to the user (because he's absent, for example), would be converted to a memo... Would that make sense?

I think we can still merge this in at this point, it's just a possible future improvement.

Oh and regarding "me", well, if you're called "me" on irc, you are going to suffer a lot of fruitless pings and other weird things, sorry.

[anarcat (anarcat)]

What's the status here? The branches have been merged... do we need to do anything else for this to "get in"? Was this part of the 1.1 release?

Thanks for the followup.

[Stefano Rivera (stefanor)]

> do we need to do anything else for this to "get in"?

It's in. In trunk. But it'll only be released when we do the 0.2.0 release.

However it still doesn't work very well, because dateutil isn't very good at parsing dates. parsedatetime would be better. This merge needs to happen to improve it: https://code.launchpad.net/~ibid-dev/ibid/parsedatetime/+merge/46561

But it looks like Marco has lost interest for now. Maybe someone else can pick it up?