[PATCH] Fix for bug #314251 [mv via rm+add breaking dirstate]

[Charles Duffy]

Ian Clatworthy wrote:
> Please add a NEWS item into the BUGS section.

Done.

> I'm not sure if the inner if test needs an else clause or not?
> If they do match, is that OK or a logic/data error we ought to
> report? Please check this with a dirstate guru, e.g. lifeless
> or jam.

If they do match, we're re-adding a file with the same name (and in the 
same directory) as when it was first deleted. dirstate handles this case 
correctly as-is, and so falling through (without an else clause) gives 
us the same behavior we would have had were include_deleted not passed 
to _get_entry (in which case the outermost if, testing file_id_entry, 
would have failed).

>> +        if rename_from:
>> +            if rename_from[0]:
>> +                old_path_utf8 = '%s/%s' % rename_from
>> +            else:
>> +                old_path_utf8 = rename_from[1]
>> +            parent_info[0] = ('r', old_path_utf8, 0, False, '')
> 
> The top "if" will be slightly more efficient if written as
> 
>   if rename_from is not None:

Done.

>> -    def _get_entry(self, tree_index, fileid_utf8=None, path_utf8=None):
>> +    def _get_entry(self, tree_index, fileid_utf8=None, path_utf8=None, include_deleted=False):
>>          """Get the dirstate entry for path in tree tree_index.
> 
> Please add a docstring entry for the new param, making it clear
> that it only applies when path_utf8 is None.

Ahh -- good point. Done.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr.cduffy.bug314251-r2.patch
Type: text/x-patch
Size: 8489 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090108/1a8d4a3b/attachment.bin