Patchwork [1,of,8] util: refactor util.parsedate to raises ValueError

login
register
mail settings
Submitter Boris Feld
Date May 22, 2017, 6:46 p.m.
Message ID <ba781657de9f2e5b735e.1495478781@FB>
Download mbox | patch
Permalink /patch/20827/
State Accepted
Headers show

Comments

Boris Feld - May 22, 2017, 6:46 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1495188443 -7200
#      Fri May 19 12:07:23 2017 +0200
# Node ID ba781657de9f2e5b735e8a207cdf0b858047190c
# Parent  c87db79b95079c3fd0032be7a25cd41567ed11cb
# EXP-Topic develforcedate
util: refactor util.parsedate to raises ValueError

Split most of util.parsedate in util.rawparsedate and make it raises ValueError instead
of error.Abort.

The util.parsedate function is now just a shell function converting ValueError
to error.Abort for existing users.

I need to parse a date from config in a later patch and use util.rawparsedate
with ui.configwith which expect a convert that raises ValueError.
Yuya Nishihara - May 23, 2017, 12:39 p.m.
On Mon, 22 May 2017 20:46:21 +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1495188443 -7200
> #      Fri May 19 12:07:23 2017 +0200
> # Node ID ba781657de9f2e5b735e8a207cdf0b858047190c
> # Parent  c87db79b95079c3fd0032be7a25cd41567ed11cb
> # EXP-Topic develforcedate
> util: refactor util.parsedate to raises ValueError
> 
> Split most of util.parsedate in util.rawparsedate and make it raises ValueError instead
> of error.Abort.
> 
> The util.parsedate function is now just a shell function converting ValueError
> to error.Abort for existing users.
> 
> I need to parse a date from config in a later patch and use util.rawparsedate
> with ui.configwith which expect a convert that raises ValueError.
> 
> diff -r c87db79b95079c3fd0032be7a25cd41567ed11cb -r ba781657de9f2e5b735e8a207cdf0b858047190c mercurial/util.py
> --- a/mercurial/util.py	Sat May 20 22:27:52 2017 -0700
> +++ b/mercurial/util.py	Fri May 19 12:07:23 2017 +0200
> @@ -1924,6 +1924,9 @@
>      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.
>  
> +    This function calls rawparsedate and convert ValueError to Abort for
> +    functions that needs higher level exception.
> +
>      >>> parsedate(' today ') == parsedate(\
>                                    datetime.date.today().strftime('%b %d'))
>      True
> @@ -1938,6 +1941,20 @@
>      >>> 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.

