( )⚙ D8502 cleanup: avoid extra node/ctx conversions in logcmdutil.diffordiffstat

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8502

cleanup: avoid extra node/ctx conversions in logcmdutil.diffordiffstat
ClosedPublic

Authored by durin42 on May 7 2020, 5:25 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG3b7aabd02e11: cleanup: avoid extra node/ctx conversions in logcmdutil.diffordiffstat
Summary

I'm about to write some code that wants to pass a memctx to
diffordiffstat, but this feels like a meritorious cleanup anyway,
since the first thing this method does is turn nodes into contexts,
and most callers have a context handy.

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

durin42 created this revision.May 7 2020, 5:25 PM
durin42 updated this revision to Diff 21339.May 11 2020, 5:34 PM
martinvonz added a subscriber: martinvonz.May 12 2020, 2:21 AM

Mention API change in relnotes/next?

mercurial/commands.py
2492–2495

Can avoid converting to nodes and back by changing this

2492–2495

Oh, next patch does that...

indygreg accepted this revision.May 12 2020, 8:17 PM
This revision is now accepted and ready to land.May 12 2020, 8:17 PM

Revision Contents
Changeset List

PathPackages
Mhgext/mq.py (10 lines)
Mmercurial/commands.py (4 lines)
Mmercurial/logcmdutil.py (16 lines)
Mmercurial/subrepo.py (4 lines)

Diff 21366

hgext/mq.py

changes=None, changes=None,
opts=None, opts=None,
): ):
if opts is None: if opts is None:
opts = {} opts = {}
stat = opts.get(b'stat') stat = opts.get(b'stat')
m = scmutil.match(repo[node1], files, opts) m = scmutil.match(repo[node1], files, opts)
logcmdutil.diffordiffstat( logcmdutil.diffordiffstat(
self.ui, repo, diffopts, node1, node2, m, changes, stat, fp self.ui,
repo,
diffopts,
repo[node1],
repo[node2],
m,
changes,
stat,
fp,
) )
def mergeone(self, repo, mergeq, head, patch, rev, diffopts): def mergeone(self, repo, mergeq, head, patch, rev, diffopts):
# first try just applying the patch # first try just applying the patch
(err, n) = self.apply( (err, n) = self.apply(
repo, [patch], update_status=False, strict=True, merge=rev repo, [patch], update_status=False, strict=True, merge=rev
) )

mercurial/commands.py

if change: if change:
repo = scmutil.unhidehashlikerevs(repo, [change], b'nowarn') repo = scmutil.unhidehashlikerevs(repo, [change], b'nowarn')
ctx2 = scmutil.revsingle(repo, change, None) ctx2 = scmutil.revsingle(repo, change, None)
ctx1 = ctx2.p1() ctx1 = ctx2.p1()
else: else:
repo = scmutil.unhidehashlikerevs(repo, revs, b'nowarn') repo = scmutil.unhidehashlikerevs(repo, revs, b'nowarn')
ctx1, ctx2 = scmutil.revpair(repo, revs) ctx1, ctx2 = scmutil.revpair(repo, revs)
node1, node2 = ctx1.node(), ctx2.node() node1, node2 = ctx1.node(), ctx2.node()
if reverse: if reverse:
node1, node2 = node2, node1 node1, node2 = node2, node1
martinvonzUnsubmitted
Not Done

Can avoid converting to nodes and back by changing this

martinvonz: Can avoid converting to nodes and back by changing this
martinvonzUnsubmitted
Not Done

Oh, next patch does that...

