Merge lp://staging/~kai-mast/friends/links into lp://staging/friends
Proposed by
Kai Mast
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 249 | ||||
Proposed branch: | lp://staging/~kai-mast/friends/links | ||||
Merge into: | lp://staging/friends | ||||
Diff against target: |
437 lines (+361/-17) 4 files modified
friends/protocols/twitter.py (+48/-15) friends/tests/data/twitter-hashtags.dat (+94/-0) friends/tests/data/twitter-multiple-links.dat (+144/-0) friends/tests/test_twitter.py (+75/-2) |
||||
To merge this branch: | bzr merge lp://staging/~kai-mast/friends/links | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Robert Bruce Park | Approve | ||
Review via email: mp+202954@code.staging.launchpad.net |
Description of the change
This branch likifies mentions and hashtags. Hashtags open a link with the search in a browser. Friends has support for searches but afaik there is no way to add links to them. So this is the best we can do right now. Imo that makes tweets (especially retweets) much more interakctive.
I also fixed a problem when there are multiple links in a tweet and also added a test so this stays fixed.
To post a comment you must log in.
Wow, congrats on the test coverage, that's really impressive ;-)
One thing that I'm a little bit confused about. You named a variable "urls_sorted" but I don't actually see any sorting going on there. Then you go on to use a concept of 'offset' in order to keep track of where each link should go, and I find it a bit sloppy.
What I'd like to see instead, is just iterate over urls_sorted in a sorted way, with descending values of 'begin' (eg, linkify the last URL first and progress backwards through the string). That way there's never an offset to have to worry about as the change in string length does not affect links that appear prior to the change being made.
Another (small) problem is that you used iter() in a useless way. Friends is python3-only, so dict.items() always returns an iter and your extra call to iter() doesn't do anything.
So instead it should probably look like this:
for key, url in sorted( urls_sorted. items() , reverse=True):
And then...
if content: [message[ :begin] , content, message[end:]])
message = ''.join(
Also, I'm slightly annoyed by the inconsistency between the _linkify_mention function which is nearly identical to other linkification techniques that are inlined in the larger function. I'm sure there can be a general purpose _linkify method that can be used to linkify all kinds of links, in a much more consistent and space-efficient manner (ie, fewer lines of code).