Merge lp:~dholbach/help-app/1433525 into lp:help-app
- 1433525
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Daniel Holbach |
Approved revision: | 141 |
Merged at revision: | 135 |
Proposed branch: | lp:~dholbach/help-app/1433525 |
Merge into: | lp:help-app |
Diff against target: |
371 lines (+195/-59) 8 files modified
HACKING (+1/-1) debian/control (+2/-0) internals/local/external-links.py (+26/-0) internals/pelicanconf.py (+1/-1) internals/tests/test_links.py (+150/-48) internals/translations/build.py (+1/-1) internals/translations/utils.py (+14/-1) static/themes/app/static/css/app.css (+0/-7) |
To merge this branch: | bzr merge lp:~dholbach/help-app/1433525 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Nicholas Skaggs (community) | Approve | ||
Ubuntu Phone Apps Jenkins Bot | continuous-integration | Approve | |
Review via email:
|
Commit message
Add markdown extension to make external links use target="_blank".
Description of the change
Add markdown extension to make external links use target="_blank".
- 137. By Daniel Holbach
-
simplify python2/3 urlparse code

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
- 138. By Daniel Holbach
-
simplify code in link checking somewhat, this will allow us to add more tests to check links, add more atomic tests test_simple_
local_broken_ link, test_simple_ local_working_ link, test_non_ existing_ build

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:138
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 139. By Daniel Holbach
-
fix tests run during package build, by fixing override of build location (temporary dir to avoid changing live data)

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:139
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 140. By Daniel Holbach
-
extend and generalise MyHTMLParser and HTMLLinkChecker so we can run other checks on links as well - added a test to see if the markdown extension for external links was run

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:140
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 141. By Daniel Holbach
-
fix links in the CSS

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:141
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://

