ubuntu-touch-coreapps-reviewers team mailing list archive

[Nicholas Skaggs <nskaggs@xxxxxxxxxx>]

Review: Needs Information

A couple small comments.

Diff comments:

> === added directory 'internals/local'
> === added file 'internals/local/__init__.py'
> === renamed file 'internals/q-and-a.py' => 'internals/local/q-and-a.py'
> === modified file 'internals/pelicanconf.py'
> --- internals/pelicanconf.py	2015-03-19 14:23:27 +0000
> +++ internals/pelicanconf.py	2015-03-24 13:13:58 +0000
> @@ -8,6 +8,7 @@
>  
>  TOP_LEVEL_DIR = '../'
>  PATH = TOP_LEVEL_DIR+'content/'
> +INTERNALS_DIR = TOP_LEVEL_DIR+'internals/'
>  TRANSLATIONS_DIR = TOP_LEVEL_DIR+'po/'
>  
>  TIMEZONE = 'Europe/Paris'
> @@ -46,7 +47,7 @@
>  TAG_SAVE_AS = ''
>  THEME = TOP_LEVEL_DIR+'static/themes/app/'
>  
> -MD_EXTENSIONS = ['q-and-a', 'attr_list', 'toc']
> +MD_EXTENSIONS = ['local.q-and-a', 'attr_list', 'toc']
>  
>  META_TAGS = [
>      '[TOC]',
> 
> === modified file 'internals/tests/test_files.py'
> --- internals/tests/test_files.py	2015-03-19 13:37:00 +0000
> +++ internals/tests/test_files.py	2015-03-24 13:13:58 +0000
> @@ -26,6 +26,6 @@
>  
>      def test_markdown_files(self):
>          fns = self.translations.documents.find_docs()
> -        checked_fns = [utils.verify_markdown_file(fn)
> +        checked_fns = [utils.verify_file_type(fn)
>                         for fn in fns]
>          self.assertEqual(len(fns), checked_fns.count(True))
> 
> === added file 'internals/tests/test_markdown.py'
> --- internals/tests/test_markdown.py	1970-01-01 00:00:00 +0000
> +++ internals/tests/test_markdown.py	2015-03-24 13:13:58 +0000
> @@ -0,0 +1,24 @@
> +from unittest import TestCase
> +
> +from translations.build import Documents
> +from translations import utils
> +from pelicanconf import MD_EXTENSIONS
> +
> +
> +class HelpTestCase(TestCase):
> +    def __init__(self, *args):
> +        TestCase.__init__(self, *args)
> +
> +    def test_load_extensions(self):
> +        md = utils.Markdown()
> +        for ext in MD_EXTENSIONS:
> +            self.assertTrue(ext in md.md.treeprocessors or
> +                            ext in [q.__module__ for q
> +                                    in md.md.registeredExtensions])
> +
> +    def test_convert_all_docs(self):
> +        md = utils.Markdown()
> +        docs = Documents()
> +        self.assertEqual(len(docs.docs),
> +                         len([md.can_convert_md_file(fn) for fn
> +                              in docs.docs]))
> 
> === modified file 'internals/translations/build.py'
> --- internals/translations/build.py	2015-03-20 10:31:12 +0000
> +++ internals/translations/build.py	2015-03-24 13:13:58 +0000
> @@ -9,10 +9,11 @@
>  from translations.utils import (
>      find_bcp47_code,
>      full_path,
> +    Markdown,
>      normalise_path,
>      require,
>      use_top_level_dir,
> -    verify_markdown_file,
> +    verify_file_type,
>  )
>  
>  from translations.po4a import PO4A
> @@ -233,8 +234,9 @@
>  
>  class Documents(object):
>      def __init__(self):
> +        md = Markdown()
>          self.docs = [fn for fn in self.find_docs()
> -                     if verify_markdown_file(fn)]
> +                     if verify_file_type(fn) and md.can_convert_md_file(fn)]
>  
>      def find_docs(self):
>          docs = []
> 
> === modified file 'internals/translations/utils.py'
> --- internals/translations/utils.py	2015-03-19 13:22:42 +0000
> +++ internals/translations/utils.py	2015-03-24 13:13:58 +0000
> @@ -1,4 +1,3 @@
> -import codecs
>  import os
>  import sys
>  import tempfile
> @@ -20,7 +19,9 @@
>      require('python3-markdown')
>  
>  from pelicanconf import (
> +    INTERNALS_DIR,
>      MD_EXTENSIONS,
> +    PATH,
>      TOP_LEVEL_DIR,
>  )
>  
> @@ -44,37 +45,49 @@
>      return os.path.abspath(os.path.join(TOP_LEVEL_DIR, path))
>  
>  
> -def use_top_level_dir():
> +def use_path(path):
>      pwd = os.getcwd()
> -    os.chdir(TOP_LEVEL_DIR)
> +    os.chdir(path)
>      return pwd
>  
>  
> -def _temp_write_markdown(fn):
> -    (ret, tmp) = tempfile.mkstemp()
> -    length = 0
> -    ret = True
> -    try:
> -        markdown.markdownFromFile(fn, extensions=MD_EXTENSIONS, output=tmp)
> -    except:
> -        print('Could not convert "%s" into Markdown.' % fn)
> -        ret = False
> -    if ret:
> -        length = len(codecs.open(tmp, 'r', 'utf-8').read())
> -    os.remove(tmp)
> -    return (ret, length)
> -
> -
> -def verify_markdown_file(fn):
> +def use_top_level_dir():
> +    return use_path(TOP_LEVEL_DIR)
> +
> +
> +def use_docs_path():
> +    return use_path(PATH)
> +
> +
> +def use_internals_dir():
> +    return use_path(INTERNALS_DIR)
> +
> +
> +class Markdown(object):
> +    def __init__(self):
> +        pwd = use_internals_dir()
> +        try:
> +            self.md = markdown.Markdown(extensions=MD_EXTENSIONS)
> +        except ImportError:
> +            os.chdir(pwd)
> +            print('Could not load all markdown extensions.')

does it make sense to raise this as an exception instead?

> +            sys.exit(1)
> +        os.chdir(pwd)
> +
> +    def can_convert_md_file(self, fn):
> +        (ret, tmp) = tempfile.mkstemp()
> +        pwd = use_top_level_dir()
> +        res = self.md.convertFile(fn, tmp, 'utf-8')
> +        os.remove(tmp)

mkstemp should clean up after itself.

> +        os.chdir(pwd)
> +        return len(res.lines) > 0
> +
> +
> +def verify_file_type(fn):
>      ms = magic.open(magic.MAGIC_NONE)
>      ms.load()
>      fn = full_path(fn)
> -    if ms.file(fn) not in MD_MAGIC_FILE_TYPES:
> -        return False
> -    (ret, length) = _temp_write_markdown(fn)
> -    if not length:
> -        return False
> -    return ret
> +    return ms.file(fn) in MD_MAGIC_FILE_TYPES
>  
>  
>  def find_bcp47_code(gettext_code):
> 
> === modified file 'po/help.pot'
> --- po/help.pot	2015-03-24 08:35:50 +0000
> +++ po/help.pot	2015-03-24 13:13:58 +0000
> @@ -7,7 +7,7 @@
>  msgid ""
>  msgstr ""
>  "Project-Id-Version: PACKAGE VERSION\n"
> -"POT-Creation-Date: 2015-03-24 09:35+0100\n"
> +"POT-Creation-Date: 2015-03-24 14:13+0100\n"
>  "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
>  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
>  "Language-Team: LANGUAGE <LL@xxxxxx>\n"
> 


-- 
https://code.launchpad.net/~dholbach/help-app/1434415/+merge/253844
Your team Ubuntu Help app developers is subscribed to branch lp:help-app.