Philip Martin wrote:
> Julian Foad <julianfoad_at_btopenworld.com> writes:
>>> +
>>> + if (merge_outcome == svn_wc_merge_conflict)
>>> + {
>>> + content_state = svn_wc_notify_state_conflicted;
>>> + }
>>> + else
>>> + {
>>> + SVN_ERR(svn_wc__internal_file_modified_p(&is_locally_modified,
>>> + db, local_abspath,
>>> + FALSE,
>>> + scratch_pool));
>>> + if (is_locally_modified)
>>> + content_state = svn_wc_notify_state_merged;
>>> + else
>>> + content_state = svn_wc_notify_state_changed;
>>> + }
>>
>> I don't understand that last 'else' block.
>>
>> We're asking 'Is it locally modified?' but I don't think we have a
>> consistent notion of what 'it' is, since the above svn_wc__internal_merge()
>> might or might not have updated the WC file at local_abspath -- it might
>> instead have created a temp file and set up some work items which will
>> install that file later [1].
>>
>> Do we actually want to ask here, 'Was the file locally modified before we
>> started merging?' If so, this whole block should be replaced with just
>> "content_state = svn_wc_notify_state_merged;".
>
> I'm a bit confused by that code as well but I didn't add that code in
> this commit I just changed the identation. It dates back to r1406631.
You didn't add this code but you repeated the 'svn_wc__internal_file_modified_p' call earlier, and moved all the existing code inside the 'else' of that earlier test, thus changing the conditions under which this can be executed -- it can no longer be executed if the file wasn't already locally modified.
I think the intent is to print 'U' if we're updating the file content
when it was not already locally modified, or 'G' if we are merging into a
file that was already locally modified. Or something close to that.
I'm not convinced that we're very consistent about this aspect of
updates and merges.
- Julian
Received on 2013-03-13 22:49:29 CET