Nicholas Skaggs (nskaggs) wrote : | # |
LGTM. Couldn't check on device; will do before release.
Preview Diff
1 | === modified file 'HACKING' | |||
2 | --- HACKING 2015-03-31 12:27:56 +0000 | |||
3 | +++ HACKING 2015-05-13 13:46:52 +0000 | |||
4 | @@ -86,7 +86,7 @@ | |||
5 | 86 | 86 | ||
6 | 87 | sudo apt install python-pelican po4a make bzrtools \ | 87 | sudo apt install python-pelican po4a make bzrtools \ |
7 | 88 | ubuntu-html5-ui-toolkit python3-polib python3-magic \ | 88 | ubuntu-html5-ui-toolkit python3-polib python3-magic \ |
9 | 89 | python3-markdown pep8 pyflakes | 89 | python3-markdown pep8 pyflakes python-magic python-polib |
10 | 90 | 90 | ||
11 | 91 | This will install the necessary files, so you can build the app or web build | 91 | This will install the necessary files, so you can build the app or web build |
12 | 92 | locally and check if your changes actually make sense and look and work well. | 92 | locally and check if your changes actually make sense and look and work well. |
13 | 93 | 93 | ||
14 | === modified file 'debian/control' | |||
15 | --- debian/control 2015-03-25 10:14:27 +0000 | |||
16 | +++ debian/control 2015-05-13 13:46:52 +0000 | |||
17 | @@ -8,6 +8,8 @@ | |||
18 | 8 | po4a, | 8 | po4a, |
19 | 9 | pyflakes, | 9 | pyflakes, |
20 | 10 | python-pelican (>= 3.5.0~), | 10 | python-pelican (>= 3.5.0~), |
21 | 11 | python-magic, | ||
22 | 12 | python-polib, | ||
23 | 11 | python3-magic, | 13 | python3-magic, |
24 | 12 | python3-markdown, | 14 | python3-markdown, |
25 | 13 | python3-polib, | 15 | python3-polib, |
26 | 14 | 16 | ||
27 | === added file 'internals/local/external-links.py' | |||
28 | --- internals/local/external-links.py 1970-01-01 00:00:00 +0000 | |||
29 | +++ internals/local/external-links.py 2015-05-13 13:46:52 +0000 | |||
30 | @@ -0,0 +1,26 @@ | |||
31 | 1 | from __future__ import absolute_import | ||
32 | 2 | from __future__ import unicode_literals | ||
33 | 3 | from markdown import Extension | ||
34 | 4 | from markdown.treeprocessors import Treeprocessor | ||
35 | 5 | |||
36 | 6 | from translations.utils import link_is_local | ||
37 | 7 | |||
38 | 8 | |||
39 | 9 | class ExternalLinksProcessor(Treeprocessor): | ||
40 | 10 | def run(self, doc): | ||
41 | 11 | for elem in doc.iter('a'): | ||
42 | 12 | if 'href' in elem.attrib and \ | ||
43 | 13 | not link_is_local(elem.get('href')): | ||
44 | 14 | elem.set('target', '_blank') | ||
45 | 15 | |||
46 | 16 | |||
47 | 17 | class ExternalLinksExtension(Extension): | ||
48 | 18 | def extendMarkdown(self, md, md_globals): | ||
49 | 19 | md.treeprocessors.add('external-links', | ||
50 | 20 | ExternalLinksProcessor(md.parser), | ||
51 | 21 | '_end') | ||
52 | 22 | md.registerExtension(self) | ||
53 | 23 | |||
54 | 24 | |||
55 | 25 | def makeExtension(*args, **kwargs): | ||
56 | 26 | return ExternalLinksExtension(*args, **kwargs) | ||
57 | 0 | 27 | ||
58 | === modified file 'internals/pelicanconf.py' | |||
59 | --- internals/pelicanconf.py 2015-03-31 08:18:07 +0000 | |||
60 | +++ internals/pelicanconf.py 2015-05-13 13:46:52 +0000 | |||
61 | @@ -54,7 +54,7 @@ | |||
62 | 54 | TAG_SAVE_AS = '' | 54 | TAG_SAVE_AS = '' |
63 | 55 | THEME = TOP_LEVEL_DIR+'static/themes/app/' | 55 | THEME = TOP_LEVEL_DIR+'static/themes/app/' |
64 | 56 | 56 | ||
66 | 57 | MD_EXTENSIONS = ['local.q-and-a', 'attr_list', 'toc'] | 57 | MD_EXTENSIONS = ['local.q-and-a', 'local.external-links', 'attr_list', 'toc'] |
67 | 58 | 58 | ||
68 | 59 | META_TAGS = [ | 59 | META_TAGS = [ |
69 | 60 | '[TOC]', | 60 | '[TOC]', |
70 | 61 | 61 | ||
71 | === modified file 'internals/tests/test_links.py' | |||
72 | --- internals/tests/test_links.py 2015-03-31 06:25:44 +0000 | |||
73 | +++ internals/tests/test_links.py 2015-05-13 13:46:52 +0000 | |||
74 | @@ -5,22 +5,12 @@ | |||
75 | 5 | import subprocess | 5 | import subprocess |
76 | 6 | import tempfile | 6 | import tempfile |
77 | 7 | from unittest import TestCase | 7 | from unittest import TestCase |
94 | 8 | import urllib.parse | 8 | |
95 | 9 | 9 | from translations.utils import ( | |
96 | 10 | from translations.utils import use_top_level_dir | 10 | link_is_anchor, |
97 | 11 | 11 | link_is_local, | |
98 | 12 | 12 | use_top_level_dir, | |
99 | 13 | def require_build(build): | 13 | ) |
84 | 14 | tempdir = tempfile.mkdtemp() | ||
85 | 15 | env = {} | ||
86 | 16 | if build == 'app': | ||
87 | 17 | env = {'OUTPUTDIR_APP': tempdir} | ||
88 | 18 | if build == 'web': | ||
89 | 19 | env = {'OUTPUTDIR_WEB': tempdir} | ||
90 | 20 | pwd = use_top_level_dir() | ||
91 | 21 | ret = subprocess.call(['make', '-es', build], env=env) | ||
92 | 22 | os.chdir(pwd) | ||
93 | 23 | return (ret, tempdir) | ||
100 | 24 | 14 | ||
101 | 25 | 15 | ||
102 | 26 | def clean_tempdir(tempdir): | 16 | def clean_tempdir(tempdir): |
103 | @@ -28,53 +18,165 @@ | |||
104 | 28 | shutil.rmtree(tempdir) | 18 | shutil.rmtree(tempdir) |
105 | 29 | 19 | ||
106 | 30 | 20 | ||
107 | 21 | class BuildRunner(): | ||
108 | 22 | def __init__(self, build): | ||
109 | 23 | self.tempdir = tempfile.mkdtemp() | ||
110 | 24 | self.env = {} | ||
111 | 25 | self.build = build | ||
112 | 26 | self.html_files = [] | ||
113 | 27 | self.rc = None | ||
114 | 28 | if self.build == 'app': | ||
115 | 29 | self.env = {'APP_DIR': self.tempdir} | ||
116 | 30 | if self.build == 'web': | ||
117 | 31 | self.env = {'OUTPUTDIR_WEB': self.tempdir} | ||
118 | 32 | self.pwd = use_top_level_dir() | ||
119 | 33 | self.run_build() | ||
120 | 34 | self.find_html_files() | ||
121 | 35 | |||
122 | 36 | def run_build(self): | ||
123 | 37 | self.rc = subprocess.call(['make', '-es', self.build], env=self.env) | ||
124 | 38 | os.chdir(self.pwd) | ||
125 | 39 | |||
126 | 40 | def find_html_files(self): | ||
127 | 41 | for dirpath, dirnames, filenames in os.walk(self.tempdir): | ||
128 | 42 | self.html_files.extend([os.path.join(dirpath, fn) | ||
129 | 43 | for fn in filenames | ||
130 | 44 | if fn.endswith('.html')]) | ||
131 | 45 | |||
132 | 46 | def __del__(self): | ||
133 | 47 | os.chdir(self.pwd) | ||
134 | 48 | clean_tempdir(self.tempdir) | ||
135 | 49 | |||
136 | 50 | |||
137 | 31 | class MyHTMLParser(HTMLParser): | 51 | class MyHTMLParser(HTMLParser): |
138 | 32 | links = [] | 52 | links = [] |
139 | 33 | 53 | ||
140 | 34 | def handle_starttag(self, tag, attrs): | 54 | def handle_starttag(self, tag, attrs): |
142 | 35 | if tag == "a": | 55 | if tag == 'a': |
143 | 56 | found = {} | ||
144 | 36 | for name, value in attrs: | 57 | for name, value in attrs: |
150 | 37 | if name == "href": | 58 | if name == 'href' and \ |
151 | 38 | url = urllib.parse.urlparse(value) | 59 | not link_is_anchor(value) and \ |
152 | 39 | if not value.startswith('#') and \ | 60 | value != '/': |
153 | 40 | url.netloc in [None, '', 'localhost']: | 61 | found['href'] = value |
154 | 41 | self.links += [value] | 62 | if name == 'target': |
155 | 63 | found['target'] = value | ||
156 | 64 | if found != {}: | ||
157 | 65 | self.links += [found] | ||
158 | 42 | 66 | ||
161 | 43 | def reload(self): | 67 | def feed(self, data): |
160 | 44 | links = self.links | ||
162 | 45 | self.links = [] | 68 | self.links = [] |
164 | 46 | return links | 69 | super(MyHTMLParser, self).feed(data) |
165 | 70 | |||
166 | 71 | |||
167 | 72 | class HTMLLinkChecker(): | ||
168 | 73 | def __init__(self, html_files): | ||
169 | 74 | self.local_link_results = [] | ||
170 | 75 | self.htmlparser = MyHTMLParser() | ||
171 | 76 | self.links = {} | ||
172 | 77 | for fn in html_files: | ||
173 | 78 | html = codecs.open(fn, encoding='utf-8').read() | ||
174 | 79 | self.links[fn] = self._read_links(html) | ||
175 | 80 | |||
176 | 81 | def _read_links(self, html): | ||
177 | 82 | self.htmlparser.feed(html) | ||
178 | 83 | return self.htmlparser.links | ||
179 | 84 | |||
180 | 85 | def check_local_links(self): | ||
181 | 86 | self.local_link_results = [] | ||
182 | 87 | pwd = os.getcwd() | ||
183 | 88 | for filename in self.links: | ||
184 | 89 | for link in [a['href'] for a | ||
185 | 90 | in self.links[filename] | ||
186 | 91 | if link_is_local(a['href'])]: | ||
187 | 92 | os.chdir(os.path.dirname(filename)) | ||
188 | 93 | full_link_fn = os.path.join(os.path.dirname(filename), link) | ||
189 | 94 | rel_path = os.path.relpath(full_link_fn, filename) | ||
190 | 95 | path = os.path.normpath(os.path.join(filename, rel_path)) | ||
191 | 96 | self.local_link_results += [os.path.exists(path)] | ||
192 | 97 | os.chdir(pwd) | ||
193 | 98 | |||
194 | 99 | def check_external_links(self): | ||
195 | 100 | self.external_link_results = [] | ||
196 | 101 | for filename in self.links: | ||
197 | 102 | for link in [a for a | ||
198 | 103 | in self.links[filename] | ||
199 | 104 | if not link_is_local(a['href'])]: | ||
200 | 105 | self.external_link_results += [('target' in link and | ||
201 | 106 | link['target'] == '_blank')] | ||
202 | 47 | 107 | ||
203 | 48 | 108 | ||
204 | 49 | class HelpTestCase(TestCase): | 109 | class HelpTestCase(TestCase): |
205 | 50 | def __init__(self, *args): | 110 | def __init__(self, *args): |
206 | 51 | self.pwd = os.getcwd() | 111 | self.pwd = os.getcwd() |
208 | 52 | self.htmlparser = MyHTMLParser() | 112 | self.build_runner = None |
209 | 53 | TestCase.__init__(self, *args) | 113 | TestCase.__init__(self, *args) |
210 | 114 | self.html_link_checker = None | ||
211 | 54 | 115 | ||
212 | 55 | def __del__(self): | 116 | def __del__(self): |
213 | 56 | os.chdir(self.pwd) | 117 | os.chdir(self.pwd) |
214 | 57 | 118 | ||
215 | 119 | def _require_build(self, build): | ||
216 | 120 | if self.build_runner is None or \ | ||
217 | 121 | self.build_runner.build != build: | ||
218 | 122 | self.build_runner = BuildRunner(build) | ||
219 | 123 | |||
220 | 58 | def _test_local_links(self, build): | 124 | def _test_local_links(self, build): |
243 | 59 | (ret, tempdir) = require_build(build) | 125 | self._require_build(build) |
244 | 60 | self.assertEqual(ret, 0) | 126 | self.html_link_checker = HTMLLinkChecker(self.build_runner.html_files) |
245 | 61 | for dirpath, dirnames, filenames in os.walk(tempdir): | 127 | self.html_link_checker.check_local_links() |
246 | 62 | for fn in filenames: | 128 | |
247 | 63 | full_fn = os.path.join(dirpath, fn) | 129 | def _test_external_links(self, build): |
248 | 64 | os.chdir(dirpath) | 130 | self._require_build(build) |
249 | 65 | if full_fn.endswith('.html'): | 131 | self.html_link_checker = HTMLLinkChecker(self.build_runner.html_files) |
250 | 66 | html = codecs.open(full_fn, encoding='utf-8').read() | 132 | self.html_link_checker.check_external_links() |
251 | 67 | self.htmlparser.feed(html) | 133 | |
252 | 68 | links = self.htmlparser.reload() | 134 | def test_simple_local_broken_link(self): |
253 | 69 | for link in links: | 135 | dirname = tempfile.mkdtemp() |
254 | 70 | rel_path = os.path.relpath(link, full_fn) | 136 | filename = os.path.join(dirname, 'first-test.html') |
255 | 71 | path = os.path.normpath(os.path.join(full_fn, rel_path)) | 137 | with open(filename, 'w') as f: |
256 | 72 | self.assertTrue(os.path.exists(path)) | 138 | f.write('<html><body>' + |
257 | 73 | clean_tempdir(tempdir) | 139 | '<a href="nonexisting-test.html">broken test link</a>' + |
258 | 74 | return True | 140 | '</body><html>') |
259 | 75 | 141 | self.html_link_checker = HTMLLinkChecker([filename]) | |
260 | 76 | def test_local_phone_links(self): | 142 | self.html_link_checker.check_local_links() |
261 | 77 | return self._test_local_links('html') | 143 | self.assertNotIn(True, self.html_link_checker.local_link_results) |
262 | 78 | 144 | clean_tempdir(dirname) | |
263 | 79 | def test_local_web_links(self): | 145 | |
264 | 80 | return self._test_local_links('web') | 146 | def test_simple_local_working_link(self): |
265 | 147 | html = '<html><body>' + \ | ||
266 | 148 | '<a href="second-test.html">broken test link</a>' + \ | ||
267 | 149 | '</body><html>' | ||
268 | 150 | dirname = tempfile.mkdtemp() | ||
269 | 151 | filename1 = os.path.join(dirname, 'first-test.html') | ||
270 | 152 | with open(filename1, 'w') as f: | ||
271 | 153 | f.write(html) | ||
272 | 154 | filename2 = os.path.join(dirname, 'second-test.html') | ||
273 | 155 | with open(filename2, 'w') as f: | ||
274 | 156 | f.write(html) | ||
275 | 157 | self.html_link_checker = HTMLLinkChecker([filename1]) | ||
276 | 158 | self.html_link_checker.check_local_links() | ||
277 | 159 | self.assertEqual([True], self.html_link_checker.local_link_results) | ||
278 | 160 | clean_tempdir(dirname) | ||
279 | 161 | |||
280 | 162 | def test_non_existing_build(self): | ||
281 | 163 | self._require_build('not-existing') | ||
282 | 164 | self.assertNotEqual(self.build_runner.rc, 0) | ||
283 | 165 | |||
284 | 166 | def test_local_links_in_phone_build(self): | ||
285 | 167 | self._test_local_links('app') | ||
286 | 168 | self.assertEqual(self.build_runner.rc, 0) | ||
287 | 169 | self.assertNotEqual(self.build_runner.html_files, []) | ||
288 | 170 | self.assertNotIn(False, self.html_link_checker.local_link_results) | ||
289 | 171 | |||
290 | 172 | def test_external_links_in_phone_build(self): | ||
291 | 173 | self._test_external_links('app') | ||
292 | 174 | self.assertEqual(self.build_runner.rc, 0) | ||
293 | 175 | self.assertNotEqual(self.build_runner.html_files, []) | ||
294 | 176 | self.assertNotIn(False, self.html_link_checker.external_link_results) | ||
295 | 177 | |||
296 | 178 | def test_local_links_in_web_build(self): | ||
297 | 179 | self._test_local_links('web') | ||
298 | 180 | self.assertEqual(self.build_runner.rc, 0) | ||
299 | 181 | self.assertNotEqual(self.build_runner.html_files, []) | ||
300 | 182 | self.assertNotIn(False, self.html_link_checker.local_link_results) | ||
301 | 81 | 183 | ||
302 | === modified file 'internals/translations/build.py' | |||
303 | --- internals/translations/build.py 2015-03-31 11:38:30 +0000 | |||
304 | +++ internals/translations/build.py 2015-05-13 13:46:52 +0000 | |||
305 | @@ -20,7 +20,7 @@ | |||
306 | 20 | try: | 20 | try: |
307 | 21 | import polib | 21 | import polib |
308 | 22 | except ImportError: | 22 | except ImportError: |
310 | 23 | require('python3-polib') | 23 | require('python3-polib pyton-polib') |
311 | 24 | 24 | ||
312 | 25 | from pelicanconf import ( | 25 | from pelicanconf import ( |
313 | 26 | DOCS_PATH, | 26 | DOCS_PATH, |
314 | 27 | 27 | ||
315 | === modified file 'internals/translations/utils.py' | |||
316 | --- internals/translations/utils.py 2015-03-31 07:57:56 +0000 | |||
317 | +++ internals/translations/utils.py 2015-05-13 13:46:52 +0000 | |||
318 | @@ -1,6 +1,10 @@ | |||
319 | 1 | import os | 1 | import os |
320 | 2 | import sys | 2 | import sys |
321 | 3 | import tempfile | 3 | import tempfile |
322 | 4 | if sys.version_info.major == 2: | ||
323 | 5 | import urlparse | ||
324 | 6 | else: | ||
325 | 7 | import urllib.parse as urlparse | ||
326 | 4 | 8 | ||
327 | 5 | 9 | ||
328 | 6 | def require(package): | 10 | def require(package): |
329 | @@ -11,7 +15,7 @@ | |||
330 | 11 | try: | 15 | try: |
331 | 12 | import magic | 16 | import magic |
332 | 13 | except: | 17 | except: |
334 | 14 | require('python3-magic') | 18 | require('python3-magic python-magic') |
335 | 15 | 19 | ||
336 | 16 | try: | 20 | try: |
337 | 17 | import markdown | 21 | import markdown |
338 | @@ -37,6 +41,15 @@ | |||
339 | 37 | ] | 41 | ] |
340 | 38 | 42 | ||
341 | 39 | 43 | ||
342 | 44 | def link_is_anchor(link): | ||
343 | 45 | return link.startswith('#') | ||
344 | 46 | |||
345 | 47 | |||
346 | 48 | def link_is_local(link): | ||
347 | 49 | url = urlparse.urlparse(link) | ||
348 | 50 | return url.netloc in [None, '', 'localhost'] | ||
349 | 51 | |||
350 | 52 | |||
351 | 40 | def normalise_path(path): | 53 | def normalise_path(path): |
352 | 41 | return os.path.relpath(path, TOP_LEVEL_DIR) | 54 | return os.path.relpath(path, TOP_LEVEL_DIR) |
353 | 42 | 55 | ||
354 | 43 | 56 | ||
355 | === modified file 'static/themes/app/static/css/app.css' | |||
356 | --- static/themes/app/static/css/app.css 2015-03-18 12:06:41 +0000 | |||
357 | +++ static/themes/app/static/css/app.css 2015-05-13 13:46:52 +0000 | |||
358 | @@ -47,13 +47,6 @@ | |||
359 | 47 | display: none; | 47 | display: none; |
360 | 48 | } | 48 | } |
361 | 49 | 49 | ||
362 | 50 | /* Disable links until they work | ||
363 | 51 | properly */ | ||
364 | 52 | a { | ||
365 | 53 | pointer-events: none; | ||
366 | 54 | cursor: default; | ||
367 | 55 | } | ||
368 | 56 | |||
369 | 57 | /* Disable this when the app container | 50 | /* Disable this when the app container |
370 | 58 | properly supports tab scrolling. | 51 | properly supports tab scrolling. |
371 | 59 | We hardcode support for showing | 52 | We hardcode support for showing |
PASSED: Continuous integration, rev:137 91.189. 93.70:8080/ job/help- app-ci/ 26/ 91.189. 93.70:8080/ job/help- app-utopic- amd64-ci/ 26 91.189. 93.70:8080/ job/help- app-vivid- amd64-ci/ 28
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: 91.189. 93.70:8080/ job/help- app-ci/ 26/rebuild
http://