This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8886

mergeresult: introduce getfile() and use it where required
ClosedPublic

Authored by pulkit on Aug 5 2020, 8:11 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG4c6004afd836: mergeresult: introduce getfile() and use it where required
Summary

We want to hide the underlying dictionary from the users and provide API for
valid and sane use cases.

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.Aug 5 2020, 8:11 AM
pulkit updated this revision to Diff 22307.Aug 6 2020, 7:16 AM
indygreg accepted this revision.Aug 8 2020, 12:30 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
mercurial/merge.py
622

This pattern is better expressed as self._filemapping.get(filename, default_return) since it avoids an extra lookup.

Please submit a follow-up if you think this code is performance sensitive.

This revision is now accepted and ready to land.Aug 8 2020, 12:30 PM

Revision Contents
Changeset List

PathPackages
Mhgext/largefiles/overrides.py (4 lines)
Mmercurial/merge.py (18 lines)

Diff 22344

hgext/largefiles/overrides.py

splitstandin = lfutil.splitstandin(f) splitstandin = lfutil.splitstandin(f)
if splitstandin is not None and splitstandin in p1: if splitstandin is not None and splitstandin in p1:
lfiles.add(splitstandin) lfiles.add(splitstandin)
elif lfutil.standin(f) in p1: elif lfutil.standin(f) in p1:
lfiles.add(f) lfiles.add(f)
for lfile in sorted(lfiles): for lfile in sorted(lfiles):
standin = lfutil.standin(lfile) standin = lfutil.standin(lfile)
(lm, largs, lmsg) = mresult.actions.get(lfile, (None, None, None)) (lm, largs, lmsg) = mresult.getfile(lfile, (None, None, None))
(sm, sargs, smsg) = mresult.actions.get(standin, (None, None, None)) (sm, sargs, smsg) = mresult.getfile(standin, (None, None, None))
if sm in (b'g', b'dc') and lm != b'r': if sm in (b'g', b'dc') and lm != b'r':
if sm == b'dc': if sm == b'dc':
f1, f2, fa, move, anc = sargs f1, f2, fa, move, anc = sargs
sargs = (p2[f2].flags(), False) sargs = (p2[f2].flags(), False)
# Case 1: normal file in the working copy, largefile in # Case 1: normal file in the working copy, largefile in
# the second parent # the second parent
usermsg = ( usermsg = (
_( _(

mercurial/merge.py

# A file is in a directory which aliases both a local # A file is in a directory which aliases both a local
# and a remote file. This is an internal inconsistency # and a remote file. This is an internal inconsistency
# within the remote manifest. # within the remote manifest.
invalidconflicts.add(p) invalidconflicts.add(p)
else: else:
# A file is in a directory which aliases a local file. # A file is in a directory which aliases a local file.
# We will need to rename the local file. # We will need to rename the local file.
localconflicts.add(p) localconflicts.add(p)
if p in mresult.actions and mresult.actions[p][0] in ( pd = mresult.getfile(p)
if pd and pd[0] in (
mergestatemod.ACTION_CREATED, mergestatemod.ACTION_CREATED,
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
mergestatemod.ACTION_MERGE, mergestatemod.ACTION_MERGE,
mergestatemod.ACTION_CREATED_MERGE, mergestatemod.ACTION_CREATED_MERGE,
): ):
# The file is in a directory which aliases a remote file. # The file is in a directory which aliases a remote file.
# This is an internal inconsistency within the remote # This is an internal inconsistency within the remote
# manifest. # manifest.
b'path conflict', b'path conflict',
) )
if remoteconflicts: if remoteconflicts:
# Check if all files in the conflicting directories have been removed. # Check if all files in the conflicting directories have been removed.
ctxname = bytes(mctx).rstrip(b'+') ctxname = bytes(mctx).rstrip(b'+')
for f, p in _filesindirs(repo, mf, remoteconflicts): for f, p in _filesindirs(repo, mf, remoteconflicts):
if f not in deletedfiles: if f not in deletedfiles:
m, args, msg = mresult.actions[p] m, args, msg = mresult.getfile(p)
pnew = util.safename( pnew = util.safename(
p, ctxname, wctx, set(mresult.actions.keys()) p, ctxname, wctx, set(mresult.actions.keys())
) )
if m in ( if m in (
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
mergestatemod.ACTION_MERGE, mergestatemod.ACTION_MERGE,
): ):
# Action was merge, just update target. # Action was merge, just update target.
# entry form _actionmapping too # entry form _actionmapping too
if filename in self._filemapping: if filename in self._filemapping:
a, d, m = self._filemapping[filename] a, d, m = self._filemapping[filename]
del self._actionmapping[a][filename] del self._actionmapping[a][filename]
self._filemapping[filename] = (action, data, message) self._filemapping[filename] = (action, data, message)
self._actionmapping[action][filename] = (data, message) self._actionmapping[action][filename] = (data, message)
def getfile(self, filename, default_return=None):
""" returns (action, args, msg) about this file
returns default_return if the file is not present """
if filename in self._filemapping:
return self._filemapping[filename]
return default_return
indygregUnsubmitted
Not Done

This pattern is better expressed as self._filemapping.get(filename, default_return) since it avoids an extra lookup.

Please submit a follow-up if you think this code is performance sensitive.

indygreg: This pattern is better expressed as `self._filemapping.get(filename, default_return)` since it…
def removefile(self, filename): def removefile(self, filename):
""" removes a file from the mergeresult object as the file might """ removes a file from the mergeresult object as the file might
not merging anymore """ not merging anymore """
action, data, message = self._filemapping[filename] action, data, message = self._filemapping[filename]
del self._filemapping[filename] del self._filemapping[filename]
del self._actionmapping[action][filename] del self._actionmapping[action][filename]
def getactions(self, actions, sort=False): def getactions(self, actions, sort=False):
if mresult.hasconflicts(): if mresult.hasconflicts():
msg = _(b"conflicting changes") msg = _(b"conflicting changes")
hint = _(b"commit or update --clean to discard changes") hint = _(b"commit or update --clean to discard changes")
raise error.Abort(msg, hint=hint) raise error.Abort(msg, hint=hint)
# Prompt and create actions. Most of this is in the resolve phase # Prompt and create actions. Most of this is in the resolve phase
# already, but we can't handle .hgsubstate in filemerge or # already, but we can't handle .hgsubstate in filemerge or
# subrepoutil.submerge yet so we have to keep prompting for it. # subrepoutil.submerge yet so we have to keep prompting for it.
if b'.hgsubstate' in mresult.actions: vals = mresult.getfile(b'.hgsubstate')
if vals:
f = b'.hgsubstate' f = b'.hgsubstate'
m, args, msg = mresult.actions[f] m, args, msg = vals
prompts = filemerge.partextras(labels) prompts = filemerge.partextras(labels)
prompts[b'f'] = f prompts[b'f'] = f
if m == mergestatemod.ACTION_CHANGED_DELETED: if m == mergestatemod.ACTION_CHANGED_DELETED:
if repo.ui.promptchoice( if repo.ui.promptchoice(
_( _(
b"local%(l)s changed %(f)s which other%(o)s deleted\n" b"local%(l)s changed %(f)s which other%(o)s deleted\n"
b"use (c)hanged version or (d)elete?" b"use (c)hanged version or (d)elete?"
b"$$ &Changed $$ &Delete" b"$$ &Changed $$ &Delete"