martinvonz: Oh, next patch does that...
diffopts = patch.diffallopts(ui, opts) diffopts = patch.diffallopts(ui, opts)
m = scmutil.match(ctx2, pats, opts) m = scmutil.match(ctx2, pats, opts)
m = repo.narrowmatch(m) m = repo.narrowmatch(m)
ui.pager(b'diff') ui.pager(b'diff')
logcmdutil.diffordiffstat( logcmdutil.diffordiffstat(
ui, ui,
repo, repo,
diffopts, diffopts,
node1, repo[node1],
node2, repo[node2],
m, m,
stat=stat, stat=stat,
listsubrepos=opts.get(b'subrepos'), listsubrepos=opts.get(b'subrepos'),
root=opts.get(b'root'), root=opts.get(b'root'),
) )
@command( @command(

mercurial/logcmdutil.py

limit = None limit = None
return limit return limit
def diffordiffstat( def diffordiffstat(
ui, ui,
repo, repo,
diffopts, diffopts,
node1, ctx1,
node2, ctx2,
match, match,
changes=None, changes=None,
stat=False, stat=False,
fp=None, fp=None,
graphwidth=0, graphwidth=0,
prefix=b'', prefix=b'',
root=b'', root=b'',
listsubrepos=False, listsubrepos=False,
hunksfilterfn=None, hunksfilterfn=None,
): ):
'''show diff or diffstat.''' '''show diff or diffstat.'''
ctx1 = repo[node1]
ctx2 = repo[node2]
if root: if root:
relroot = pathutil.canonpath(repo.root, repo.getcwd(), root) relroot = pathutil.canonpath(repo.root, repo.getcwd(), root)
else: else:
relroot = b'' relroot = b''
copysourcematch = None copysourcematch = None
def compose(f, g): def compose(f, g):
return lambda x: f(g(x)) return lambda x: f(g(x))
yield ui.label(chunk, label=label) yield ui.label(chunk, label=label)
for chunk in util.filechunkiter(util.chunkbuffer(gen())): for chunk in util.filechunkiter(util.chunkbuffer(gen())):
ui.write(chunk) ui.write(chunk)
else: else:
for chunk, label in chunks: for chunk, label in chunks:
ui.write(chunk, label=label) ui.write(chunk, label=label)
node2 = ctx2.node()
for subpath, sub in scmutil.itersubrepos(ctx1, ctx2): for subpath, sub in scmutil.itersubrepos(ctx1, ctx2):
tempnode2 = node2 tempnode2 = node2
try: try:
if node2 is not None: if node2 is not None:
tempnode2 = ctx2.substate[subpath][1] tempnode2 = ctx2.substate[subpath][1]
except KeyError: except KeyError:
# A subrepo that existed in node1 was deleted between node1 and # A subrepo that existed in node1 was deleted between node1 and
# node2 (inclusive). Thus, ctx2's substate won't contain that # node2 (inclusive). Thus, ctx2's substate won't contain that
def _makefilematcher(self, ctx): def _makefilematcher(self, ctx):
return scmutil.matchall(ctx.repo()) return scmutil.matchall(ctx.repo())
def _makehunksfilter(self, ctx): def _makehunksfilter(self, ctx):
return None return None
def showdiff(self, ui, ctx, diffopts, graphwidth=0, stat=False): def showdiff(self, ui, ctx, diffopts, graphwidth=0, stat=False):
repo = ctx.repo()
node = ctx.node()
prev = ctx.p1().node()
diffordiffstat( diffordiffstat(
ui, ui,
repo, ctx.repo(),
diffopts, diffopts,
prev, ctx.p1(),
node, ctx,
match=self._makefilematcher(ctx), match=self._makefilematcher(ctx),
stat=stat, stat=stat,
graphwidth=graphwidth, graphwidth=graphwidth,
hunksfilterfn=self._makehunksfilter(ctx), hunksfilterfn=self._makehunksfilter(ctx),
) )
def changesetlabels(ctx): def changesetlabels(ctx):

mercurial/subrepo.py

# We currently expect node2 to come from substate and be # We currently expect node2 to come from substate and be
# in hex format # in hex format
if node2 is not None: if node2 is not None:
node2 = node.bin(node2) node2 = node.bin(node2)
logcmdutil.diffordiffstat( logcmdutil.diffordiffstat(
ui, ui,
self._repo, self._repo,
diffopts, diffopts,
node1, self._repo[node1],
node2, self._repo[node2],
match, match,
prefix=prefix, prefix=prefix,
listsubrepos=True, listsubrepos=True,
**opts **opts
) )
except error.RepoLookupError as inst: except error.RepoLookupError as inst:
self.ui.warn( self.ui.warn(
_(b'warning: error "%s" in subrepository "%s"\n') _(b'warning: error "%s" in subrepository "%s"\n')