Make all from* functions return False if patch parsing is not successful.

Only one argument for not doing this with exceptions:
 - checking if a file is a valid patch should be a one-liner

 >>> patch.fromfile('not-a-patch')
 False

Author: techtonik

patch.py

@@ -109,26 +109,38 @@ def xstrip(filename):
 # Main API functions
 
 def fromfile(filename):
-  """ Parse patch file and return PatchSet() object
-      XXX error reporting
+  """ Parse patch file. If successful, returns
+      PatchSet() object. Otherwise returns False.
   """
+  patchset = PatchSet()
   debug("reading %s" % filename)
   fp = open(filename, "rb")
-  patchset = PatchSet(fp)
+  res = patchset.parse(fp)
   fp.close()
-  return patchset
+  if res == True:
+    return patchset
+  return False
 
 
 def fromstring(s):
-  """ Parse text string and return PatchSet() object
+  """ Parse text string and return PatchSet()
+      object (or False if parsing fails)
   """
-  return PatchSet( StringIO(s) )
+  ps = PatchSet( StringIO(s) )
+  if ps.errors == 0:
+    return ps
+  return False
 
 
 def fromurl(url):
-  """ Read patch from URL
+  """ Parse patch from an URL, return False
+      if an error occured. Note that this also
+      can throw urlopen() exceptions.
   """
-  return PatchSet( urllib2.urlopen(url) )
+  ps = PatchSet( urllib2.urlopen(url) )
+  if ps.errors == 0:
+    return ps
+  return False
 
 
 # --- Utility functions ---

tests/run_tests.py

@@ -37,8 +37,10 @@
   verbose = True
 
 
-#: full path for directory with tests
+# full path for directory with tests
 tests_dir = dirname(abspath(__file__))
+def testfile(name):
+  return join(tests_dir, 'data', name)
 
 
 # import patch.py from parent directory
@@ -226,6 +228,13 @@ def test_fromstring(self):
         pst = patch.fromstring(readstr)
         self.assertEqual(len(pst), 5)
 
+    def test_fromfile(self):
+        pst = patch.fromfile(join(tests_dir, "01uni_multi.patch"))
+        self.assertNotEqual(pst, False)
+        self.assertEqual(len(pst), 5)
+        ps2 = patch.fromfile(testfile("failing/not-a-patch.log"))
+        self.assertFalse(ps2)
+
     def test_no_header_for_plain_diff_with_single_file(self):
         pto = patch.fromfile(join(tests_dir, "03trail_fname.patch"))
         self.assertEqual(pto.items[0].header, [])