Patchwork [05,of,12,topics] flake8: fix W503 style

login
register
mail settings
Submitter Sean Farley
Date Aug. 31, 2016, 3:58 a.m.
Message ID <752a62b50ba31b768c52.1472615891@laptop.local>
Download mbox | patch
Permalink /patch/16503/
State Accepted
Headers show

Comments

Sean Farley - Aug. 31, 2016, 3:58 a.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1472595240 25200
#      Tue Aug 30 15:14:00 2016 -0700
# Node ID 752a62b50ba31b768c52c6b423cd1a36d0c6eae2
# Parent  8f1e18702ba107ffcb35be04dc8a1e650e5c1e7f
# EXP-Topic flake8
flake8: fix W503 style
Anton Shestakov - Aug. 31, 2016, 5:22 a.m.
On Tue, 30 Aug 2016 20:58:11 -0700
Sean Farley <sean@farley.io> wrote:

> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1472595240 25200
> #      Tue Aug 30 15:14:00 2016 -0700
> # Node ID 752a62b50ba31b768c52c6b423cd1a36d0c6eae2
> # Parent  8f1e18702ba107ffcb35be04dc8a1e650e5c1e7f
> # EXP-Topic flake8
> flake8: fix W503 style
> 
> diff --git a/hgext3rd/topic/destination.py b/hgext3rd/topic/destination.py
> --- a/hgext3rd/topic/destination.py
> +++ b/hgext3rd/topic/destination.py
> @@ -72,12 +72,12 @@ def _destupdatetopic(repo, clean, check)
>      if bookmarks.isactivewdirparent(repo):
>          movemark = repo['.'].node()
>      return node, movemark, None
>  
>  def desthistedit(orig, ui, repo):
> -    if not (ui.config('histedit', 'defaultrev', None) is None
> -            and repo.currenttopic):
> +    if not (ui.config('histedit', 'defaultrev', None) is None and
> +            repo.currenttopic):
>          return orig(ui, repo)
>      revs = repo.revs('::. and stack()')
>      if revs:
>          return revs.min()
>      return None
> @@ -104,13 +104,13 @@ def modsetup(ui):
>          extensions.wrapfunction(destutil, '_destmergebranch', _destmergebranch)
>      try:
>          rebase = extensions.find('rebase')
>      except KeyError:
>          rebase = None
> -    if (util.safehasattr(rebase, '_destrebase')
> +    if (util.safehasattr(rebase, '_destrebase') and
>              # logic not shared with merge yet < hg-3.8
> -            and not util.safehasattr(rebase, '_definesets')):
> +            not util.safehasattr(rebase, '_definesets')):
>          extensions.wrapfunction(rebase, '_destrebase', _destmergebranch)
>      if util.safehasattr(destutil, 'destupdatesteps'):
>          bridx = destutil.destupdatesteps.index('branch')
>          destutil.destupdatesteps.insert(bridx, 'topic')
>          destutil.destupdatestepmap['topic'] = _destupdatetopic
> diff --git a/hgext3rd/topic/discovery.py b/hgext3rd/topic/discovery.py
> --- a/hgext3rd/topic/discovery.py
> +++ b/hgext3rd/topic/discovery.py
> @@ -14,12 +14,12 @@ from mercurial import (
>  )
>  
>  from . import topicmap
>  
>  def _headssummary(orig, repo, remote, outgoing):
> -    publishing = ('phases' not in remote.listkeys('namespaces')
> -                  or bool(remote.listkeys('phases').get('publishing', False)))
> +    publishing = ('phases' not in remote.listkeys('namespaces') or
> +                  bool(remote.listkeys('phases').get('publishing', False)))
>      if publishing or not remote.capable('topics'):
>          return orig(repo, remote, outgoing)
>      oldrepo = repo.__class__
>      oldbranchcache = branchmap.branchcache
>      oldfilename = branchmap._filename
> diff --git a/hgext3rd/topic/topicmap.py b/hgext3rd/topic/topicmap.py
> --- a/hgext3rd/topic/topicmap.py
> +++ b/hgext3rd/topic/topicmap.py
> @@ -124,12 +124,12 @@ class topiccache(oldbranchcache):
>          try:
>              if (self.tipnode == repo.changelog.node(self.tiprev)):
>                  fh = scmutil.filteredhash(repo, self.tiprev)
>                  if fh is None:
>                      fh = nullid
> -                if ((self.filteredhash == fh)
> -                    and (self.phaseshash == _phaseshash(repo, self.tiprev))):
> +                if ((self.filteredhash == fh) and
> +                    (self.phaseshash == _phaseshash(repo, self.tiprev))):
>                      return True
>              return False
>          except IndexError:
>              return False
>  
> diff --git a/setup.cfg b/setup.cfg
> --- a/setup.cfg
> +++ b/setup.cfg
> @@ -1,2 +1,2 @@
>  [flake8]
> -ignore = E261, E266, E302, E129, E731, E124, E713, E301, E501, E111, E123, E222, W503
> +ignore = E261, E266, E302, E129, E731, E124, E713, E301, E501, E111, E123, E222

