This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9418

copies: clarify the return of _merge_copies_dict
ClosedPublic

Authored by Alphare on Nov 27 2020, 11:11 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGf9f8d8aa9a92: copies: clarify the return of _merge_copies_dict
rHG2f3516a0d942: copies: clarify the return of _merge_copies_dict
Summary

I misused that function twice in the past few days, so lets clarify the API.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Nov 27 2020, 11:11 AM
marmoute updated this revision to Diff 23816.Nov 29 2020, 6:13 AM
pulkit added a subscriber: pulkit.Nov 29 2020, 2:15 PM
pulkit added inline comments.
mercurial/copies.py
466

Is there any reason we will like to return this?

marmoute added a subscriber: marmoute.Nov 30 2020, 4:27 AM
marmoute added inline comments.
mercurial/copies.py
466

Absolutely. It allow us to (update and) return whichever of the two dicts we prefer depending on the situation.

(it also make the code a bit clearer, since we don't have to remember which of the two dict argument get updated)

pulkit accepted this revision.Nov 30 2020, 11:39 AM
This revision is now accepted and ready to land.Nov 30 2020, 11:39 AM
marmoute added a commit: rHG2f3516a0d942: copies: clarify the return of _merge_copies_dict.Nov 30 2020, 12:24 PM
Closed by commit rHG2f3516a0d942: copies: clarify the return of _merge_copies_dict (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/copies.py (7 lines)

Diff 23816

mercurial/copies.py

# In case of conflict, parent 1 take precedence over parent 2. # In case of conflict, parent 1 take precedence over parent 2.
# This is an arbitrary choice made anew when implementing # This is an arbitrary choice made anew when implementing
# changeset based copies. It was made without regards with # changeset based copies. It was made without regards with
# potential filelog related behavior. # potential filelog related behavior.
if parent == 1: if parent == 1:
minor, major = othercopies, newcopies minor, major = othercopies, newcopies
else: else:
minor, major = newcopies, othercopies minor, major = newcopies, othercopies
_merge_copies_dict(minor, major, isancestor, changes) copies = _merge_copies_dict(minor, major, isancestor, changes)
all_copies[c] = minor all_copies[c] = copies
final_copies = {} final_copies = {}
for dest, (tt, source) in all_copies[targetrev].items(): for dest, (tt, source) in all_copies[targetrev].items():
if source is not None: if source is not None:
final_copies[dest] = source final_copies[dest] = source
return final_copies return final_copies
def _merge_copies_dict(minor, major, isancestor, changes): def _merge_copies_dict(minor, major, isancestor, changes):
"""merge two copies-mapping together, minor and major """merge two copies-mapping together, minor and major
In case of conflict, value from "major" will be picked. In case of conflict, value from "major" will be picked.
- `isancestors(low_rev, high_rev)`: callable return True if `low_rev` is an - `isancestors(low_rev, high_rev)`: callable return True if `low_rev` is an
ancestors of `high_rev`, ancestors of `high_rev`,
- `ismerged(path)`: callable return True if `path` have been merged in the - `ismerged(path)`: callable return True if `path` have been merged in the
current revision, current revision,
return the resulting dict (in practice, the "minor" object, updated)
""" """
for dest, value in major.items(): for dest, value in major.items():
other = minor.get(dest) other = minor.get(dest)
if other is None: if other is None:
minor[dest] = value minor[dest] = value
else: else:
new_tt = value[0] new_tt = value[0]
other_tt = other[0] other_tt = other[0]
minor[dest] = value minor[dest] = value
elif changes is not None and dest in changes.merged: elif changes is not None and dest in changes.merged:
minor[dest] = value minor[dest] = value
elif not isancestor(new_tt, other_tt): elif not isancestor(new_tt, other_tt):
if value[1] is not None: if value[1] is not None:
minor[dest] = value minor[dest] = value
elif isancestor(other_tt, new_tt): elif isancestor(other_tt, new_tt):
minor[dest] = value minor[dest] = value
return minor
pulkitUnsubmitted
Not Done

Is there any reason we will like to return this?

pulkit: Is there any reason we will like to return this?
marmouteUnsubmitted
Not Done

Absolutely. It allow us to (update and) return whichever of the two dicts we prefer depending on the situation.

(it also make the code a bit clearer, since we don't have to remember which of the two dict argument get updated)

marmoute: Absolutely. It allow us to (update and) return whichever of the two dicts we prefer depending…
def _revinfo_getter_extra(repo): def _revinfo_getter_extra(repo):
"""return a function that return multiple data given a <rev>"i """return a function that return multiple data given a <rev>"i
* p1: revision number of first parent * p1: revision number of first parent
* p2: revision number of first parent * p2: revision number of first parent
* p1copies: mapping of copies from p1 * p1copies: mapping of copies from p1