Merge lp:~mnn282/bzr/auto-rename-fix into lp:bzr

Proposed by mnn on 2012-07-29
Status: Needs review
Proposed branch: lp:~mnn282/bzr/auto-rename-fix
Merge into: lp:bzr
Diff against target: 51 lines (+25/-1)
2 files modified
bzrlib/rename_map.py (+8/-1)
bzrlib/tests/test_rename_map.py (+17/-0)
To merge this branch: bzr merge lp:~mnn282/bzr/auto-rename-fix
Reviewer Review Type Date Requested Status
Martin Packman (community) 2012-07-29 Approve on 2012-07-30
bzr-core 2012-07-30 Pending
Review via email: mp+117203@code.launchpad.net

Description of the Change

Fixed bug with RenameMap (bzr move --auto), also RenameMap has ability to detect files moved into new unversioned directories.

To post a comment you must log in.
lp:~mnn282/bzr/auto-rename-fix updated on 2012-07-29
6549. By mnn on 2012-07-29

Simplified test_guess_rename_handles_new_directories

Martin Packman (gz) wrote :

Thanks for working on this!

I'm still a little uncertain on the core logic here, will look again later.

+ if self.tree.has_filename(self.tree.id2path(parent[0])) == False:

Normal style is not to compare against booleans, just use `if not condition` instead.

+ self.assertEqual('added:\n folder/\nrenamed:\n file => folder/file2\n',
+ self.run_bzr("status")[0])

At this level, in bzrlib.tests rather than bzrlib.tests.blackbox, you really want to be writing assertions based on api calls rather than the commandline ui. Mostly this just confirms the check above, but adding an assertion for the current tree shape does seem reasonable.

I see you've already fixed the other style stuff I mentioned on IRC, thanks!

review: Approve

Unmerged revisions

6549. By mnn on 2012-07-29

Simplified test_guess_rename_handles_new_directories

6548. By mnn on 2012-07-29

Fixed issue with RenameMap - also it supports renaming into new unversioned directory

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/rename_map.py'
2--- bzrlib/rename_map.py 2011-12-18 15:28:38 +0000
3+++ bzrlib/rename_map.py 2012-07-29 23:12:20 +0000
4@@ -183,7 +183,8 @@
5 for (file_id, paths, changed_content, versioned, parent, name,
6 kind, executable) in iterator:
7 if kind[1] is None and versioned[1]:
8- missing_parents.setdefault(parent[0], set()).add(file_id)
9+ if self.tree.has_filename(self.tree.id2path(parent[0])) == False:
10+ missing_parents.setdefault(parent[0], set()).add(file_id)
11 if kind[0] == 'file':
12 missing_files.add(file_id)
13 else:
14@@ -255,6 +256,12 @@
15 parent_id = matches.get(parent_path)
16 if parent_id is None:
17 parent_id = self.tree.path2id(parent_path)
18+ if parent_id is None:
19+ added, ignored = self.tree.smart_add([parent_path], recurse=False)
20+ if len(ignored) > 0 and ignored[0] == parent_path:
21+ continue
22+ else:
23+ parent_id = self.tree.path2id(parent_path)
24 if entry.name == new_name and entry.parent_id == parent_id:
25 continue
26 new_entry = entry.copy()
27
28=== modified file 'bzrlib/tests/test_rename_map.py'
29--- bzrlib/tests/test_rename_map.py 2009-05-08 16:34:09 +0000
30+++ bzrlib/tests/test_rename_map.py 2012-07-29 23:12:20 +0000
31@@ -200,3 +200,20 @@
32 notes = self.captureNotes(RenameMap.guess_renames, tree,
33 dry_run=False)[0]
34 self.assertEqual('file => file2', ''.join(notes))
35+
36+ def test_guess_rename_handles_new_directories(self):
37+ """When a file was moved into a new directory."""
38+ tree = self.make_branch_and_tree('.')
39+ tree.lock_write()
40+ #self.addCleanup(tree.unlock)
41+ self.build_tree(['file'])
42+ tree.add('file', 'file-id')
43+ tree.commit('Added file')
44+ os.mkdir('folder')
45+ os.rename('file', 'folder/file2')
46+ notes = self.captureNotes(RenameMap.guess_renames, tree)[0]
47+ self.assertEqual('file => folder/file2', ''.join(notes))
48+
49+ tree.unlock()
50+ self.assertEqual('added:\n folder/\nrenamed:\n file => folder/file2\n',
51+ self.run_bzr("status")[0])