This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4588

py3: fix kwargs handling in hgext/fastannotate.py
ClosedPublic

Authored by pulkit on Sep 14 2018, 5:42 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGb77f2ea51da5: py3: fix kwargs handling in hgext/fastannotate.py

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Sep 14 2018, 5:42 PM
indygreg requested changes to this revision.Sep 14 2018, 6:13 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
hgext/fastannotate/commands.py
212–217

Shouldn't we be doing the `pycompat.byteskwargs() at the beginning of the function and using regular string literals on the keys we set?

247–258

Ditto.

This revision now requires changes to proceed.Sep 14 2018, 6:13 PM
pulkit added inline comments.Sep 16 2018, 11:49 AM
hgext/fastannotate/commands.py
212–217

We can do that, but then in the end we have to do pycompat.strkwargs() again to pass into orig() into line 220 below. So I am preventing a conversion here is rev is None.

pulkit updated this revision to Diff 11101.Sep 16 2018, 11:54 AM
Closed by commit rHGb77f2ea51da5: py3: fix kwargs handling in hgext/fastannotate.py (authored by pulkit). · Explain WhySep 18 2018, 9:47 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/fastannotate/commands.py (9 lines)

Diff 11139

hgext/fastannotate/commands.py

""" """
if not pats: if not pats:
raise error.Abort(_('at least one filename or pattern is required')) raise error.Abort(_('at least one filename or pattern is required'))
# performance hack: filtered repo can be slow. unfilter by default. # performance hack: filtered repo can be slow. unfilter by default.
if ui.configbool('fastannotate', 'unfilteredrepo'): if ui.configbool('fastannotate', 'unfilteredrepo'):
repo = repo.unfiltered() repo = repo.unfiltered()
opts = pycompat.byteskwargs(opts)
rev = opts.get('rev', '.') rev = opts.get('rev', '.')
rebuild = opts.get('rebuild', False) rebuild = opts.get('rebuild', False)
diffopts = patch.difffeatureopts(ui, opts, section='annotate', diffopts = patch.difffeatureopts(ui, opts, section='annotate',
whitespace=True) whitespace=True)
aopts = facontext.annotateopts( aopts = facontext.annotateopts(
diffopts=diffopts, diffopts=diffopts,
followmerge=not opts.get('linear', False), followmerge=not opts.get('linear', False),
def _annotatewrapper(orig, ui, repo, *pats, **opts): def _annotatewrapper(orig, ui, repo, *pats, **opts):
"""used by wrapdefault""" """used by wrapdefault"""
# we need this hack until the obsstore has 0.0 seconds perf impact # we need this hack until the obsstore has 0.0 seconds perf impact
if ui.configbool('fastannotate', 'unfilteredrepo'): if ui.configbool('fastannotate', 'unfilteredrepo'):
repo = repo.unfiltered() repo = repo.unfiltered()
# treat the file as text (skip the isbinary check) # treat the file as text (skip the isbinary check)
if ui.configbool('fastannotate', 'forcetext'): if ui.configbool('fastannotate', 'forcetext'):
opts['text'] = True opts[r'text'] = True
# check if we need to do prefetch (client-side) # check if we need to do prefetch (client-side)
rev = opts.get('rev') rev = opts.get(r'rev')
if util.safehasattr(repo, 'prefetchfastannotate') and rev is not None: if util.safehasattr(repo, 'prefetchfastannotate') and rev is not None:
paths = list(_matchpaths(repo, rev, pats, opts)) paths = list(_matchpaths(repo, rev, pats, pycompat.byteskwargs(opts)))
indygregUnsubmitted
Not Done

Shouldn't we be doing the `pycompat.byteskwargs() at the beginning of the function and using regular string literals on the keys we set?

indygreg: Shouldn't we be doing the ``pycompat.byteskwargs()` at the beginning of the function and using…
pulkitAuthorUnsubmitted
Not Done

We can do that, but then in the end we have to do pycompat.strkwargs() again to pass into orig() into line 220 below. So I am preventing a conversion here is rev is None.

pulkit: We can do that, but then in the end we have to do `pycompat.strkwargs()` again to pass into…
repo.prefetchfastannotate(paths) repo.prefetchfastannotate(paths)
return orig(ui, repo, *pats, **opts) return orig(ui, repo, *pats, **opts)
def registercommand(): def registercommand():
"""register the fastannotate command""" """register the fastannotate command"""
name = '^fastannotate|fastblame|fa' name = '^fastannotate|fastblame|fa'
command(name, **fastannotatecommandargs)(fastannotate) command(name, **fastannotatecommandargs)(fastannotate)
If REV is not specified, use the config 'fastannotate.mainbranch'. If REV is not specified, use the config 'fastannotate.mainbranch'.
If fastannotate.client is True, download the annotate cache from the If fastannotate.client is True, download the annotate cache from the
server. Otherwise, build the annotate cache locally. server. Otherwise, build the annotate cache locally.
The annotate cache will be built using the default diff and follow The annotate cache will be built using the default diff and follow
options and lives in '.hg/fastannotate/default'. options and lives in '.hg/fastannotate/default'.
""" """
opts = pycompat.byteskwargs(opts)
rev = opts.get('REV') or ui.config('fastannotate', 'mainbranch') rev = opts.get('REV') or ui.config('fastannotate', 'mainbranch')
if not rev: if not rev:
raise error.Abort(_('you need to provide a revision'), raise error.Abort(_('you need to provide a revision'),
hint=_('set fastannotate.mainbranch or use --rev')) hint=_('set fastannotate.mainbranch or use --rev'))
if ui.configbool('fastannotate', 'unfilteredrepo'): if ui.configbool('fastannotate', 'unfilteredrepo'):
repo = repo.unfiltered() repo = repo.unfiltered()
ctx = scmutil.revsingle(repo, rev) ctx = scmutil.revsingle(repo, rev)
m = scmutil.match(ctx, pats, opts) m = scmutil.match(ctx, pats, opts)
paths = list(ctx.walk(m)) paths = list(ctx.walk(m))
if util.safehasattr(repo, 'prefetchfastannotate'): if util.safehasattr(repo, 'prefetchfastannotate'):
# client # client
if opts.get('REV'): if opts.get('REV'):
indygregUnsubmitted
Not Done

Ditto.

indygreg: Ditto.
raise error.Abort(_('--rev cannot be used for client')) raise error.Abort(_('--rev cannot be used for client'))
repo.prefetchfastannotate(paths) repo.prefetchfastannotate(paths)
else: else:
# server, or full repo # server, or full repo
for i, path in enumerate(paths): for i, path in enumerate(paths):
ui.progress(_('building'), i, total=len(paths)) ui.progress(_('building'), i, total=len(paths))
with facontext.annotatecontext(repo, path) as actx: with facontext.annotatecontext(repo, path) as actx:
try: try: