This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9832

simplemerge: delete unused find_unconflicted()
ClosedPublic

Authored by martinvonz on Jan 19 2021, 1:36 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG98e3a693061a: simplemerge: delete unused find_unconflicted()
Summary

The function has been unused ever since it was introduced in
465b9ea02868 (Import 3-way merge code from bzr, 2007-04-16).

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

martinvonz created this revision.Jan 19 2021, 1:36 AM
mharbison72 added a subscriber: mharbison72.Jan 19 2021, 10:01 AM

Was its purpose to be used in the tests? (It probably should have been put there in that case, but maybe importing mdiff there is more complicated?)

martinvonz added a comment.Jan 19 2021, 11:49 AM
In D9832#148843, @mharbison72 wrote:

Was its purpose to be used in the tests? (It probably should have been put there in that case, but maybe importing mdiff there is more complicated?)

I'm not sure I follow. Do you mean that the test conditions that I removed in this patch might be worth keeping the function for? I had assumed that the function existed because it was used somewhere in Bazaar, but I didn't check if it actually was.

mharbison72 added a comment.Jan 19 2021, 11:58 AM
In D9832#148844, @martinvonz wrote:
In D9832#148843, @mharbison72 wrote:

Was its purpose to be used in the tests? (It probably should have been put there in that case, but maybe importing mdiff there is more complicated?)

I'm not sure I follow. Do you mean that the test conditions that I removed in this patch might be worth keeping the function for? I had assumed that the function existed because it was used somewhere in Bazaar, but I didn't check if it actually was.

I'm not sure what the test conditions were trying to assert, TBH. I just meant that presumably they were doing something, like asserting the state of the already-performed merge?

martinvonz added a comment.Jan 19 2021, 12:13 PM
In D9832#148845, @mharbison72 wrote:
In D9832#148844, @martinvonz wrote:
In D9832#148843, @mharbison72 wrote:

Was its purpose to be used in the tests? (It probably should have been put there in that case, but maybe importing mdiff there is more complicated?)

I'm not sure I follow. Do you mean that the test conditions that I removed in this patch might be worth keeping the function for? I had assumed that the function existed because it was used somewhere in Bazaar, but I didn't check if it actually was.

I'm not sure what the test conditions were trying to assert, TBH. I just meant that presumably they were doing something, like asserting the state of the already-performed merge?

I think they just test that find_unconflicted() returns the right values. If you look at the test case names, they seem to refer to the state that they're testing, not to the function name. I think they set up a particular conflict and then test that find_unconflicted() and
find_sync_regions() return the expected output.

