Status: | Merged |
---|---|
Approved by: | Martin Packman |
Approved revision: | no longer in the source branch. |
Merge reported by: | The Breezy Bot |
Merged at revision: | not available |
Proposed branch: | lp:~gz/brz/lt_by_dirs |
Merge into: | lp:brz |
Diff against target: |
246 lines (+48/-52) 4 files modified
breezy/_dirstate_helpers_py.py (+6/-8) breezy/_dirstate_helpers_pyx.pyx (+9/-11) breezy/dirstate.py (+5/-5) breezy/tests/test__dirstate_helpers.py (+28/-28) |
To merge this branch: | bzr merge lp:~gz/brz/lt_by_dirs |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jelmer Vernooij | Approve | ||
Review via email: mp+325435@code.launchpad.net |
Commit message
Replace dirstate helpers cmp_by_dirs with lt_by_dirs
Description of the change
Change dirstate helpers function cmp_by_dirs into lt_by_dirs. It's only used as lt in the dirstate code, and Python 3 doesn't have a builtin cmp.
To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'breezy/_dirstate_helpers_py.py' | |||
2 | --- breezy/_dirstate_helpers_py.py 2017-06-04 18:09:30 +0000 | |||
3 | +++ breezy/_dirstate_helpers_py.py 2017-06-10 00:42:09 +0000 | |||
4 | @@ -163,22 +163,20 @@ | |||
5 | 163 | return lo | 163 | return lo |
6 | 164 | 164 | ||
7 | 165 | 165 | ||
9 | 166 | def cmp_by_dirs(path1, path2): | 166 | def lt_by_dirs(path1, path2): |
10 | 167 | """Compare two paths directory by directory. | 167 | """Compare two paths directory by directory. |
11 | 168 | 168 | ||
12 | 169 | This is equivalent to doing:: | 169 | This is equivalent to doing:: |
13 | 170 | 170 | ||
15 | 171 | cmp(path1.split('/'), path2.split('/')) | 171 | operator.lt(path1.split('/'), path2.split('/')) |
16 | 172 | 172 | ||
17 | 173 | The idea is that you should compare path components separately. This | 173 | The idea is that you should compare path components separately. This |
20 | 174 | differs from plain ``cmp(path1, path2)`` for paths like ``'a-b'`` and | 174 | differs from plain ``path1 < path2`` for paths like ``'a-b'`` and ``a/b``. |
21 | 175 | ``a/b``. "a-b" comes after "a" but would come before "a/b" lexically. | 175 | "a-b" comes after "a" but would come before "a/b" lexically. |
22 | 176 | 176 | ||
23 | 177 | :param path1: first path | 177 | :param path1: first path |
24 | 178 | :param path2: second path | 178 | :param path2: second path |
28 | 179 | :return: negative number if ``path1`` comes first, | 179 | :return: True if path1 comes first, otherwise False |
26 | 180 | 0 if paths are equal, | ||
27 | 181 | and positive number if ``path2`` sorts first | ||
29 | 182 | """ | 180 | """ |
30 | 183 | if not isinstance(path1, str): | 181 | if not isinstance(path1, str): |
31 | 184 | raise TypeError("'path1' must be a plain string, not %s: %r" | 182 | raise TypeError("'path1' must be a plain string, not %s: %r" |
32 | @@ -186,7 +184,7 @@ | |||
33 | 186 | if not isinstance(path2, str): | 184 | if not isinstance(path2, str): |
34 | 187 | raise TypeError("'path2' must be a plain string, not %s: %r" | 185 | raise TypeError("'path2' must be a plain string, not %s: %r" |
35 | 188 | % (type(path2), path2)) | 186 | % (type(path2), path2)) |
37 | 189 | return cmp(path1.split('/'), path2.split('/')) | 187 | return path1.split('/') < path2.split('/') |
38 | 190 | 188 | ||
39 | 191 | 189 | ||
40 | 192 | def _cmp_path_by_dirblock(path1, path2): | 190 | def _cmp_path_by_dirblock(path1, path2): |
41 | 193 | 191 | ||
42 | === modified file 'breezy/_dirstate_helpers_pyx.pyx' | |||
43 | --- breezy/_dirstate_helpers_pyx.pyx 2017-06-10 00:09:13 +0000 | |||
44 | +++ breezy/_dirstate_helpers_pyx.pyx 2017-06-10 00:42:09 +0000 | |||
45 | @@ -244,22 +244,20 @@ | |||
46 | 244 | return 0 | 244 | return 0 |
47 | 245 | 245 | ||
48 | 246 | 246 | ||
50 | 247 | def cmp_by_dirs(path1, path2): | 247 | def lt_by_dirs(path1, path2): |
51 | 248 | """Compare two paths directory by directory. | 248 | """Compare two paths directory by directory. |
52 | 249 | 249 | ||
53 | 250 | This is equivalent to doing:: | 250 | This is equivalent to doing:: |
54 | 251 | 251 | ||
56 | 252 | cmp(path1.split('/'), path2.split('/')) | 252 | operator.lt(path1.split('/'), path2.split('/')) |
57 | 253 | 253 | ||
58 | 254 | The idea is that you should compare path components separately. This | 254 | The idea is that you should compare path components separately. This |
61 | 255 | differs from plain ``cmp(path1, path2)`` for paths like ``'a-b'`` and | 255 | differs from plain ``path1 < path2`` for paths like ``'a-b'`` and ``a/b``. |
62 | 256 | ``a/b``. "a-b" comes after "a" but would come before "a/b" lexically. | 256 | "a-b" comes after "a" but would come before "a/b" lexically. |
63 | 257 | 257 | ||
64 | 258 | :param path1: first path | 258 | :param path1: first path |
65 | 259 | :param path2: second path | 259 | :param path2: second path |
69 | 260 | :return: negative number if ``path1`` comes first, | 260 | :return: True if path1 comes first, otherwise False |
67 | 261 | 0 if paths are equal, | ||
68 | 262 | and positive number if ``path2`` sorts first | ||
70 | 263 | """ | 261 | """ |
71 | 264 | if not PyString_CheckExact(path1): | 262 | if not PyString_CheckExact(path1): |
72 | 265 | raise TypeError("'path1' must be a plain string, not %s: %r" | 263 | raise TypeError("'path1' must be a plain string, not %s: %r" |
73 | @@ -267,10 +265,10 @@ | |||
74 | 267 | if not PyString_CheckExact(path2): | 265 | if not PyString_CheckExact(path2): |
75 | 268 | raise TypeError("'path2' must be a plain string, not %s: %r" | 266 | raise TypeError("'path2' must be a plain string, not %s: %r" |
76 | 269 | % (type(path2), path2)) | 267 | % (type(path2), path2)) |
81 | 270 | return _cmp_by_dirs(PyString_AsString(path1), | 268 | return -1 == _cmp_by_dirs(PyString_AsString(path1), |
82 | 271 | PyString_Size(path1), | 269 | PyString_Size(path1), |
83 | 272 | PyString_AsString(path2), | 270 | PyString_AsString(path2), |
84 | 273 | PyString_Size(path2)) | 271 | PyString_Size(path2)) |
85 | 274 | 272 | ||
86 | 275 | 273 | ||
87 | 276 | def _cmp_path_by_dirblock(path1, path2): | 274 | def _cmp_path_by_dirblock(path1, path2): |
88 | 277 | 275 | ||
89 | === modified file 'breezy/dirstate.py' | |||
90 | --- breezy/dirstate.py 2017-06-05 20:48:31 +0000 | |||
91 | +++ breezy/dirstate.py 2017-06-10 00:42:09 +0000 | |||
92 | @@ -2861,7 +2861,7 @@ | |||
93 | 2861 | # both sides are dealt with, move on | 2861 | # both sides are dealt with, move on |
94 | 2862 | current_old = advance(old_iterator) | 2862 | current_old = advance(old_iterator) |
95 | 2863 | current_new = advance(new_iterator) | 2863 | current_new = advance(new_iterator) |
97 | 2864 | elif (cmp_by_dirs(new_dirname, current_old[0][0]) < 0 | 2864 | elif (lt_by_dirs(new_dirname, current_old[0][0]) |
98 | 2865 | or (new_dirname == current_old[0][0] | 2865 | or (new_dirname == current_old[0][0] |
99 | 2866 | and new_entry_key[1:] < current_old[0][1:])): | 2866 | and new_entry_key[1:] < current_old[0][1:])): |
100 | 2867 | # new comes before: | 2867 | # new comes before: |
101 | @@ -3803,7 +3803,7 @@ | |||
102 | 3803 | def iter_changes(self): | 3803 | def iter_changes(self): |
103 | 3804 | """Iterate over the changes.""" | 3804 | """Iterate over the changes.""" |
104 | 3805 | utf8_decode = cache_utf8._utf8_decode | 3805 | utf8_decode = cache_utf8._utf8_decode |
106 | 3806 | _cmp_by_dirs = cmp_by_dirs | 3806 | _lt_by_dirs = lt_by_dirs |
107 | 3807 | _process_entry = self._process_entry | 3807 | _process_entry = self._process_entry |
108 | 3808 | search_specific_files = self.search_specific_files | 3808 | search_specific_files = self.search_specific_files |
109 | 3809 | searched_specific_files = self.searched_specific_files | 3809 | searched_specific_files = self.searched_specific_files |
110 | @@ -3947,7 +3947,7 @@ | |||
111 | 3947 | current_block is not None): | 3947 | current_block is not None): |
112 | 3948 | if (current_dir_info and current_block | 3948 | if (current_dir_info and current_block |
113 | 3949 | and current_dir_info[0][0] != current_block[0]): | 3949 | and current_dir_info[0][0] != current_block[0]): |
115 | 3950 | if _cmp_by_dirs(current_dir_info[0][0], current_block[0]) < 0: | 3950 | if _lt_by_dirs(current_dir_info[0][0], current_block[0]): |
116 | 3951 | # filesystem data refers to paths not covered by the dirblock. | 3951 | # filesystem data refers to paths not covered by the dirblock. |
117 | 3952 | # this has two possibilities: | 3952 | # this has two possibilities: |
118 | 3953 | # A) it is versioned but empty, so there is no block for it | 3953 | # A) it is versioned but empty, so there is no block for it |
119 | @@ -4269,7 +4269,7 @@ | |||
120 | 4269 | bisect_dirblock, | 4269 | bisect_dirblock, |
121 | 4270 | _bisect_path_left, | 4270 | _bisect_path_left, |
122 | 4271 | _bisect_path_right, | 4271 | _bisect_path_right, |
124 | 4272 | cmp_by_dirs, | 4272 | lt_by_dirs, |
125 | 4273 | pack_stat, | 4273 | pack_stat, |
126 | 4274 | ProcessEntryC as _process_entry, | 4274 | ProcessEntryC as _process_entry, |
127 | 4275 | update_entry as update_entry, | 4275 | update_entry as update_entry, |
128 | @@ -4281,7 +4281,7 @@ | |||
129 | 4281 | bisect_dirblock, | 4281 | bisect_dirblock, |
130 | 4282 | _bisect_path_left, | 4282 | _bisect_path_left, |
131 | 4283 | _bisect_path_right, | 4283 | _bisect_path_right, |
133 | 4284 | cmp_by_dirs, | 4284 | lt_by_dirs, |
134 | 4285 | pack_stat, | 4285 | pack_stat, |
135 | 4286 | ) | 4286 | ) |
136 | 4287 | # FIXME: It would be nice to be able to track moved lines so that the | 4287 | # FIXME: It would be nice to be able to track moved lines so that the |
137 | 4288 | 4288 | ||
138 | === modified file 'breezy/tests/test__dirstate_helpers.py' | |||
139 | --- breezy/tests/test__dirstate_helpers.py 2017-05-22 00:56:52 +0000 | |||
140 | +++ breezy/tests/test__dirstate_helpers.py 2017-06-10 00:42:09 +0000 | |||
141 | @@ -376,20 +376,20 @@ | |||
142 | 376 | return bisect_dirblock | 376 | return bisect_dirblock |
143 | 377 | 377 | ||
144 | 378 | 378 | ||
147 | 379 | class TestCmpByDirs(tests.TestCase): | 379 | class TestLtByDirs(tests.TestCase): |
148 | 380 | """Test an implementation of cmp_by_dirs() | 380 | """Test an implementation of lt_by_dirs() |
149 | 381 | 381 | ||
151 | 382 | cmp_by_dirs() compares 2 paths by their directory sections, rather than as | 382 | lt_by_dirs() compares 2 paths by their directory sections, rather than as |
152 | 383 | plain strings. | 383 | plain strings. |
153 | 384 | 384 | ||
155 | 385 | Child test cases can override ``get_cmp_by_dirs`` to test a specific | 385 | Child test cases can override ``get_lt_by_dirs`` to test a specific |
156 | 386 | implementation. | 386 | implementation. |
157 | 387 | """ | 387 | """ |
158 | 388 | 388 | ||
163 | 389 | def get_cmp_by_dirs(self): | 389 | def get_lt_by_dirs(self): |
164 | 390 | """Get a specific implementation of cmp_by_dirs.""" | 390 | """Get a specific implementation of lt_by_dirs.""" |
165 | 391 | from breezy._dirstate_helpers_py import cmp_by_dirs | 391 | from breezy._dirstate_helpers_py import lt_by_dirs |
166 | 392 | return cmp_by_dirs | 392 | return lt_by_dirs |
167 | 393 | 393 | ||
168 | 394 | def assertCmpByDirs(self, expected, str1, str2): | 394 | def assertCmpByDirs(self, expected, str1, str2): |
169 | 395 | """Compare the two strings, in both directions. | 395 | """Compare the two strings, in both directions. |
170 | @@ -399,17 +399,17 @@ | |||
171 | 399 | :param str1: string to compare | 399 | :param str1: string to compare |
172 | 400 | :param str2: string to compare | 400 | :param str2: string to compare |
173 | 401 | """ | 401 | """ |
175 | 402 | cmp_by_dirs = self.get_cmp_by_dirs() | 402 | lt_by_dirs = self.get_lt_by_dirs() |
176 | 403 | if expected == 0: | 403 | if expected == 0: |
177 | 404 | self.assertEqual(str1, str2) | 404 | self.assertEqual(str1, str2) |
180 | 405 | self.assertEqual(0, cmp_by_dirs(str1, str2)) | 405 | self.assertFalse(lt_by_dirs(str1, str2)) |
181 | 406 | self.assertEqual(0, cmp_by_dirs(str2, str1)) | 406 | self.assertFalse(lt_by_dirs(str2, str1)) |
182 | 407 | elif expected > 0: | 407 | elif expected > 0: |
185 | 408 | self.assertPositive(cmp_by_dirs(str1, str2)) | 408 | self.assertFalse(lt_by_dirs(str1, str2)) |
186 | 409 | self.assertNegative(cmp_by_dirs(str2, str1)) | 409 | self.assertTrue(lt_by_dirs(str2, str1)) |
187 | 410 | else: | 410 | else: |
190 | 411 | self.assertNegative(cmp_by_dirs(str1, str2)) | 411 | self.assertTrue(lt_by_dirs(str1, str2)) |
191 | 412 | self.assertPositive(cmp_by_dirs(str2, str1)) | 412 | self.assertFalse(lt_by_dirs(str2, str1)) |
192 | 413 | 413 | ||
193 | 414 | def test_cmp_empty(self): | 414 | def test_cmp_empty(self): |
194 | 415 | """Compare against the empty string.""" | 415 | """Compare against the empty string.""" |
195 | @@ -475,10 +475,10 @@ | |||
196 | 475 | self.assertCmpByDirs(-1, 'ab/cd', 'ab-cd') | 475 | self.assertCmpByDirs(-1, 'ab/cd', 'ab-cd') |
197 | 476 | 476 | ||
198 | 477 | def test_cmp_unicode_not_allowed(self): | 477 | def test_cmp_unicode_not_allowed(self): |
203 | 478 | cmp_by_dirs = self.get_cmp_by_dirs() | 478 | lt_by_dirs = self.get_lt_by_dirs() |
204 | 479 | self.assertRaises(TypeError, cmp_by_dirs, u'Unicode', 'str') | 479 | self.assertRaises(TypeError, lt_by_dirs, u'Unicode', 'str') |
205 | 480 | self.assertRaises(TypeError, cmp_by_dirs, 'str', u'Unicode') | 480 | self.assertRaises(TypeError, lt_by_dirs, 'str', u'Unicode') |
206 | 481 | self.assertRaises(TypeError, cmp_by_dirs, u'Unicode', u'Unicode') | 481 | self.assertRaises(TypeError, lt_by_dirs, u'Unicode', u'Unicode') |
207 | 482 | 482 | ||
208 | 483 | def test_cmp_non_ascii(self): | 483 | def test_cmp_non_ascii(self): |
209 | 484 | self.assertCmpByDirs(-1, '\xc2\xb5', '\xc3\xa5') # u'\xb5', u'\xe5' | 484 | self.assertCmpByDirs(-1, '\xc2\xb5', '\xc3\xa5') # u'\xb5', u'\xe5' |
210 | @@ -488,14 +488,14 @@ | |||
211 | 488 | self.assertCmpByDirs(-1, 'b/a', 'b/\xc2\xb5') # u'b/a', u'b/\xb5' | 488 | self.assertCmpByDirs(-1, 'b/a', 'b/\xc2\xb5') # u'b/a', u'b/\xb5' |
212 | 489 | 489 | ||
213 | 490 | 490 | ||
216 | 491 | class TestCompiledCmpByDirs(TestCmpByDirs): | 491 | class TestCompiledLtByDirs(TestLtByDirs): |
217 | 492 | """Test the pyrex implementation of cmp_by_dirs""" | 492 | """Test the pyrex implementation of lt_by_dirs""" |
218 | 493 | 493 | ||
219 | 494 | _test_needs_features = [compiled_dirstate_helpers_feature] | 494 | _test_needs_features = [compiled_dirstate_helpers_feature] |
220 | 495 | 495 | ||
224 | 496 | def get_cmp_by_dirs(self): | 496 | def get_lt_by_dirs(self): |
225 | 497 | from breezy._dirstate_helpers_pyx import cmp_by_dirs | 497 | from breezy._dirstate_helpers_pyx import lt_by_dirs |
226 | 498 | return cmp_by_dirs | 498 | return lt_by_dirs |
227 | 499 | 499 | ||
228 | 500 | 500 | ||
229 | 501 | class TestCmpPathByDirblock(tests.TestCase): | 501 | class TestCmpPathByDirblock(tests.TestCase): |
230 | @@ -778,12 +778,12 @@ | |||
231 | 778 | from breezy._dirstate_helpers_py import _bisect_path_right | 778 | from breezy._dirstate_helpers_py import _bisect_path_right |
232 | 779 | self.assertIs(_bisect_path_right, dirstate._bisect_path_right) | 779 | self.assertIs(_bisect_path_right, dirstate._bisect_path_right) |
233 | 780 | 780 | ||
235 | 781 | def test_cmp_by_dirs(self): | 781 | def test_lt_by_dirs(self): |
236 | 782 | if compiled_dirstate_helpers_feature.available(): | 782 | if compiled_dirstate_helpers_feature.available(): |
238 | 783 | from breezy._dirstate_helpers_pyx import cmp_by_dirs | 783 | from breezy._dirstate_helpers_pyx import lt_by_dirs |
239 | 784 | else: | 784 | else: |
242 | 785 | from breezy._dirstate_helpers_py import cmp_by_dirs | 785 | from breezy._dirstate_helpers_py import lt_by_dirs |
243 | 786 | self.assertIs(cmp_by_dirs, dirstate.cmp_by_dirs) | 786 | self.assertIs(lt_by_dirs, dirstate.lt_by_dirs) |
244 | 787 | 787 | ||
245 | 788 | def test__read_dirblocks(self): | 788 | def test__read_dirblocks(self): |
246 | 789 | if compiled_dirstate_helpers_feature.available(): | 789 | if compiled_dirstate_helpers_feature.available(): |