Submitter | Brett Cannon |
---|---|
Date | March 26, 2015, 12:39 a.m. |
Message ID | <5efe91a4c2d49d74d110.1427330398@bcannon-macbookpro2.roam.corp.google.com> |
Download | mbox | patch |
Permalink | /patch/8281/ |
State | Accepted |
Headers | show |
Comments
On Wed, 25 Mar 2015 20:39:58 -0400, Brett Cannon wrote: > # HG changeset patch > # User Brett Cannon <brett@python.org> > # Date 1427329317 14400 > # Wed Mar 25 20:21:57 2015 -0400 > # Node ID 5efe91a4c2d49d74d1104edd50cd5a82003df4b6 > # Parent 6d273d0a51aa24f9c837d67b656467f98f98786d > hglib: make util.cmdbuilder work with bytes (issue4520) > > diff -r 6d273d0a51aa -r 5efe91a4c2d4 hglib/util.py > --- a/hglib/util.py Wed Mar 25 20:19:09 2015 -0400 > +++ b/hglib/util.py Wed Mar 25 20:21:57 2015 -0400 > @@ -103,7 +103,7 @@ > if val is None: > continue > > - arg = arg.replace(b('_'), b('-')) > + arg = b(arg).replace(b('_'), b('-')) > if arg != b('-'): > if len(arg) == 1: > arg = b('-') + arg > @@ -115,10 +115,13 @@ > elif isinstance(val, list): > for v in val: > cmd.append(arg) > - cmd.append(str(v)) > + cmd.append(v) Doctest fails here. % python -m doctest hglib/util.py ********************************************************************** File "hglib/util.py", line 97, in util.cmdbuilder Failed example: cmdbuilder('cmd', list=[1, 2]) Expected: ['cmd', '--list', '1', '--list', '2'] Got: ['cmd', '--list', 1, '--list', 2] ********************************************************************** > + elif isinstance(val, int): > + cmd.append(arg) > + cmd.append(strtobytes(val)) > else: > cmd.append(arg) > - cmd.append(str(val)) > + cmd.append(val) Why not just replace str() by strtobytes() ?
On Thu, Mar 26, 2015 at 10:01 AM Yuya Nishihara <yuya@tcha.org> wrote: > On Wed, 25 Mar 2015 20:39:58 -0400, Brett Cannon wrote: > > # HG changeset patch > > # User Brett Cannon <brett@python.org> > > # Date 1427329317 14400 > > # Wed Mar 25 20:21:57 2015 -0400 > > # Node ID 5efe91a4c2d49d74d1104edd50cd5a82003df4b6 > > # Parent 6d273d0a51aa24f9c837d67b656467f98f98786d > > hglib: make util.cmdbuilder work with bytes (issue4520) > > > > diff -r 6d273d0a51aa -r 5efe91a4c2d4 hglib/util.py > > --- a/hglib/util.py Wed Mar 25 20:19:09 2015 -0400 > > +++ b/hglib/util.py Wed Mar 25 20:21:57 2015 -0400 > > @@ -103,7 +103,7 @@ > > if val is None: > > continue > > > > - arg = arg.replace(b('_'), b('-')) > > + arg = b(arg).replace(b('_'), b('-')) > > if arg != b('-'): > > if len(arg) == 1: > > arg = b('-') + arg > > @@ -115,10 +115,13 @@ > > elif isinstance(val, list): > > for v in val: > > cmd.append(arg) > > - cmd.append(str(v)) > > + cmd.append(v) > > Doctest fails here. > > % python -m doctest hglib/util.py > ********************************************************************** > File "hglib/util.py", line 97, in util.cmdbuilder > Failed example: > cmdbuilder('cmd', list=[1, 2]) > Expected: > ['cmd', '--list', '1', '--list', '2'] > Got: > ['cmd', '--list', 1, '--list', 2] > ********************************************************************** > > Are doctests typically used? http://mercurial.selenic.com/wiki/ContributingChanges doesn't mention them and I would assume that if they were important they would be integrated into the test.py script in hglib. Regardless, how does this get fixed? Does this cause all of my patches to get rejected and thus I have to go back and re-submit all of them? Does Matt apply the other patches that don't break anything, skip the ones that do, and then I re-submit just the ones that got rejected? > > + elif isinstance(val, int): > > + cmd.append(arg) > > + cmd.append(strtobytes(val)) > > else: > > cmd.append(arg) > > - cmd.append(str(val)) > > + cmd.append(val) > > Why not just replace str() by strtobytes() ? > It might work. strtobytes() post-dates all of these changes in my ported local repo so I just didn't think about it. It's quite possible it will fix this problem and negate the need to special-case int.
On Thu, Mar 26, 2015 at 7:19 AM, Brett Cannon <brett@python.org> wrote: > > > On Thu, Mar 26, 2015 at 10:01 AM Yuya Nishihara <yuya@tcha.org> wrote: >> >> On Wed, 25 Mar 2015 20:39:58 -0400, Brett Cannon wrote: >> > # HG changeset patch >> > # User Brett Cannon <brett@python.org> >> > # Date 1427329317 14400 >> > # Wed Mar 25 20:21:57 2015 -0400 >> > # Node ID 5efe91a4c2d49d74d1104edd50cd5a82003df4b6 >> > # Parent 6d273d0a51aa24f9c837d67b656467f98f98786d >> > hglib: make util.cmdbuilder work with bytes (issue4520) >> > >> > diff -r 6d273d0a51aa -r 5efe91a4c2d4 hglib/util.py >> > --- a/hglib/util.py Wed Mar 25 20:19:09 2015 -0400 >> > +++ b/hglib/util.py Wed Mar 25 20:21:57 2015 -0400 >> > @@ -103,7 +103,7 @@ >> > if val is None: >> > continue >> > >> > - arg = arg.replace(b('_'), b('-')) >> > + arg = b(arg).replace(b('_'), b('-')) >> > if arg != b('-'): >> > if len(arg) == 1: >> > arg = b('-') + arg >> > @@ -115,10 +115,13 @@ >> > elif isinstance(val, list): >> > for v in val: >> > cmd.append(arg) >> > - cmd.append(str(v)) >> > + cmd.append(v) >> >> Doctest fails here. >> >> % python -m doctest hglib/util.py >> ********************************************************************** >> File "hglib/util.py", line 97, in util.cmdbuilder >> Failed example: >> cmdbuilder('cmd', list=[1, 2]) >> Expected: >> ['cmd', '--list', '1', '--list', '2'] >> Got: >> ['cmd', '--list', 1, '--list', 2] >> ********************************************************************** >> > > Are doctests typically used? > http://mercurial.selenic.com/wiki/ContributingChanges doesn't mention them > and I would assume that if they were important they would be integrated into > the test.py script in hglib. Use 'make tests', it passes the flag needed to test.py to also run doctests.
On Thu, 26 Mar 2015 14:19:01 +0000, Brett Cannon wrote: > On Thu, Mar 26, 2015 at 10:01 AM Yuya Nishihara <yuya@tcha.org> wrote: > > > On Wed, 25 Mar 2015 20:39:58 -0400, Brett Cannon wrote: > > > # HG changeset patch > > > # User Brett Cannon <brett@python.org> > > > # Date 1427329317 14400 > > > # Wed Mar 25 20:21:57 2015 -0400 > > > # Node ID 5efe91a4c2d49d74d1104edd50cd5a82003df4b6 > > > # Parent 6d273d0a51aa24f9c837d67b656467f98f98786d > > > hglib: make util.cmdbuilder work with bytes (issue4520) > > > > > > diff -r 6d273d0a51aa -r 5efe91a4c2d4 hglib/util.py > > > --- a/hglib/util.py Wed Mar 25 20:19:09 2015 -0400 > > > +++ b/hglib/util.py Wed Mar 25 20:21:57 2015 -0400 > > > @@ -103,7 +103,7 @@ > > > if val is None: > > > continue > > > > > > - arg = arg.replace(b('_'), b('-')) > > > + arg = b(arg).replace(b('_'), b('-')) > > > if arg != b('-'): > > > if len(arg) == 1: > > > arg = b('-') + arg > > > @@ -115,10 +115,13 @@ > > > elif isinstance(val, list): > > > for v in val: > > > cmd.append(arg) > > > - cmd.append(str(v)) > > > + cmd.append(v) > > > > Doctest fails here. > > > > % python -m doctest hglib/util.py > > ********************************************************************** > > File "hglib/util.py", line 97, in util.cmdbuilder > > Failed example: > > cmdbuilder('cmd', list=[1, 2]) > > Expected: > > ['cmd', '--list', '1', '--list', '2'] > > Got: > > ['cmd', '--list', 1, '--list', 2] > > ********************************************************************** > > > > > Are doctests typically used? > http://mercurial.selenic.com/wiki/ContributingChanges doesn't mention them > and I would assume that if they were important they would be integrated > into the test.py script in hglib. They are integrated. See Makefile. But, I have no idea how b'' can be handled in doctest on Python 3. Regards,
On Thu, Mar 26, 2015 at 10:35 AM Yuya Nishihara <yuya@tcha.org> wrote: > On Thu, 26 Mar 2015 14:19:01 +0000, Brett Cannon wrote: > > On Thu, Mar 26, 2015 at 10:01 AM Yuya Nishihara <yuya@tcha.org> wrote: > > > > > On Wed, 25 Mar 2015 20:39:58 -0400, Brett Cannon wrote: > > > > # HG changeset patch > > > > # User Brett Cannon <brett@python.org> > > > > # Date 1427329317 14400 > > > > # Wed Mar 25 20:21:57 2015 -0400 > > > > # Node ID 5efe91a4c2d49d74d1104edd50cd5a82003df4b6 > > > > # Parent 6d273d0a51aa24f9c837d67b656467f98f98786d > > > > hglib: make util.cmdbuilder work with bytes (issue4520) > > > > > > > > diff -r 6d273d0a51aa -r 5efe91a4c2d4 hglib/util.py > > > > --- a/hglib/util.py Wed Mar 25 20:19:09 2015 -0400 > > > > +++ b/hglib/util.py Wed Mar 25 20:21:57 2015 -0400 > > > > @@ -103,7 +103,7 @@ > > > > if val is None: > > > > continue > > > > > > > > - arg = arg.replace(b('_'), b('-')) > > > > + arg = b(arg).replace(b('_'), b('-')) > > > > if arg != b('-'): > > > > if len(arg) == 1: > > > > arg = b('-') + arg > > > > @@ -115,10 +115,13 @@ > > > > elif isinstance(val, list): > > > > for v in val: > > > > cmd.append(arg) > > > > - cmd.append(str(v)) > > > > + cmd.append(v) > > > > > > Doctest fails here. > > > > > > % python -m doctest hglib/util.py > > > ********************************************************************** > > > File "hglib/util.py", line 97, in util.cmdbuilder > > > Failed example: > > > cmdbuilder('cmd', list=[1, 2]) > > > Expected: > > > ['cmd', '--list', '1', '--list', '2'] > > > Got: > > > ['cmd', '--list', 1, '--list', 2] > > > ********************************************************************** > > > > > > > > Are doctests typically used? > > http://mercurial.selenic.com/wiki/ContributingChanges doesn't mention > them > > and I would assume that if they were important they would be integrated > > into the test.py script in hglib. > > They are integrated. See Makefile. > > But, I have no idea how b'' can be handled in doctest on Python 3. > With compatibility with Python 2.5 and older it simply can't. You would have to modify the doctest to either check different results based on Python 2 or 3 or modify so that it basically becomes a unit test instead of a doctest.
Patch
diff -r 6d273d0a51aa -r 5efe91a4c2d4 hglib/util.py --- a/hglib/util.py Wed Mar 25 20:19:09 2015 -0400 +++ b/hglib/util.py Wed Mar 25 20:21:57 2015 -0400 @@ -103,7 +103,7 @@ if val is None: continue - arg = arg.replace(b('_'), b('-')) + arg = b(arg).replace(b('_'), b('-')) if arg != b('-'): if len(arg) == 1: arg = b('-') + arg @@ -115,10 +115,13 @@ elif isinstance(val, list): for v in val: cmd.append(arg) - cmd.append(str(v)) + cmd.append(v) + elif isinstance(val, int): + cmd.append(arg) + cmd.append(strtobytes(val)) else: cmd.append(arg) - cmd.append(str(val)) + cmd.append(val) for a in args: if a is not None: