Closed
Bug 954284
Opened 10 years ago
Closed 10 years ago
Twitter commands
Categories
(Chat Core :: Twitter, enhancement)
Chat Core
Twitter
Tracking
(Not tracked)
RESOLVED
FIXED
1.4
People
(Reporter: clokep, Assigned: clokep)
References
Details
Attachments
(1 file, 3 obsolete files)
3.37 KB,
patch
|
aleth
:
review+
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 851 at 2011-06-22 01:21:00 UTC *** This might overlap a bit with other bugs (bug 954117 (bio 682) and bug 954119 (bio 684)). But twitter should have some commands built in, most of them can probably be gleaned from the SMS commands [1]. But we should really have a /follow, /unfollow, /rt, @ (at message), /d (direct message) commands at least. [1] http://support.twitter.com/entries/14020-official-twitter-text-commands
Assignee | ||
Updated•10 years ago
|
Component: General → Twitter
Comment 2•10 years ago
|
||
*** Original post on bio 851 at 2012-02-11 00:02:46 UTC *** /follow and /unfollow already exist, but are not in /help? (This came up in IRC) Also, /follow strangely enough responds with a system message (good) and displaying a repeat of the last tweet the user sent (bad).
Comment 3•10 years ago
|
||
*** Original post on bio 851 at 2012-02-11 00:04:59 UTC *** Whereas /unfollow does not respond with any kind of message :( So there is no feedback whether the command succeeded.
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 851 at 2012-02-11 05:25:07 UTC *** (In reply to comment #2) > /follow and /unfollow already exist, but are not in /help? (This came up in > IRC) No, FOLLOW and UNFOLLOW are handled by the server when the server receives them (as they are for SMS), we should make actually /follow and /unfollow commands, however.
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 851 as attmnt 2257 at 2013-03-09 14:22:00 UTC *** This adds a "follow" and "unfollow" command.
Attachment #8354022 -
Flags: review?(aleth)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment 6•10 years ago
|
||
Comment on attachment 8354022 [details] [diff] [review] Patch *** Original change on bio 851 attmnt 2257 at 2013-03-09 14:29:56 UTC *** Thanks for fixing this! >+command.follow=%S <screen name>: Start following tweets from a user >+command.unfollow=%S <screen name>: Stop following tweets from a user Nit: Doesn't twitter use "username" when referring to this to users? (E.g. twitter.com) >+ run: function(aMsg, aConv) { >+ if (aMsg.contains(" ")) >+ return false; For both of these, I think adding something of the sort aMsg = aMsg.trim(); if (!aMsg) return false first would be a good idea.
Attachment #8354022 -
Flags: review?(aleth) → review-
Comment 7•10 years ago
|
||
*** Original post on bio 851 at 2013-03-09 14:31:19 UTC *** I'd say we should support lists of user names for both commands.
Assignee | ||
Comment 8•10 years ago
|
||
*** Original post on bio 851 as attmnt 2258 at 2013-03-09 15:04:00 UTC *** This adds a aMsg.trim() and then splits on spaces and follows/unfollows each one. I tested it with aleth and Mic's twitter handles (and bwinton's by mistake...)
Attachment #8354023 -
Flags: review?(aleth)
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8354022 [details] [diff] [review] Patch *** Original change on bio 851 attmnt 2257 at 2013-03-09 15:04:08 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354022 -
Attachment is obsolete: true
Comment 10•10 years ago
|
||
Comment on attachment 8354023 [details] [diff] [review] Patch v2 *** Original change on bio 851 attmnt 2258 at 2013-03-09 15:15:15 UTC *** Thanks!
Attachment #8354023 -
Flags: review?(aleth) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 11•10 years ago
|
||
Comment on attachment 8354023 [details] [diff] [review] Patch v2 *** Original change on bio 851 attmnt 2258 at 2013-03-11 12:34:44 UTC *** >diff --git a/chat/locales/en-US/twitter.properties b/chat/locales/en-US/twitter.properties >+command.follow=%S <username>[ <username>]: Start following tweets from a user >+command.unfollow=%S <username>[ <username>]: Stop following tweets from a user Shouldn't there be a period at the end of these sentences? We do it for IRC at least: http://lxr.instantbird.org/instantbird/source/chat/locales/en-US/irc.properties#49 Is "Start following tweets from a user" really better than "Start following a user"? If the number of usernames can be arbitrary, shouldn't we include [ ...] inside the help string? How do we do it in other similar cases? Should we write "a user / users" in the help string? >+ commands: [ >+ { >+ name: "follow", >+ get helpString() _("command.follow", "follow"), Is there a reason why we need a formatted string here? Seems to be just for consistency with what we do elsewhere, but do you remember why we do it elsewhere? >+ run: function(aMsg, aConv) { >+ aMsg = aMsg.trim(); >+ if (!aMsg) >+ return false; >+ let account = getAccount(aConv); >+ aMsg.split(" ").forEach(function(aName) account.follow(aName)); The function created here seems a waste, would .forEach(account.follow); work? If not, would .forEach(account.follow, account); work? >+ name: "unfollow", >+ get helpString() _("command.unfollow", "unfollow"), >+ run: function(aMsg, aConv) { >+ aMsg = aMsg.trim(); >+ if (!aMsg) >+ return false; >+ let account = getAccount(aConv); >+ aMsg.split(" ").forEach(function(aName) account.stopFollowing(aName)); Same here.
Attachment #8354023 -
Flags: review-
Assignee | ||
Comment 12•10 years ago
|
||
*** Original post on bio 851 at 2013-03-11 12:42:25 UTC *** (In reply to comment #10) > Comment on attachment 8354023 [details] [diff] [review] (bio-attmnt 2258) [details] > Is "Start following tweets from a user" really better than "Start following a > user"? > > If the number of usernames can be arbitrary, shouldn't we include [ ...] inside > the help string? How do we do it in other similar cases? It looks like we typically put a * after the optional part http://lxr.instantbird.org/instantbird/source/chat/locales/en-US/irc.properties#59 > Should we write "a user / users" in the help string? Probably. > >+ commands: [ > >+ { > >+ name: "follow", > >+ get helpString() _("command.follow", "follow"), > > Is there a reason why we need a formatted string here? Seems to be just for > consistency with what we do elsewhere, but do you remember why we do it > elsewhere? So translators do not translate the command name. The other comments I agree with and will provide a new patch for.
Whiteboard: [checkin-needed]
Comment 13•10 years ago
|
||
*** Original post on bio 851 at 2013-03-11 12:47:44 UTC *** (In reply to comment #11) > > Is there a reason why we need a formatted string here? Seems to be just for > > consistency with what we do elsewhere, but do you remember why we do it > > elsewhere? > So translators do not translate the command name. Makes sense, thanks for the reminder! I wonder if we should explain that in a code comment somewhere.
Assignee | ||
Comment 14•10 years ago
|
||
*** Original post on bio 851 as attmnt 2269 at 2013-03-13 01:50:00 UTC *** (In reply to comment #12) > (In reply to comment #11) > > > > Is there a reason why we need a formatted string here? Seems to be just for > > > consistency with what we do elsewhere, but do you remember why we do it > > > elsewhere? > > So translators do not translate the command name. > > Makes sense, thanks for the reminder! I wonder if we should explain that in a > code comment somewhere. Sure, where would we put that comment?
Attachment #8354034 -
Flags: review?(aleth)
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8354023 [details] [diff] [review] Patch v2 *** Original change on bio 851 attmnt 2258 at 2013-03-13 01:50:11 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354023 -
Attachment is obsolete: true
Comment 16•10 years ago
|
||
*** Original post on bio 851 at 2013-03-13 08:41:46 UTC *** (In reply to comment #13) > > Makes sense, thanks for the reminder! I wonder if we should explain that in a > > code comment somewhere. > > Sure, where would we put that comment? Maybe just before the |var commands = [| line of ircCommands.jsm, and before |commands: [| for Twitter?
Assignee | ||
Comment 17•10 years ago
|
||
*** Original post on bio 851 as attmnt 2274 at 2013-03-13 22:49:00 UTC *** With updated comments for Twitter and IRC.
Attachment #8354039 -
Flags: review?(florian)
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8354034 [details] [diff] [review] Patch v3 *** Original change on bio 851 attmnt 2269 at 2013-03-13 22:49:35 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354034 -
Attachment is obsolete: true
Attachment #8354034 -
Flags: review?(aleth)
Comment 19•10 years ago
|
||
Comment on attachment 8354039 [details] [diff] [review] Patch v4 *** Original change on bio 851 attmnt 2274 at 2013-03-14 12:08:14 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354039 -
Flags: review?(florian) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 20•10 years ago
|
||
Comment on attachment 8354039 [details] [diff] [review] Patch v4 *** Original change on bio 851 attmnt 2274 at 2013-03-15 21:44:16 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354039 -
Flags: review+
Assignee | ||
Comment 21•10 years ago
|
||
*** Original post on bio 851 at 2013-03-16 12:47:49 UTC *** Committed as http://hg.instantbird.org/instantbird/rev/f28f9f841f5d Thanks.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.4
You need to log in
before you can comment on or make changes to this bug.
Description
•