Re: [PATCH] Fix issue #4316 - Merge errors out after resolving conflicts

--
Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
----- Original Message -----
> From: Julian Foad <julianfoad_at_btopenworld.com>
> To: C. Michael Pilato <cmpilato_at_collab.net>
> Cc: Subversion Development <dev_at_subversion.apache.org>
> Sent: Wednesday, 20 March 2013, 15:38
> Subject: Re: [PATCH] Fix issue #4316 - Merge errors out after resolving conflicts
> 
> C. Michael Pilato wrote:
>>  On 03/15/2013 06:03 PM, Julian Foad wrote:
>>>   NOTIFICATIONS CHANGED
>>> 
>>>   As I mentioned in my "Conflict resolver callback design" 
> email, 
>>>  doing this does mean that the notification receiver will get a 
> 'C' 
>>>  (conflict) notification for every conflict even if that conflict is 
> going to be 
>>>  resolved to a pre-determined choice.  In terms of the 'svn' 
> client and 
>>>  the 'svn merge' command, this means that 'svn merge 
>>>  --accept=[mine-full, etc.]' will, if we don't take further 
> action, print 
>>>  something like in this example:
>>> 
>>>   [[[
>>>   --- Merging r3 through r4 into 'merge_tests-135/A2/mu':
>>>    C   merge_tests-135/A2/mu
>>>   --- Recording mergeinfo for merge of r3 through r4 into 
>>>  'merge_tests-135/A2/mu':
>>>    G   merge_tests-135/A2/mu
>>>   Resolved conflicted state of 'merge_tests-135/A2/mu'
>>>   --- Merging r6 through r8 into 'merge_tests-135/A2/mu':
>>>    U   merge_tests-135/A2/mu
>>>   --- Merging r10 through r11 into 'merge_tests-135/A2/mu':
>>>    U   merge_tests-135/A2/mu
>>>   --- Recording mergeinfo for merge of r5 through r11 into 
>>>  'merge_tests-135/A2/mu':
>>>    G   merge_tests-135/A2/mu
>>>   Summary of conflicts:
>>>     Property conflicts: 1
>>>   ]]]
>>> 
>>>   I think if this was changed to print a slightly different summary, 
>>>  something like...
>>> 
>>>   [[[
>>>   Summary of conflicts:
>>>     Property conflicts: 0 (and 1 already resolved)
>>>   ]]]
>>> 
>>>   ... then it would be fine.  I don't see that the interleaved 
>>>  'Resolved ...' line is a problem.
>>> 
>>>   Do others agree?
>> 
>>  The point of the summary section is to draw attention to details that might
>>  have whizzed by the screen.  Given that, I agree it's a bit misleading 
> to
>>  alert the user to a problem which may not really be a problem any longer.
>>  So yeah, a change such as what you've suggested makes sense to me.
>> 
>>  (Sorry, no feedback on your actual patch.)
> 
> I have committed a complete fix, with the Summary of Conflicts as discussed 
> here, in <http://svn.apache.org/r1459012>.
Remaining issues:
  * There is an inconsistency with what rev ranges get recorded as merged, in a file merge vs. a dir merge -- see the comment and work-around in merge_tests.py 138.
  * Tweak the Summary of Conflicts to use the extended "%d remaining (and %d already resolved)" format for all three kinds of conflict iff it uses it for any kind.  That would look better and be more plainly understandable, in a realistic merge where there will often be more than one kind of conflict raised.
  * Update and switch should work the same way.
- Julian