Patchwork [default] util: raise ParseError when parsing dates

login
register
mail settings
Submitter Boris Feld
Date May 25, 2017, 6:21 p.m.
Message ID <34e55919835ad870ba9d.1495736471@FB>
Download mbox | patch
Permalink /patch/20902/
State Accepted
Headers show

Comments

Boris Feld - May 25, 2017, 6:21 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1495641017 -7200
#      Wed May 24 17:50:17 2017 +0200
# Node ID 34e55919835ad870ba9df463c07e500988fd114e
# Parent  b647b923486f38d83b92089eafa9faafaa79785d
# EXP-Topic parsedateparseerror
util: raise ParseError when parsing dates

a7dce526c462 refactored util.parsedate in order to raise ValueError instead of Abort for using with ui.configwith.

It causes several problems, putting arbitrary bytes in ValueError can cause issues with Python 3. Moreover, we added a function to convert ValueError exceptions back to Abort.

A better approach would be to make parsedate raises ParseError, removing the convert function and update configwith to also catch ParseError.

The side-effect is that error message when giving an invalid date in CLI change from:

  abort: invalid date: 'foo bar'

to:

  hg: parse error: invalid date: 'foo bar'

I'm not sure if it's an acceptable change, I found personally the error message more clear but more verbose too.
Yuya Nishihara - May 26, 2017, 2:42 p.m.
On Thu, 25 May 2017 20:21:11 +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1495641017 -7200
> #      Wed May 24 17:50:17 2017 +0200
> # Node ID 34e55919835ad870ba9df463c07e500988fd114e
> # Parent  b647b923486f38d83b92089eafa9faafaa79785d
> # EXP-Topic parsedateparseerror
> util: raise ParseError when parsing dates

Marked as "(BC)" (= behavior change) and queued, thanks. I believe raising
ParseError makes more sense (though ParseError has an unfortunate overridden
meanings.)

I also updated the doc of parsedate().

Patch

diff -r b647b923486f -r 34e55919835a mercurial/ui.py
--- a/mercurial/ui.py	Thu May 25 01:45:52 2017 +0200
+++ b/mercurial/ui.py	Wed May 24 17:50:17 2017 +0200
@@ -522,7 +522,7 @@ 
             return default
         try:
             return convert(v)
