| Submitter | Pulkit Goyal |
|---|---|
| Date | Nov. 2, 2016, 10:23 p.m. |
| Message ID | <aef03902a5c1a13e9775.1478125391@pulkit-goyal> |
| Download | mbox | patch |
| Permalink | /patch/17303/ |
| State | Accepted |
| Headers | show |
Comments
On Thu, 03 Nov 2016 03:53:11 +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pulkit@gmail.com> > # Date 1478121825 -19800 > # Thu Nov 03 02:53:45 2016 +0530 > # Node ID aef03902a5c1a13e9775059a5efdeb2466399ada > # Parent 9e259e7b59b6358eb842eabbc99f4c18a4cc5009 > py3: use try/except to check for basestring > > The term basestring don't exist in Python 3.5 and throws a NameError. > It used to refer to unicodes in Python 2. Used try/expect to handle this. > > diff -r 9e259e7b59b6 -r aef03902a5c1 mercurial/ui.py > --- a/mercurial/ui.py Thu Nov 03 02:27:46 2016 +0530 > +++ b/mercurial/ui.py Thu Nov 03 02:53:45 2016 +0530 > @@ -520,7 +520,12 @@ > result = self.config(section, name, untrusted=untrusted) > if result is None: > result = default or [] > - if isinstance(result, basestring): > + checkunicode = False > + try: > + checkunicode = isinstance(result, basestring) > + except NameError: > + checkunicode = isinstance(result, str) > + if checkunicode: > result = _configlist(result.lstrip(' ,\n')) Given "result" is a source variable of a list to be returned, it shouldn't be a unicode. So we can simply use bytes instead of basestring here.
This https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089099.html is a better version of what I want to do, since this didn't went through I will be using this. On Fri, Nov 4, 2016 at 9:21 AM, Yuya Nishihara <yuya@tcha.org> wrote: > On Thu, 03 Nov 2016 03:53:11 +0530, Pulkit Goyal wrote: >> # HG changeset patch >> # User Pulkit Goyal <7895pulkit@gmail.com> >> # Date 1478121825 -19800 >> # Thu Nov 03 02:53:45 2016 +0530 >> # Node ID aef03902a5c1a13e9775059a5efdeb2466399ada >> # Parent 9e259e7b59b6358eb842eabbc99f4c18a4cc5009 >> py3: use try/except to check for basestring >> >> The term basestring don't exist in Python 3.5 and throws a NameError. >> It used to refer to unicodes in Python 2. Used try/expect to handle this. >> >> diff -r 9e259e7b59b6 -r aef03902a5c1 mercurial/ui.py >> --- a/mercurial/ui.py Thu Nov 03 02:27:46 2016 +0530 >> +++ b/mercurial/ui.py Thu Nov 03 02:53:45 2016 +0530 >> @@ -520,7 +520,12 @@ >> result = self.config(section, name, untrusted=untrusted) >> if result is None: >> result = default or [] >> - if isinstance(result, basestring): >> + checkunicode = False >> + try: >> + checkunicode = isinstance(result, basestring) >> + except NameError: >> + checkunicode = isinstance(result, str) >> + if checkunicode: >> result = _configlist(result.lstrip(' ,\n')) > > Given "result" is a source variable of a list to be returned, it shouldn't > be a unicode. So we can simply use bytes instead of basestring here.
On Mon, 7 Nov 2016 00:15:21 +0530, Pulkit Goyal wrote: > This https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089099.html > is a better version of what I want to do, since this didn't went > through I will be using this. I'm okay with that pycompat.basestring stuff, but I'm pretty sure most of our basestring uses are moot since we avoid using unicodes except for very specific string manipulations. > >> @@ -520,7 +520,12 @@ > >> result = self.config(section, name, untrusted=untrusted) > >> if result is None: > >> result = default or [] > >> - if isinstance(result, basestring): > >> + checkunicode = False > >> + try: > >> + checkunicode = isinstance(result, basestring) > >> + except NameError: > >> + checkunicode = isinstance(result, str) > >> + if checkunicode: > >> result = _configlist(result.lstrip(' ,\n')) And with this change, ui.configlist() would look as if it supports unicodes, which seems confusing.
On Mon, Nov 7, 2016 at 5:55 PM, Yuya Nishihara <yuya@tcha.org> wrote: > On Mon, 7 Nov 2016 00:15:21 +0530, Pulkit Goyal wrote: >> This https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089099.html >> is a better version of what I want to do, since this didn't went >> through I will be using this. > > I'm okay with that pycompat.basestring stuff, but I'm pretty sure most of > our basestring uses are moot since we avoid using unicodes except for very > specific string manipulations. > >> >> @@ -520,7 +520,12 @@ >> >> result = self.config(section, name, untrusted=untrusted) >> >> if result is None: >> >> result = default or [] >> >> - if isinstance(result, basestring): >> >> + checkunicode = False >> >> + try: >> >> + checkunicode = isinstance(result, basestring) >> >> + except NameError: >> >> + checkunicode = isinstance(result, str) >> >> + if checkunicode: >> >> result = _configlist(result.lstrip(' ,\n')) > > And with this change, ui.configlist() would look as if it supports unicodes, > which seems confusing. Can you cherry pick some commits from that series?
On Mon, 7 Nov 2016 20:37:14 +0530, Pulkit Goyal wrote: > On Mon, Nov 7, 2016 at 5:55 PM, Yuya Nishihara <yuya@tcha.org> wrote: > > On Mon, 7 Nov 2016 00:15:21 +0530, Pulkit Goyal wrote: > >> This https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089099.html > >> is a better version of what I want to do, since this didn't went > >> through I will be using this. > > > > I'm okay with that pycompat.basestring stuff, but I'm pretty sure most of > > our basestring uses are moot since we avoid using unicodes except for very > > specific string manipulations. > > > >> >> @@ -520,7 +520,12 @@ > >> >> result = self.config(section, name, untrusted=untrusted) > >> >> if result is None: > >> >> result = default or [] > >> >> - if isinstance(result, basestring): > >> >> + checkunicode = False > >> >> + try: > >> >> + checkunicode = isinstance(result, basestring) > >> >> + except NameError: > >> >> + checkunicode = isinstance(result, str) > >> >> + if checkunicode: > >> >> result = _configlist(result.lstrip(' ,\n')) > > > > And with this change, ui.configlist() would look as if it supports unicodes, > > which seems confusing. > Can you cherry pick some commits from that series? Can you pick out good ones? FYI, the basestring patch had a problem, which didn't work on Python 2. https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089160.html
> Can you pick out good ones? These ones! https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089096.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089097.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089098.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089100.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089103.html - You can drop u'' there in flight https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089102.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089104.html They all look good and are a part of fixing things while `hg version` on Python 3. Still if you find one wrong, drop that so that I can follow up with a better patch. > > FYI, the basestring patch had a problem, which didn't work on Python 2. > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089160.html Okay I will send a better patch which will fix that.
On Tue, 8 Nov 2016 20:05:51 +0530, Pulkit Goyal wrote: > > Can you pick out good ones? > These ones! Pushed some of them, thanks. > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089096.html Already queued. > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089097.html > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089098.html > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089100.html Queued these. > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089103.html > - You can drop u'' there in flight Dropped per discussion. > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089102.html Added comment. > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089104.html Syntax error on Python 2.
There is something strange. One of the patch you pushed and replied to the mail is not the same. On Wed, Nov 9, 2016 at 6:09 PM, Yuya Nishihara <yuya@tcha.org> wrote: > On Tue, 8 Nov 2016 20:05:51 +0530, Pulkit Goyal wrote: >> > Can you pick out good ones? >> These ones! > > Pushed some of them, thanks. > >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089096.html > > Already queued. > >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089097.html >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089098.html >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089100.html > > Queued these. > >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089103.html >> - You can drop u'' there in flight > > Dropped per discussion. I can't see the u'' dropped. https://www.mercurial-scm.org/repo/hg-committed/rev/9df29b7c62cf > >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089102.html > > Added comment. I don't know what you mean here as this is not pushed. > >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089104.html > > Syntax error on Python 2. But you pushed this one. https://www.mercurial-scm.org/repo/hg-committed/rev/954002426f78. I am unsure of whether the reply contained wrong links or you pushed wrong ones. Let me know so that I can follow up accordingly.
On Fri, 2 Dec 2016 04:32:47 +0530, Pulkit Goyal wrote: > There is something strange. One of the patch you pushed and replied to > the mail is not the same. I've queued new versions of these patches. > >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089102.html > > > > Added comment. Er, I don't remember. I guess I would wanted to tell you that I wrote some comments to that patch. https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-November/090218.html
Patch
diff -r 9e259e7b59b6 -r aef03902a5c1 mercurial/ui.py --- a/mercurial/ui.py Thu Nov 03 02:27:46 2016 +0530 +++ b/mercurial/ui.py Thu Nov 03 02:53:45 2016 +0530 @@ -520,7 +520,12 @@ result = self.config(section, name, untrusted=untrusted) if result is None: result = default or [] - if isinstance(result, basestring): + checkunicode = False + try: + checkunicode = isinstance(result, basestring) + except NameError: + checkunicode = isinstance(result, str) + if checkunicode: result = _configlist(result.lstrip(' ,\n')) if result is None: result = default or []