Can you add a specific exception type instead? Embedding non-ascii string in
ValueError can cause problems on Python 3.
Boris Feld - May 23, 2017, 1:20 p.m.
On Tue, 2017-05-23 at 21:39 +0900, Yuya Nishihara wrote:
> On Mon, 22 May 2017 20:46:21 +0200, Boris Feld wrote:
> > # HG changeset patch
> > # User Boris Feld <boris.feld@octobus.net>
> > # Date 1495188443 -7200
> > #      Fri May 19 12:07:23 2017 +0200
> > # Node ID ba781657de9f2e5b735e8a207cdf0b858047190c
> > # Parent  c87db79b95079c3fd0032be7a25cd41567ed11cb
> > # EXP-Topic develforcedate
> > util: refactor util.parsedate to raises ValueError
> > 
> > Split most of util.parsedate in util.rawparsedate and make it
> > raises ValueError instead
> > of error.Abort.
> > 
> > The util.parsedate function is now just a shell function converting
> > ValueError
> > to error.Abort for existing users.
> > 
> > I need to parse a date from config in a later patch and use
> > util.rawparsedate
> > with ui.configwith which expect a convert that raises ValueError.
> > 
> > diff -r c87db79b95079c3fd0032be7a25cd41567ed11cb -r
> > ba781657de9f2e5b735e8a207cdf0b858047190c mercurial/util.py
> > --- a/mercurial/util.py	Sat May 20 22:27:52 2017 -0700
> > +++ b/mercurial/util.py	Fri May 19 12:07:23 2017 +0200
> > @@ -1924,6 +1924,9 @@
> >      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.
> >  
> > +    This function calls rawparsedate and convert ValueError to
> > Abort for
> > +    functions that needs higher level exception.
> > +
> >      >>> parsedate(' today ') == parsedate(\
> >                                    datetime.date.today().strftime('
> > %b %d'))
> >      True
> > @@ -1938,6 +1941,20 @@
> >      >>> 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.
> 
> Can you add a specific exception type instead? Embedding non-ascii
> string in
> ValueError can cause problems on Python 3.

I could but only if I made the new exception type inherits ValueError
as ui.configwith only catch ValueError (or subclasses), will it cause
problems if the new InvalidDateValue (example name) inherits ValueError
?
Yuya Nishihara - May 23, 2017, 2:10 p.m.
On Tue, 23 May 2017 15:20:36 +0200, Boris Feld wrote:
> On Tue, 2017-05-23 at 21:39 +0900, Yuya Nishihara wrote:
> > On Mon, 22 May 2017 20:46:21 +0200, Boris Feld wrote:
> > > # HG changeset patch
> > > # User Boris Feld <boris.feld@octobus.net>
> > > # Date 1495188443 -7200
> > > #      Fri May 19 12:07:23 2017 +0200
> > > # Node ID ba781657de9f2e5b735e8a207cdf0b858047190c
> > > # Parent  c87db79b95079c3fd0032be7a25cd41567ed11cb
> > > # EXP-Topic develforcedate
> > > util: refactor util.parsedate to raises ValueError
> > > 
> > > Split most of util.parsedate in util.rawparsedate and make it
> > > raises ValueError instead
> > > of error.Abort.
> > > 
> > > The util.parsedate function is now just a shell function converting
> > > ValueError
> > > to error.Abort for existing users.
> > > 
> > > I need to parse a date from config in a later patch and use
> > > util.rawparsedate
> > > with ui.configwith which expect a convert that raises ValueError.
> > > 
> > > diff -r c87db79b95079c3fd0032be7a25cd41567ed11cb -r
> > > ba781657de9f2e5b735e8a207cdf0b858047190c mercurial/util.py
> > > --- a/mercurial/util.py	Sat May 20 22:27:52 2017 -0700
> > > +++ b/mercurial/util.py	Fri May 19 12:07:23 2017 +0200
> > > @@ -1924,6 +1924,9 @@
> > >      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.
> > >  
> > > +    This function calls rawparsedate and convert ValueError to
> > > Abort for
> > > +    functions that needs higher level exception.
> > > +
> > >      >>> parsedate(' today ') == parsedate(\
> > >                                    datetime.date.today().strftime('
> > > %b %d'))
> > >      True
> > > @@ -1938,6 +1941,20 @@
> > >      >>> 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.
> > 
> > Can you add a specific exception type instead? Embedding non-ascii
> > string in
> > ValueError can cause problems on Python 3.
> 
> I could but only if I made the new exception type inherits ValueError
> as ui.configwith only catch ValueError (or subclasses), will it cause
> problems if the new InvalidDateValue (example name) inherits ValueError
> ?

I think it's okay to extend ui.configwith() to catch the new exception.
Perhaps ParseError could be reused, but the default output is slightly
different than Abort.

The problem of using ValueError is that ValueError will hold a unicode
string in Python 3. Putting a bytes into ValueError may cause problems
when catching ValueError raised from arbitrary locations.

Patch

diff -r c87db79b95079c3fd0032be7a25cd41567ed11cb -r ba781657de9f2e5b735e8a207cdf0b858047190c mercurial/util.py
--- a/mercurial/util.py	Sat May 20 22:27:52 2017 -0700
+++ b/mercurial/util.py	Fri May 19 12:07:23 2017 +0200
@@ -1924,6 +1924,9 @@ 
     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.
 
+    This function calls rawparsedate and convert ValueError to Abort for
+    functions that needs higher level exception.
+
     >>> parsedate(' today ') == parsedate(\
                                   datetime.date.today().strftime('%b %d'))
     True
@@ -1938,6 +1941,20 @@ 
     >>> 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:
@@ -1984,15 +2001,15 @@ 
             else:
                 break
         else:
-            raise Abort(_('invalid date: %r') % date)
+            raise ValueError(_('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 Abort(_('date exceeds 32 bits: %d') % when)
+        raise ValueError(_('date exceeds 32 bits: %d') % when)
     if offset < -50400 or offset > 43200:
-        raise Abort(_('impossible time zone offset: %d') % offset)
+        raise ValueError(_('impossible time zone offset: %d') % offset)
     return when, offset
 
 def matchdate(date):