-        except ValueError:
+        except (ValueError, error.ParseError):
             if desc is None:
                 desc = convert.__name__
             raise error.ConfigError(_("%s.%s is not a valid %s ('%s')")
@@ -607,7 +607,7 @@ 
         (0, 0)
         """
         if self.config(section, name, default, untrusted):
-            return self.configwith(util.rawparsedate, section, name, default,
+            return self.configwith(util.parsedate, section, name, default,
                                    'date', untrusted)
         return default
 
diff -r b647b923486f -r 34e55919835a mercurial/util.py
--- a/mercurial/util.py	Thu May 25 01:45:52 2017 +0200
+++ b/mercurial/util.py	Wed May 24 17:50:17 2017 +0200
@@ -1941,20 +1941,6 @@ 
     >>> tz == strtz
     True
     """
-    try:
-        return rawparsedate(date, formats=formats, bias=bias)
-    except ValueError as exception:
-        raise Abort(str(exception))
-
-def rawparsedate(date, formats=None, bias=None):
-    """parse a localized date/time and return a (unixtime, offset) tuple.
-
-    The date may be a "unixtime offset" string or in one of the specified
-    formats. If the date already is a (unixtime, offset) tuple, it is returned.
-
-    See docstring of parsedate for example.
-    Raise ValueError for invalid date value.
-    """
     if bias is None:
         bias = {}
     if not date:
@@ -2001,15 +1987,15 @@ 
             else:
                 break
         else:
-            raise ValueError(_('invalid date: %r') % date)
+            raise error.ParseError(_('invalid date: %r') % date)
     # validate explicit (probably user-specified) date and
     # time zone offset. values must fit in signed 32 bits for
     # current 32-bit linux runtimes. timezones go from UTC-12
     # to UTC+14
     if when < -0x80000000 or when > 0x7fffffff:
-        raise ValueError(_('date exceeds 32 bits: %d') % when)
+        raise error.ParseError(_('date exceeds 32 bits: %d') % when)
     if offset < -50400 or offset > 43200:
-        raise ValueError(_('impossible time zone offset: %d') % offset)
+        raise error.ParseError(_('impossible time zone offset: %d') % offset)
     return when, offset
 
 def matchdate(date):
diff -r b647b923486f -r 34e55919835a tests/test-commit.t
--- a/tests/test-commit.t	Thu May 25 01:45:52 2017 +0200
+++ b/tests/test-commit.t	Wed May 24 17:50:17 2017 +0200
@@ -15,20 +15,20 @@ 
   $ hg commit -d '0 0' -m commit-1
   $ echo foo >> foo
   $ hg commit -d '1 4444444' -m commit-3
-  abort: impossible time zone offset: 4444444
+  hg: parse error: impossible time zone offset: 4444444
   [255]
   $ hg commit -d '1	15.1' -m commit-4
-  abort: invalid date: '1\t15.1'
+  hg: parse error: invalid date: '1\t15.1'
   [255]
   $ hg commit -d 'foo bar' -m commit-5
-  abort: invalid date: 'foo bar'
+  hg: parse error: invalid date: 'foo bar'
   [255]
   $ hg commit -d ' 1 4444' -m commit-6
   $ hg commit -d '111111111111 0' -m commit-7
-  abort: date exceeds 32 bits: 111111111111
+  hg: parse error: date exceeds 32 bits: 111111111111
   [255]
   $ hg commit -d '-111111111111 0' -m commit-7
-  abort: date exceeds 32 bits: -111111111111
+  hg: parse error: date exceeds 32 bits: -111111111111
   [255]
   $ echo foo >> foo
   $ hg commit -d '1901-12-13 20:45:52 +0000' -m commit-7-2
@@ -38,10 +38,10 @@ 
   3 1901-12-13 20:45:52 +0000
   2 1901-12-13 20:45:52 +0000
   $ hg commit -d '1901-12-13 20:45:51 +0000' -m commit-7
-  abort: date exceeds 32 bits: -2147483649
+  hg: parse error: date exceeds 32 bits: -2147483649
   [255]
   $ hg commit -d '-2147483649 0' -m commit-7
-  abort: date exceeds 32 bits: -2147483649
+  hg: parse error: date exceeds 32 bits: -2147483649
   [255]
 
 commit added file that has been deleted
diff -r b647b923486f -r 34e55919835a tests/test-glog.t
--- a/tests/test-glog.t	Thu May 25 01:45:52 2017 +0200
+++ b/tests/test-glog.t	Wed May 24 17:50:17 2017 +0200
@@ -1513,7 +1513,7 @@ 
       ('symbol', 'date')
       ('string', '2 0 to 4 0')))
   $ hg log -G -d 'brace ) in a date'
-  abort: invalid date: 'brace ) in a date'
+  hg: parse error: invalid date: 'brace ) in a date'
   [255]
   $ testlog --prune 31 --prune 32
   []
diff -r b647b923486f -r 34e55919835a tests/test-parse-date.t
--- a/tests/test-parse-date.t	Thu May 25 01:45:52 2017 +0200
+++ b/tests/test-parse-date.t	Wed May 24 17:50:17 2017 +0200
@@ -17,13 +17,13 @@ 
   $ hg ci -d "1150000000 14400" -m "rev 4 (merge)"
   $ echo "fail" >> a
   $ hg ci -d "should fail" -m "fail"
-  abort: invalid date: 'should fail'
+  hg: parse error: invalid date: 'should fail'
   [255]
   $ hg ci -d "100000000000000000 1400" -m "fail"
-  abort: date exceeds 32 bits: 100000000000000000
+  hg: parse error: date exceeds 32 bits: 100000000000000000
   [255]
   $ hg ci -d "100000 1400000" -m "fail"
-  abort: impossible time zone offset: 1400000
+  hg: parse error: impossible time zone offset: 1400000
   [255]
 
 Check with local timezone other than GMT and with DST
diff -r b647b923486f -r 34e55919835a tests/test-revset.t
--- a/tests/test-revset.t	Thu May 25 01:45:52 2017 +0200
+++ b/tests/test-revset.t	Wed May 24 17:50:17 2017 +0200
@@ -413,7 +413,7 @@ 
   hg: parse error: invalid \x escape
   [255]
   $ log 'date(tip)'
-  abort: invalid date: 'tip'
+  hg: parse error: invalid date: 'tip'
   [255]
   $ log '0:date'
   abort: unknown revision 'date'!