[PATCH 3 of 3] largefiles: cosmetics, whitespace, code style

Matt Mackall mpm at selenic.com
Fri Oct 14 15:22:42 CDT 2011


On Thu, 2011-10-13 at 21:48 -0400, Greg Ward wrote:
> # HG changeset patch
> # User Greg Ward <greg at gerg.ca>
> # Date 1318556574 14400
> # Node ID a6cbacb64c0d2ac80dfbb3a08a963f5b7106a30f
> # Parent  9094db68746ff19b97de04286bcbe647b32f5085
> largefiles: cosmetics, whitespace, code style
> 
> This is mainly about keeping code under the 80-column limit with as
> few backslashes as possible. I am deliberately not making any logic or
> behaviour changes here and have restrained myself to a few "peephole"
> refactorings.

I know this is super-tempting on your end, but it sucks on my end. If
you send me a big patch that says "this patch is all whitespace
changes", then I can dial my level of review back to back to minimal and
the big patch goes through in no time. Then you can send me a short
patch of code tweaks that also goes through quickly. If instead you send
me one big patch and say "it has little tweaks hidden in it", now I have
to go through -the whole thing- carefully, even the bulk of it where
nothing interesting happened.

Queued, after fixing up a bunch of trailing whitespace. Please consider
tweaking your editor to display it.

> diff --git a/hgext/largefiles/basestore.py b/hgext/largefiles/basestore.py
> --- a/hgext/largefiles/basestore.py
> +++ b/hgext/largefiles/basestore.py
> @@ -166,8 +166,8 @@
>      ui = repo.ui
>  
>      if not remote:
> -        path = getattr(repo, 'lfpullsource', None) or \
> -            ui.expandpath('default-push', 'default')
> +        path = (getattr(repo, 'lfpullsource', None) or
> +                ui.expandpath('default-push', 'default'))
>  
>          # ui.expandpath() leaves 'default-push' and 'default' alone if
>          # they cannot be expanded: fallback to the empty string,
> diff --git a/hgext/largefiles/lfcommands.py b/hgext/largefiles/lfcommands.py
> --- a/hgext/largefiles/lfcommands.py
> +++ b/hgext/largefiles/lfcommands.py
> @@ -383,9 +383,9 @@
>          # if it exists and its hash matches, it might have been locally
>          # modified before updating and the user chose 'local'.  in this case,
>          # it will not be in any store, so don't look for it.
> -        if (not os.path.exists(repo.wjoin(lfile)) \
> -                or expectedhash != lfutil.hashfile(repo.wjoin(lfile))) and \
> -                not lfutil.findfile(repo, expectedhash):
> +        if ((not os.path.exists(repo.wjoin(lfile)) or 
> +             expectedhash != lfutil.hashfile(repo.wjoin(lfile))) and
> +            not lfutil.findfile(repo, expectedhash)):
>              toget.append((lfile, expectedhash))
>  
>      if toget:
> @@ -441,9 +441,9 @@
>          if os.path.exists(absstandin+'.orig'):
>              shutil.copyfile(abslfile, abslfile+'.orig')
>          expecthash = lfutil.readstandin(repo, lfile)
> -        if expecthash != '' and \
> -                (not os.path.exists(abslfile) or \
> -                expecthash != lfutil.hashfile(abslfile)):
> +        if (expecthash != '' and
> +            (not os.path.exists(abslfile) or
> +             expecthash != lfutil.hashfile(abslfile))):
>              if not lfutil.copyfromcache(repo, expecthash, lfile):
>                  return None # don't try to set the mode or update the dirstate
>              ret = 1
> @@ -468,7 +468,6 @@
>  
>  # -- hg commands declarations ------------------------------------------------
>  
> -
>  cmdtable = {
>      'lfconvert': (lfconvert,
>                    [('s', 'size', '',
> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
> --- a/hgext/largefiles/lfutil.py
> +++ b/hgext/largefiles/lfutil.py
> @@ -86,8 +86,8 @@
>          path = os.path.join(path, hash)
>      else:
>          if os.name == 'nt':
> -            path = os.path.join(os.getenv('LOCALAPPDATA') or \
> -                os.getenv('APPDATA'), longname, hash)
> +            appdata = os.getenv('LOCALAPPDATA', os.getenv('APPDATA'))
> +            path = os.path.join(appdata, longname, hash)
>          elif os.name == 'posix':
>              path = os.path.join(os.getenv('HOME'), '.' + longname, hash)
>          else:
> @@ -184,7 +184,8 @@
>          matcher = getstandinmatcher(repo)
>  
>      # ignore unknown files in working directory
> -    return [splitstandin(f) for f in repo[rev].walk(matcher) \
> +    return [splitstandin(f) 
> +            for f in repo[rev].walk(matcher)
>              if rev is not None or repo.dirstate[f] != '?']
>  
>  def incache(repo, hash):
> @@ -394,8 +395,9 @@
>  
>  def getexecutable(filename):
>      mode = os.stat(filename).st_mode
> -    return (mode & stat.S_IXUSR) and (mode & stat.S_IXGRP) and (mode & \
> -        stat.S_IXOTH)
> +    return ((mode & stat.S_IXUSR) and 
> +            (mode & stat.S_IXGRP) and 
> +            (mode & stat.S_IXOTH))
>  
>  def getmode(executable):
>      if executable:
> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -92,8 +92,9 @@
>              continue
>  
>          if exact or not exists:
> -            if large or (lfsize and os.path.getsize(repo.wjoin(f)) >= \
> -                    lfsize * 1024 * 1024) or (lfmatcher and lfmatcher(f)):
> +            abovemin = (lfsize and
> +                        os.path.getsize(repo.wjoin(f)) >= lfsize * 1024 * 1024)
> +            if large or abovemin or (lfmatcher and lfmatcher(f)):
>                  lfnames.append(f)
>                  if ui.verbose or not exact:
>                      ui.status(_('adding %s as a largefile\n') % m.rel(f))
> @@ -117,8 +118,9 @@
>                  else:
>                      lfdirstate.add(f)
>              lfdirstate.write()
> -            bad += [lfutil.splitstandin(f) for f in lfutil.repo_add(repo,
> -                standins) if f in m.files()]
> +            bad += [lfutil.splitstandin(f) 
> +                    for f in lfutil.repo_add(repo, standins) 
> +                    if f in m.files()]
>      finally:
>          wlock.release()
>  
> @@ -143,8 +145,9 @@
>          s = repo.status(match=m, clean=True)
>      finally:
>          repo.lfstatus = False
> -    modified, added, deleted, clean = [[f for f in list if lfutil.standin(f) \
> -        in manifest] for list in [s[0], s[1], s[3], s[6]]]
> +    modified, added, deleted, clean = [[f for f in list 
> +                                        if lfutil.standin(f) in manifest] 
> +                                       for list in [s[0], s[1], s[3], s[6]]]
>  
>      def warn(files, reason):
>          for f in files:
> @@ -359,9 +362,10 @@
>              m._files = [lfutil.standin(f) for f in m._files if lfile(f)]
>              m._fmap = set(m._files)
>              orig_matchfn = m.matchfn
> -            m.matchfn = lambda f: lfutil.isstandin(f) and \
> -                lfile(lfutil.splitstandin(f)) and \
> -                orig_matchfn(lfutil.splitstandin(f)) or None
> +            m.matchfn = lambda f: (lfutil.isstandin(f) and
> +                                   lfile(lfutil.splitstandin(f)) and
> +                                   orig_matchfn(lfutil.splitstandin(f)) or
> +                                   None)
>              return m
>          oldmatch = installmatchfn(override_match)
>          listpats = []
> @@ -745,8 +749,8 @@
>              for f in mc:
>                  if mc[f] != mp1.get(f, None) or mc[f] != mp2.get(f, None):
>                      files.add(f)
> -        toupload = toupload.union(set([f for f in files if lfutil.isstandin(f)\
> -            and f in ctx]))
> +        toupload = toupload.union(
> +            set([f for f in files if lfutil.isstandin(f) and f in ctx]))
>      return toupload
>  
>  def override_outgoing(orig, ui, repo, dest=None, **opts):
> diff --git a/hgext/largefiles/proto.py b/hgext/largefiles/proto.py
> --- a/hgext/largefiles/proto.py
> +++ b/hgext/largefiles/proto.py
> @@ -12,9 +12,9 @@
>  
>  import lfutil
>  
> -LARGEFILES_REQUIRED_MSG = '\nThis repository uses the largefiles extension.' \
> -                          '\n\nPlease enable it in your Mercurial config ' \
> -                          'file.\n'
> +LARGEFILES_REQUIRED_MSG = ('\nThis repository uses the largefiles extension.'
> +                           '\n\nPlease enable it in your Mercurial config '
> +                           'file.\n')
>  
>  def putlfile(repo, proto, sha):
>      '''Put a largefile into a repository's local cache and into the
> diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
> --- a/hgext/largefiles/reposetup.py
> +++ b/hgext/largefiles/reposetup.py
> @@ -30,8 +30,8 @@
>          method = getattr(repo, name)
>          #if not (isinstance(method, types.MethodType) and
>          #        method.im_func is repo.__class__.commitctx.im_func):
> -        if isinstance(method, types.FunctionType) and method.func_name == \
> -            'wrap':
> +        if (isinstance(method, types.FunctionType) and 
> +            method.func_name == 'wrap'):
>              ui.warn(_('largefiles: repo method %r appears to have already been'
>                      ' wrapped by another extension: '
>                      'largefiles may behave incorrectly\n')
> @@ -193,16 +193,18 @@
>                      lfiles = (modified, added, removed, missing, [], [], clean)
>                      result = list(result)
>                      # Unknown files
> -                    result[4] = [f for f in unknown if repo.dirstate[f] == '?'\
> -                        and not lfutil.isstandin(f)]
> +                    result[4] = [f for f in unknown 
> +                                 if (repo.dirstate[f] == '?' and 
> +                                     not lfutil.isstandin(f))]
>                      # Ignored files must be ignored by both the dirstate and
>                      # lfdirstate
>                      result[5] = set(ignored).intersection(set(result[5]))
>                      # combine normal files and largefiles
> -                    normals = [[fn for fn in filelist if not \
> -                        lfutil.isstandin(fn)] for filelist in result]
> -                    result = [sorted(list1 + list2) for (list1, list2) in \
> -                        zip(normals, lfiles)]
> +                    normals = [[fn for fn in filelist 
> +                                if not lfutil.isstandin(fn)] 
> +                               for filelist in result]
> +                    result = [sorted(list1 + list2) 
> +                              for (list1, list2) in zip(normals, lfiles)]
>                  else:
>                      def toname(f):
>                          if lfutil.isstandin(f):
> @@ -250,8 +252,8 @@
>                      lfcommands.updatelfiles(repo.ui, repo)
>                  # Case 1: user calls commit with no specific files or
>                  # include/exclude patterns: refresh and commit everything.
> -                if (match is None) or (not match.anypats() and not \
> -                        match.files()):
> +                if ((match is None) or 
> +                    (not match.anypats() and not match.files())):
>                      lfiles = lfutil.listlfiles(self)
>                      lfdirstate = lfutil.openlfdirstate(ui, self)
>                      # this only loops through largefiles that exist
> @@ -333,8 +335,8 @@
>                          fstandin += os.sep
>  
>                      # prevalidate matching standin directories
> -                    if lfutil.any_(st for st in match._files if \
> -                            st.startswith(fstandin)):
> +                    if lfutil.any_(st for st in match._files 
> +                                   if st.startswith(fstandin)):
>                          continue
>                      actualfiles.append(f)
>                  match._files = actualfiles
> @@ -357,8 +359,8 @@
>                  toupload = set()
>                  o = repo.changelog.nodesbetween(o, revs)[0]
>                  for n in o:
> -                    parents = [p for p in repo.changelog.parents(n) if p != \
> -                        node.nullid]
> +                    parents = [p for p in repo.changelog.parents(n) 
> +                               if p != node.nullid]
>                      ctx = repo[n]
>                      files = set(ctx.files())
>                      if len(parents) == 2:
> @@ -376,8 +378,10 @@
>                                      None):
>                                  files.add(f)
>  
> -                    toupload = toupload.union(set([ctx[f].data().strip() for f\
> -                        in files if lfutil.isstandin(f) and f in ctx]))
> +                    toupload = toupload.union(
> +                        set([ctx[f].data().strip() 
> +                             for f in files 
> +                             if lfutil.isstandin(f) and f in ctx]))
>                  lfcommands.uploadlfiles(ui, self, remote, toupload)
>              return super(lfiles_repo, self).push(remote, force, revs,
>                  newbranch)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list