W503 is actually disabled in pep8/pycodestyle by default [0], because
the PEP is kinda on the fence about it [1].

[0] https://pep8.readthedocs.io/en/latest/intro.html#error-codes
[1] https://www.python.org/dev/peps/pep-0008/#should-a-line-break-before-or-after-a-binary-operator
Augie Fackler - Aug. 31, 2016, 12:54 p.m.
On Tue, Aug 30, 2016 at 08:58:11PM -0700, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1472595240 25200
> #      Tue Aug 30 15:14:00 2016 -0700
> # Node ID 752a62b50ba31b768c52c6b423cd1a36d0c6eae2
> # Parent  8f1e18702ba107ffcb35be04dc8a1e650e5c1e7f
> # EXP-Topic flake8
> flake8: fix W503 style
>
> diff --git a/hgext3rd/topic/destination.py b/hgext3rd/topic/destination.py
> --- a/hgext3rd/topic/destination.py
> +++ b/hgext3rd/topic/destination.py
> @@ -72,12 +72,12 @@ def _destupdatetopic(repo, clean, check)
>      if bookmarks.isactivewdirparent(repo):
>          movemark = repo['.'].node()
>      return node, movemark, None
>
>  def desthistedit(orig, ui, repo):
> -    if not (ui.config('histedit', 'defaultrev', None) is None
> -            and repo.currenttopic):
> +    if not (ui.config('histedit', 'defaultrev', None) is None and
> +            repo.currenttopic):

I know I intentionally put the binary operator at the start of the subsequent line because then it's less ambiguous when visually parsing, eg:

if (foo
    and bar):
    baz

leaves it more visually obvious where the body of the if statement begins than

if (foo and
    bar):
    baz

I'm not sure how others feel.

>          return orig(ui, repo)
>      revs = repo.revs('::. and stack()')
>      if revs:
>          return revs.min()
>      return None
> @@ -104,13 +104,13 @@ def modsetup(ui):
>          extensions.wrapfunction(destutil, '_destmergebranch', _destmergebranch)
>      try:
>          rebase = extensions.find('rebase')
>      except KeyError:
>          rebase = None
> -    if (util.safehasattr(rebase, '_destrebase')
> +    if (util.safehasattr(rebase, '_destrebase') and
>              # logic not shared with merge yet < hg-3.8
> -            and not util.safehasattr(rebase, '_definesets')):
> +            not util.safehasattr(rebase, '_definesets')):
>          extensions.wrapfunction(rebase, '_destrebase', _destmergebranch)
>      if util.safehasattr(destutil, 'destupdatesteps'):
>          bridx = destutil.destupdatesteps.index('branch')
>          destutil.destupdatesteps.insert(bridx, 'topic')
>          destutil.destupdatestepmap['topic'] = _destupdatetopic
> diff --git a/hgext3rd/topic/discovery.py b/hgext3rd/topic/discovery.py
> --- a/hgext3rd/topic/discovery.py
> +++ b/hgext3rd/topic/discovery.py
> @@ -14,12 +14,12 @@ from mercurial import (
>  )
>
>  from . import topicmap
>
>  def _headssummary(orig, repo, remote, outgoing):
> -    publishing = ('phases' not in remote.listkeys('namespaces')
> -                  or bool(remote.listkeys('phases').get('publishing', False)))
> +    publishing = ('phases' not in remote.listkeys('namespaces') or
> +                  bool(remote.listkeys('phases').get('publishing', False)))
>      if publishing or not remote.capable('topics'):
>          return orig(repo, remote, outgoing)
>      oldrepo = repo.__class__
>      oldbranchcache = branchmap.branchcache
>      oldfilename = branchmap._filename
> diff --git a/hgext3rd/topic/topicmap.py b/hgext3rd/topic/topicmap.py
> --- a/hgext3rd/topic/topicmap.py
> +++ b/hgext3rd/topic/topicmap.py
> @@ -124,12 +124,12 @@ class topiccache(oldbranchcache):
>          try:
>              if (self.tipnode == repo.changelog.node(self.tiprev)):
>                  fh = scmutil.filteredhash(repo, self.tiprev)
>                  if fh is None:
>                      fh = nullid
> -                if ((self.filteredhash == fh)
> -                    and (self.phaseshash == _phaseshash(repo, self.tiprev))):
> +                if ((self.filteredhash == fh) and
> +                    (self.phaseshash == _phaseshash(repo, self.tiprev))):
>                      return True
>              return False
>          except IndexError:
>              return False
>
> diff --git a/setup.cfg b/setup.cfg
> --- a/setup.cfg
> +++ b/setup.cfg
> @@ -1,2 +1,2 @@
>  [flake8]
> -ignore = E261, E266, E302, E129, E731, E124, E713, E301, E501, E111, E123, E222, W503
> +ignore = E261, E266, E302, E129, E731, E124, E713, E301, E501, E111, E123, E222
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Sean Farley - Aug. 31, 2016, 5:23 p.m.
Augie Fackler <raf@durin42.com> writes:

