Merge lp:~saviq/unity-lens-video/remote-videos-scope-plurals into lp:~jhodapp/unity-lens-video/remote-videos-scope-trunk

Proposed by Michał Sawicz
Status: Merged
Approved by: Jim Hodapp
Approved revision: 65
Merged at revision: 59
Proposed branch: lp:~saviq/unity-lens-video/remote-videos-scope-plurals
Merge into: lp:~jhodapp/unity-lens-video/remote-videos-scope-trunk
Diff against target: 112 lines (+24/-35)
1 file modified
src/unity-scope-video-remote (+24/-35)
To merge this branch: bzr merge lp:~saviq/unity-lens-video/remote-videos-scope-plurals
Reviewer Review Type Date Requested Status
Jim Hodapp code Approve
Review via email: mp+121609@code.launchpad.net
To post a comment you must log in.
59. By Michał Sawicz

Use ngettext and str.join in details retrieval

Use ngettext in multivalued details for plural handling
Use str.join to join multivalued details

60. By Michał Sawicz

Don't differentiate between sources

Display all data in the preview, if available, regardless of the source.

61. By Michał Sawicz

Don't use list.get when unnecessary

62. By Michał Sawicz

Only import datetime from datetime

63. By Michał Sawicz

Another dict.get vs dict[]

64. By Michał Sawicz

And another one

Revision history for this message
Jim Hodapp (jhodapp) wrote :

This looks good, but revision 60 needs to be undone since we do need to differentiate between sources given that not every video preview looks the same according to the design mockups.

review: Needs Fixing (code)
65. By Michał Sawicz

Revert r60

Revision history for this message
Jim Hodapp (jhodapp) wrote :

Looks good!

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/unity-scope-video-remote'
2--- src/unity-scope-video-remote 2012-08-27 15:56:53 +0000
3+++ src/unity-scope-video-remote 2012-08-28 14:58:23 +0000
4@@ -22,7 +22,7 @@
5 import sys
6 from urllib import urlencode
7 import time
8-import datetime
9+from datetime import datetime
10 import gettext
11
12 #pylint: disable=E0611
13@@ -50,6 +50,7 @@
14 gettext.bindtextdomain(APP_NAME, LOCAL_PATH)
15 gettext.textdomain(APP_NAME)
16 _ = gettext.gettext
17+n_ = gettext.ngettext
18
19 BUS_NAME = "net.launchpad.scope.RemoteVideos"
20 #SERVER = "http://videosearch.ubuntu.com/v0"
21@@ -294,11 +295,11 @@
22 desc = ''
23 source = ''
24 if isinstance(details, dict):
25- uri = details.get('browser_url', '')
26- title = details.get('title', '')
27- thumbnail_uri = details.get('image', '')
28- desc = details.get('description', '')
29- source = details.get('source', '')
30+ uri = details['browser_url']
31+ title = details['title']
32+ thumbnail_uri = details['image']
33+ desc = details['description']
34+ source = details['source']
35
36 try:
37 thumbnail_file = Gio.File.new_for_uri (thumbnail_uri)
38@@ -312,38 +313,38 @@
39
40 # TODO: For details of future source types, factor out common detail key/value pairs
41 if source == "Amazon":
42- directors = self.get_list_values("directors", details)
43- if len(directors) > 0:
44- preview.add_info(Unity.InfoHint.new("directors", _("Directors"),
45- None, directors))
46+ if "directors" in details:
47+ directors = details["directors"]
48+ preview.add_info(Unity.InfoHint.new("directors",
49+ n_("Director", "Directors", len(directors)), None, ', '.join(directors)))
50 if "starring" in details:
51- cast = self.get_list_values("starring", details)
52- if len(cast) > 0:
53- preview.add_info(Unity.InfoHint.new("cast", _("Cast"),
54- None, cast))
55+ cast = details["starring"]
56+ if cast:
57+ preview.add_info(Unity.InfoHint.new("cast",
58+ n_("Cast", "Cast", len(cast)), None, ', '.join(cast)))
59 if "genres" in details:
60- genres = self.get_list_values("genres", details)
61- if len(genres) > 0:
62- preview.add_info(Unity.InfoHint.new("genres", _("Genres"),
63- None, genres))
64+ genres = details["genres"]
65+ if genres:
66+ preview.add_info(Unity.InfoHint.new("genres",
67+ n_("Genre", "Genres", len(genres)), None, ', '.join(genres)))
68 if "release_date" in details:
69 try:
70 # v1 spec states release_date will be YYYY-MM-DD
71- release_date = datetime.datetime.strptime(details.get("release_date", ""), "%Y-%m-%d")
72+ release_date = datetime.strptime(details["release_date"], "%Y-%m-%d")
73 preview.year = release_date.strftime("%Y")
74 except:
75 print "Failed to parse release_date since it was not in format: YYYY-MM-DD"
76 if "duration" in details:
77- duration = int(details.get("duration")) / 60
78- # TODO: Extend VideoPreview class and set property for showing video length
79+ duration = int(details["duration"]) / 60
80+ # TODO: Extend VideoPreview class and set property for showing video length
81 # TODO: Add Vimeo & YouTube details for v1 of JSON API
82 else:
83 if "uploaded_by" in details:
84 preview.add_info(Unity.InfoHint.new("uploaded-by", _("Uploaded by"),
85- None, details.get("uploaded_by", "")))
86+ None, details["uploaded_by"]))
87 if "date_uploaded" in details:
88 preview.add_info(Unity.InfoHint.new("uploaded-on", _("Uploaded on"),
89- None, details.get("date_uploaded", "")))
90+ None, details["date_uploaded"]))
91
92 return preview
93
94@@ -368,18 +369,6 @@
95
96 return details
97
98- def get_list_values(self, key, details):
99- """Given a JSON list (i.e. "starring": ["Robert Downey Jr.", "Terrence Howard"])
100- extract the values and put them into a comma separated string."""
101- values = ""
102- if key in details:
103- for value in details.get(key, ""):
104- values += str(value) + ", "
105- # Remove the final ", "
106- values = values[0:len(values)-2]
107-
108- return values
109-
110 def play_video (self, action, uri):
111 """Preview request - Play action handler"""
112 return self.on_activate_uri (action, uri)

Subscribers

People subscribed via source and target branches

to all changes: