[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