> On Tue, Aug 30, 2016 at 08:58:11PM -0700, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean@farley.io>
>> # Date 1472595240 25200
>> #      Tue Aug 30 15:14:00 2016 -0700
>> # Node ID 752a62b50ba31b768c52c6b423cd1a36d0c6eae2
>> # Parent  8f1e18702ba107ffcb35be04dc8a1e650e5c1e7f
>> # EXP-Topic flake8
>> flake8: fix W503 style
>>
>> diff --git a/hgext3rd/topic/destination.py b/hgext3rd/topic/destination.py
>> --- a/hgext3rd/topic/destination.py
>> +++ b/hgext3rd/topic/destination.py
>> @@ -72,12 +72,12 @@ def _destupdatetopic(repo, clean, check)
>>      if bookmarks.isactivewdirparent(repo):
>>          movemark = repo['.'].node()
>>      return node, movemark, None
>>
>>  def desthistedit(orig, ui, repo):
>> -    if not (ui.config('histedit', 'defaultrev', None) is None
>> -            and repo.currenttopic):
>> +    if not (ui.config('histedit', 'defaultrev', None) is None and
>> +            repo.currenttopic):
>
> I know I intentionally put the binary operator at the start of the subsequent line because then it's less ambiguous when visually parsing, eg:
>
> if (foo
>     and bar):
>     baz
>
> leaves it more visually obvious where the body of the if statement begins than
>
> if (foo and
>     bar):
>     baz
>
> I'm not sure how others feel.

I could take or leave this one.
Pierre-Yves David - Sept. 6, 2016, 11:30 a.m.
On 08/31/2016 07:23 PM, Sean Farley wrote:
> Augie Fackler <raf@durin42.com> writes:
>
>> On Tue, Aug 30, 2016 at 08:58:11PM -0700, Sean Farley wrote:
>>> # HG changeset patch
>>> # User Sean Farley <sean@farley.io>
>>> # Date 1472595240 25200
>>> #      Tue Aug 30 15:14:00 2016 -0700
>>> # Node ID 752a62b50ba31b768c52c6b423cd1a36d0c6eae2
>>> # Parent  8f1e18702ba107ffcb35be04dc8a1e650e5c1e7f
>>> # EXP-Topic flake8
>>> flake8: fix W503 style
>>>
>>> diff --git a/hgext3rd/topic/destination.py b/hgext3rd/topic/destination.py
>>> --- a/hgext3rd/topic/destination.py
>>> +++ b/hgext3rd/topic/destination.py
>>> @@ -72,12 +72,12 @@ def _destupdatetopic(repo, clean, check)
>>>      if bookmarks.isactivewdirparent(repo):
>>>          movemark = repo['.'].node()
>>>      return node, movemark, None
>>>
>>>  def desthistedit(orig, ui, repo):
>>> -    if not (ui.config('histedit', 'defaultrev', None) is None
>>> -            and repo.currenttopic):
>>> +    if not (ui.config('histedit', 'defaultrev', None) is None and
>>> +            repo.currenttopic):
>>
>> I know I intentionally put the binary operator at the start of the subsequent line because then it's less ambiguous when visually parsing, eg:
>>
>> if (foo
>>     and bar):
>>     baz
>>
>> leaves it more visually obvious where the body of the if statement begins than
>>
>> if (foo and
>>     bar):
>>     baz
>>
>> I'm not sure how others feel.
>
> I could take or leave this one.

