From ecb191008482987a4b776ff657ed493c14181872 Mon Sep 17 00:00:00 2001
From: Olivier Mangin <olivier@mangin.com>
Date: Fri, 15 Jan 2016 17:11:36 -0500
Subject: [PATCH] Better error message on UnicodeDecodeError for reading text
 file.

Also renames read_file to read_text_file which is what is implemented.
Fixes #51.
---
 pubs/commands/import_cmd.py |  4 ++--
 pubs/content.py             | 28 ++++++++++++++++++++++------
 pubs/filebroker.py          |  6 +++---
 tests/test_filebroker.py    |  4 ++--
 4 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/pubs/commands/import_cmd.py b/pubs/commands/import_cmd.py
index 9592332..7450f6e 100644
--- a/pubs/commands/import_cmd.py
+++ b/pubs/commands/import_cmd.py
@@ -8,7 +8,7 @@ from .. import color
 from ..paper import Paper
 
 from ..uis import get_ui
-from ..content import system_path, read_file
+from ..content import system_path, read_text_file
 
 
 def parser(subparsers):
@@ -45,7 +45,7 @@ def many_from_path(bibpath):
 
     biblist = []
     for filepath in all_files:
-        biblist.append(coder.decode_bibdata(read_file(filepath)))
+        biblist.append(coder.decode_bibdata(read_text_file(filepath)))
 
     papers = {}
     for b in biblist:
diff --git a/pubs/content.py b/pubs/content.py
index f280a58..18982c9 100644
--- a/pubs/content.py
+++ b/pubs/content.py
@@ -14,6 +14,18 @@ from .p3 import urlparse, HTTPConnection, urlopen
       be prefixed by 'byte_'
 """
 
+
+class UnableToDecodeTextFile(Exception):
+
+    _msg = "unknown encoding (maybe not a text file) for: {}"
+
+    def __init__(self, path):
+        self.path = path
+
+    def __str__(self):
+        return self._msg.format(self.path)
+
+
 # files i/o
 
 def _check_system_path_exists(path, fail=True):
@@ -56,10 +68,14 @@ def check_directory(path, fail=True):
             and _check_system_path_is(u'isdir', syspath, fail=fail))
 
 
-def read_file(filepath):
+def read_text_file(filepath):
     check_file(filepath)
-    with _open(filepath, 'r') as f:
-        content = f.read()
+    try:
+        with _open(filepath, 'r') as f:
+            content = f.read()
+    except UnicodeDecodeError:
+        raise UnableToDecodeTextFile(filepath)
+        # Should "raise from". TODO once python 2 is droped.
     return content
 
 
@@ -120,7 +136,7 @@ def get_content(path, ui=None):
     if content_type(path) == u'url':
         return _get_byte_url_content(path, ui=ui).decode(encoding='utf-8')
     else:
-        return read_file(path)
+        return read_text_file(path)
 
 
 def move_content(source, target, overwrite=False):
@@ -155,7 +171,7 @@ def editor_input(editor, initial=u'', suffix='.tmp'):
     cmd = shlex.split(editor)  # this enable editor command with option, e.g. gvim -f
     cmd.append(tfile_name)
     subprocess.call(cmd)
-    content = read_file(tfile_name)
+    content = read_text_file(tfile_name)
     os.remove(tfile_name)
     return content
 
@@ -163,7 +179,7 @@ def editor_input(editor, initial=u'', suffix='.tmp'):
 def edit_file(editor, path_to_file, temporary=True):
     if temporary:
         check_file(path_to_file, fail=True)
-        content = read_file(path_to_file)
+        content = read_text_file(path_to_file)
         content = editor_input(editor, content)
         write_file(path_to_file, content)
     else:
diff --git a/pubs/filebroker.py b/pubs/filebroker.py
index 53bdb75..1d59fbb 100644
--- a/pubs/filebroker.py
+++ b/pubs/filebroker.py
@@ -2,7 +2,7 @@ import os
 import re
 from .p3 import urlparse
 
-from .content import (check_file, check_directory, read_file, write_file,
+from .content import (check_file, check_directory, read_text_file, write_file,
                       system_path, check_content, content_type, get_content,
                       copy_content)
 
@@ -43,11 +43,11 @@ class FileBroker(object):
 
     def pull_metafile(self, citekey):
         filepath = os.path.join(self.metadir, citekey + '.yaml')
-        return read_file(filepath)
+        return read_text_file(filepath)
 
     def pull_bibfile(self, citekey):
         filepath = os.path.join(self.bibdir, citekey + '.bib')
-        return read_file(filepath)
+        return read_text_file(filepath)
 
     def push_metafile(self, citekey, metadata):
         """Put content to disk. Will gladly override anything standing in its way."""
diff --git a/tests/test_filebroker.py b/tests/test_filebroker.py
index 67fa48e..26846d4 100644
--- a/tests/test_filebroker.py
+++ b/tests/test_filebroker.py
@@ -29,10 +29,10 @@ class TestFileBroker(fake_env.TestFakeFs):
         fake_env.copy_dir(self.fs, os.path.join(os.path.dirname(__file__), 'testrepo'), 'testrepo')
         fb = filebroker.FileBroker('testrepo', create = True)
 
-        bib_content = content.read_file('testrepo/bib/Page99.bib')
+        bib_content = content.read_text_file('testrepo/bib/Page99.bib')
         self.assertEqual(fb.pull_bibfile('Page99'), bib_content)
 
-        meta_content = content.read_file('testrepo/meta/Page99.yaml')
+        meta_content = content.read_text_file('testrepo/meta/Page99.yaml')
         self.assertEqual(fb.pull_metafile('Page99'), meta_content)
 
     def test_errors(self):