martinvonz added a commit: rHG98e3a693061a: simplemerge: delete unused find_unconflicted().Jan 20 2021, 3:24 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG98e3a693061a: simplemerge: delete unused find_unconflicted() (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/simplemerge.py (25 lines)
Mtests/test-simplemerge.py (10 lines)

Diff 25181

mercurial/simplemerge.py

intbase = len(self.base) intbase = len(self.base)
abase = len(self.a) abase = len(self.a)
bbase = len(self.b) bbase = len(self.b)
sl.append((intbase, intbase, abase, abase, bbase, bbase)) sl.append((intbase, intbase, abase, abase, bbase, bbase))
return sl return sl
def find_unconflicted(self):
"""Return a list of ranges in base that are not conflicted."""
am = mdiff.get_matching_blocks(self.basetext, self.atext)
bm = mdiff.get_matching_blocks(self.basetext, self.btext)
unc = []
while am and bm:
# there is an unconflicted block at i; how long does it
# extend? until whichever one ends earlier.
a1 = am[0][0]
a2 = a1 + am[0][2]
b1 = bm[0][0]
b2 = b1 + bm[0][2]
i = intersect((a1, a2), (b1, b2))
if i:
unc.append(i)
if a2 < b2:
del am[0]
else:
del bm[0]
return unc
def _verifytext(text, path, ui, opts): def _verifytext(text, path, ui, opts):
"""verifies that text is non-binary (unless opts[text] is passed, """verifies that text is non-binary (unless opts[text] is passed,
then we just warn)""" then we just warn)"""
if stringutil.binary(text): if stringutil.binary(text):
msg = _(b"%s looks like a binary file.") % path msg = _(b"%s looks like a binary file.") % path
if not opts.get('quiet'): if not opts.get('quiet'):
ui.warn(_(b'warning: %s\n') % msg) ui.warn(_(b'warning: %s\n') % msg)

tests/test-simplemerge.py

class TestMerge3(TestCase): class TestMerge3(TestCase):
def log(self, msg): def log(self, msg):
pass pass
def test_no_changes(self): def test_no_changes(self):
"""No conflicts because nothing changed""" """No conflicts because nothing changed"""
m3 = Merge3([b'aaa', b'bbb'], [b'aaa', b'bbb'], [b'aaa', b'bbb']) m3 = Merge3([b'aaa', b'bbb'], [b'aaa', b'bbb'], [b'aaa', b'bbb'])
self.assertEqual(m3.find_unconflicted(), [(0, 2)])
self.assertEqual( self.assertEqual(
list(m3.find_sync_regions()), list(m3.find_sync_regions()),
[(0, 2, 0, 2, 0, 2), (2, 2, 2, 2, 2, 2)], [(0, 2, 0, 2, 0, 2), (2, 2, 2, 2, 2, 2)],
) )
self.assertEqual(list(m3.merge_regions()), [(b'unchanged', 0, 2)]) self.assertEqual(list(m3.merge_regions()), [(b'unchanged', 0, 2)])
self.assertEqual( self.assertEqual(
self.assertEqual(list(m3.merge_lines()), [b'aaa', b'bbb']) self.assertEqual(list(m3.merge_lines()), [b'aaa', b'bbb'])
def test_no_conflicts(self): def test_no_conflicts(self):
"""No conflicts because only one side changed""" """No conflicts because only one side changed"""
m3 = Merge3( m3 = Merge3(
[b'aaa', b'bbb'], [b'aaa', b'111', b'bbb'], [b'aaa', b'bbb'] [b'aaa', b'bbb'], [b'aaa', b'111', b'bbb'], [b'aaa', b'bbb']
) )
self.assertEqual(m3.find_unconflicted(), [(0, 1), (1, 2)])
self.assertEqual( self.assertEqual(
list(m3.find_sync_regions()), list(m3.find_sync_regions()),
[(0, 1, 0, 1, 0, 1), (1, 2, 2, 3, 1, 2), (2, 2, 3, 3, 2, 2)], [(0, 1, 0, 1, 0, 1), (1, 2, 2, 3, 1, 2), (2, 2, 3, 3, 2, 2)],
) )
self.assertEqual( self.assertEqual(
list(m3.merge_regions()), list(m3.merge_regions()),
[(b'unchanged', 0, 1), (b'a', 1, 2), (b'unchanged', 1, 2)], [(b'unchanged', 0, 1), (b'a', 1, 2), (b'unchanged', 1, 2)],
def test_insert_clash(self): def test_insert_clash(self):
"""Both try to insert lines in the same place.""" """Both try to insert lines in the same place."""
m3 = Merge3( m3 = Merge3(
[b'aaa\n', b'bbb\n'], [b'aaa\n', b'bbb\n'],
[b'aaa\n', b'111\n', b'bbb\n'], [b'aaa\n', b'111\n', b'bbb\n'],
[b'aaa\n', b'222\n', b'bbb\n'], [b'aaa\n', b'222\n', b'bbb\n'],
) )
self.assertEqual(m3.find_unconflicted(), [(0, 1), (1, 2)])
self.assertEqual( self.assertEqual(
list(m3.find_sync_regions()), list(m3.find_sync_regions()),
[(0, 1, 0, 1, 0, 1), (1, 2, 2, 3, 2, 3), (2, 2, 3, 3, 3, 3)], [(0, 1, 0, 1, 0, 1), (1, 2, 2, 3, 2, 3), (2, 2, 3, 3, 3, 3)],
) )
self.assertEqual( self.assertEqual(
list(m3.merge_regions()), list(m3.merge_regions()),
[ [
def test_replace_clash(self): def test_replace_clash(self):
"""Both try to insert lines in the same place.""" """Both try to insert lines in the same place."""
m3 = Merge3( m3 = Merge3(
[b'aaa', b'000', b'bbb'], [b'aaa', b'000', b'bbb'],
[b'aaa', b'111', b'bbb'], [b'aaa', b'111', b'bbb'],
[b'aaa', b'222', b'bbb'], [b'aaa', b'222', b'bbb'],
) )
self.assertEqual(m3.find_unconflicted(), [(0, 1), (2, 3)])
self.assertEqual( self.assertEqual(
list(m3.find_sync_regions()), list(m3.find_sync_regions()),
[(0, 1, 0, 1, 0, 1), (2, 3, 2, 3, 2, 3), (3, 3, 3, 3, 3, 3)], [(0, 1, 0, 1, 0, 1), (2, 3, 2, 3, 2, 3), (3, 3, 3, 3, 3, 3)],
) )
def test_replace_multi(self): def test_replace_multi(self):
"""Replacement with regions of different size.""" """Replacement with regions of different size."""
m3 = Merge3( m3 = Merge3(
[b'aaa', b'000', b'000', b'bbb'], [b'aaa', b'000', b'000', b'bbb'],
[b'aaa', b'111', b'111', b'111', b'bbb'], [b'aaa', b'111', b'111', b'111', b'bbb'],
[b'aaa', b'222', b'222', b'222', b'222', b'bbb'], [b'aaa', b'222', b'222', b'222', b'222', b'bbb'],
) )
self.assertEqual(m3.find_unconflicted(), [(0, 1), (3, 4)])
self.assertEqual( self.assertEqual(
list(m3.find_sync_regions()), list(m3.find_sync_regions()),
[(0, 1, 0, 1, 0, 1), (3, 4, 4, 5, 5, 6), (4, 4, 5, 5, 6, 6)], [(0, 1, 0, 1, 0, 1), (3, 4, 4, 5, 5, 6), (4, 4, 5, 5, 6, 6)],
) )
def test_merge_poem(self): def test_merge_poem(self):
"""Test case from diff3 manual""" """Test case from diff3 manual"""
m3 = Merge3(TZU, LAO, TAO) m3 = Merge3(TZU, LAO, TAO)