Just a quick heads up since I didn't get to reply to this one yet:
Thanks for your review, Evgeny.
On 10/14/2016 6:24 PM, Evgeny Kotkov wrote:
Stefan Hett <luke1...@apache.org> writes:
if (option == NULL)
- return svn_error_createf(SVN_ERR_CLIENT_CONFLICT_OPTION_NOT_APPLICABLE,
- NULL,
- _("Inapplicable conflict resolution option "
- "given for conflicted path '%s'"),
- svn_dirent_local_style(conflict->local_abspath,
- scratch_pool));
+ {
+ if (option_id == svn_client_conflict_option_merged_text) {
+ mime_type = svn_client_conflict_text_get_mime_type(conflict);
+ if (mime_type && svn_mime_type_is_binary(mime_type))
+ option = svn_client_conflict_option_find_by_id(resolution_options,
+ svn_client_conflict_option_working_text);
+ }
+ }
Looks like the indentation and the brace formatting are broken here and
in the next hunk.
Right, my bad. Already wrote a patch to correct this indentation issue
and the other one on Sunday on the train but didn't get to send it yet.
Meanwhile I see that stsp already corrected (and simplified) that code
section. Thanks for that, stsp.
+
+ if (option == NULL)
+ return svn_error_createf(SVN_ERR_CLIENT_CONFLICT_OPTION_NOT_APPLICABLE,
+ NULL,
+ _("Inapplicable conflict resolution option "
+ "given for conflicted path '%s'"),
As for the change itself, I don't think that silently transforming one
conflict option id to another in svn_client_conflict_text_resolve_by_id()
API is a proper thing to do, because we don't expose this option id as a
viable resolution option in svn_client_conflict_text_get_resolution_options().
I think that a better way would be to handle this case in the command line
by using svn_client_conflict_option_working_text for binary files, if
we run with --accept=working.
I'll get back to that with some details and will double check the
current approach, as soon as I can free up some time here (not sure
whether this will be this week though). Only got half through the review
on Sunday unfortunately.
--
Regards,
Stefan Hett, Developer/Administrator
EGOSOFT GmbH, Heidestrasse 4, 52146 Würselen, Germany
Tel: +49 2405 4239970, www.egosoft.com
Geschäftsführer: Bernd Lehahn, Handelsregister Aachen HRB 13473
