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

Proposed by mnn
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) Approve
bzr-core 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
6549. By mnn

Simplified test_guess_rename_handles_new_directories

Revision history for this message
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

Simplified test_guess_rename_handles_new_directories

6548. By mnn

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])