I'm also not fan of that change, (cf Augie point), let's drop this one.

Cheers,

Patch

diff --git a/hgext3rd/topic/destination.py b/hgext3rd/topic/destination.py
--- a/hgext3rd/topic/destination.py
+++ b/hgext3rd/topic/destination.py
@@ -72,12 +72,12 @@  def _destupdatetopic(repo, clean, check)
     if bookmarks.isactivewdirparent(repo):
         movemark = repo['.'].node()
     return node, movemark, None
 
 def desthistedit(orig, ui, repo):
-    if not (ui.config('histedit', 'defaultrev', None) is None
-            and repo.currenttopic):
+    if not (ui.config('histedit', 'defaultrev', None) is None and
+            repo.currenttopic):
         return orig(ui, repo)
     revs = repo.revs('::. and stack()')
     if revs:
         return revs.min()
     return None
@@ -104,13 +104,13 @@  def modsetup(ui):
         extensions.wrapfunction(destutil, '_destmergebranch', _destmergebranch)
     try:
         rebase = extensions.find('rebase')
     except KeyError:
         rebase = None
-    if (util.safehasattr(rebase, '_destrebase')
+    if (util.safehasattr(rebase, '_destrebase') and
             # logic not shared with merge yet < hg-3.8
-            and not util.safehasattr(rebase, '_definesets')):
+            not util.safehasattr(rebase, '_definesets')):
         extensions.wrapfunction(rebase, '_destrebase', _destmergebranch)
     if util.safehasattr(destutil, 'destupdatesteps'):
         bridx = destutil.destupdatesteps.index('branch')
         destutil.destupdatesteps.insert(bridx, 'topic')
         destutil.destupdatestepmap['topic'] = _destupdatetopic
diff --git a/hgext3rd/topic/discovery.py b/hgext3rd/topic/discovery.py
--- a/hgext3rd/topic/discovery.py
+++ b/hgext3rd/topic/discovery.py
@@ -14,12 +14,12 @@  from mercurial import (
 )
 
 from . import topicmap
 
 def _headssummary(orig, repo, remote, outgoing):
-    publishing = ('phases' not in remote.listkeys('namespaces')
-                  or bool(remote.listkeys('phases').get('publishing', False)))
+    publishing = ('phases' not in remote.listkeys('namespaces') or
+                  bool(remote.listkeys('phases').get('publishing', False)))
     if publishing or not remote.capable('topics'):
         return orig(repo, remote, outgoing)
     oldrepo = repo.__class__
     oldbranchcache = branchmap.branchcache
     oldfilename = branchmap._filename
diff --git a/hgext3rd/topic/topicmap.py b/hgext3rd/topic/topicmap.py
--- a/hgext3rd/topic/topicmap.py
+++ b/hgext3rd/topic/topicmap.py
@@ -124,12 +124,12 @@  class topiccache(oldbranchcache):
         try:
             if (self.tipnode == repo.changelog.node(self.tiprev)):
                 fh = scmutil.filteredhash(repo, self.tiprev)
                 if fh is None:
                     fh = nullid
-                if ((self.filteredhash == fh)
-                    and (self.phaseshash == _phaseshash(repo, self.tiprev))):
+                if ((self.filteredhash == fh) and
+                    (self.phaseshash == _phaseshash(repo, self.tiprev))):
                     return True
             return False
         except IndexError:
             return False
 
diff --git a/setup.cfg b/setup.cfg
--- a/setup.cfg
+++ b/setup.cfg
@@ -1,2 +1,2 @@ 
 [flake8]
-ignore = E261, E266, E302, E129, E731, E124, E713, E301, E501, E111, E123, E222, W503
+ignore = E261, E266, E302, E129, E731, E124, E713, E301, E501, E111, E123, E222