Hi! [[[ Do not display a binary property diff in the interactive conflict resolver.
* subversion/include/svn_io.h (svn_stream_detect_binary_mimetype): New. Declare. * subversion/libsvn_subr/stream.c (svn_stream_detect_binary_mimetype): Check if atleast 85 % of the bytes fall within the ascii range of printable characters. * subversion/libsvn_wc/props.c (maybe_generate_propconflict): Call ..binary_mimetype() on the mergestream. Set the conflict description according to the outcome. * subversion/svn/conflict-callbacks.c (svn_cl__conflict_handler): Print err msg if desc->is_binary when invoking 'diff-full'. Patch by: Daniel Näslund <daniel{_AT_}longitudo.com> ]]] A recent thread started by Mike Samuel [1] discussed various strategies for improving the detection of 'mergeability'. This patch only uses existing solutions. I've taken a piece of code from svn_io_detect_mimetype2() for detecting if a file buffer is binary. If it is then the mime_type is set to "application/octet-stream". From the mime-type it is determined if the property is binary or not. In a follow-up patch I intend to extract the duplicated code for detecting binary content into a libsvn_subr private function. Why not get crazy and link in libmagic (used by the UNIX file command) to determine what sort of content we have? Right now it says that anything besides printable ascii is binary. I'm attaching a script for testing this patch. Since I'm checking if the content is binary on the mergestream I will end up with the diff beeing displayed if it's below a certain treshold (when the number of printable unidiff characters is more than 15 % of the total amount of bytes. Is it ok to have it like that or should I test for each of the original streams instead? This was an easy solution. Perhaps too easy? make check passed. /Daniel [
merge-conflict.sh
Description: Bourne shell script
Index: subversion/libsvn_subr/stream.c =================================================================== --- subversion/libsvn_subr/stream.c (revision 881392) +++ subversion/libsvn_subr/stream.c (arbetskopia) @@ -1347,3 +1347,44 @@ return SVN_NO_ERROR; } + +svn_error_t * +svn_stream_detect_binary_mimetype(const char **mimetype, + svn_stream_t *stream) +{ + static const char * const generic_binary = "application/octet-stream"; + char block[1024]; + apr_size_t amt_read = sizeof(block); + + /* Default return value is NULL. */ + *mimetype = NULL; + + SVN_ERR(svn_stream_read(stream, block, &amt_read)); + + if (amt_read > 0) + { + apr_size_t i; + apr_size_t binary_count = 0; + + for (i = 0; i < amt_read; i++) + { + if (block[i] == 0) + { + binary_count = amt_read; + break; + } + if ((block[i] < 0x07) + || ((block[i] > 0x0D) && (block[i] < 0x20)) + || (block[i] > 0x7F)) + { + binary_count++; + } + } + if (((binary_count * 1000) / amt_read) > 850) + { + *mimetype = generic_binary; + return SVN_NO_ERROR; + } + } + return SVN_NO_ERROR; +} Index: subversion/svn/conflict-callbacks.c =================================================================== --- subversion/svn/conflict-callbacks.c (revision 881392) +++ subversion/svn/conflict-callbacks.c (arbetskopia) @@ -615,6 +615,21 @@ } else if (strcmp(answer, "df") == 0) { + if (desc->is_binary) + { + const char *node; + if (desc->kind == svn_wc_conflict_kind_property) + node = _("property"); + else + node = _("file"); + + SVN_ERR(svn_cmdline_fprintf(stderr, subpool, + "%s %s.\n\n", + _("Invalid option; cannot " + "display conflicts for a " + "binary"), node)); + continue; + } if (! diff_allowed) { SVN_ERR(svn_cmdline_fprintf(stderr, subpool, Index: subversion/include/svn_io.h =================================================================== --- subversion/include/svn_io.h (revision 881392) +++ subversion/include/svn_io.h (arbetskopia) @@ -1186,7 +1186,16 @@ apr_pool_t *result_pool, apr_pool_t *scratch_pool); +/** + * Examine utf8-encoded @a stream to determine if it has binary content. If + * so set @a *mimetype to a character string representing a general binary + * MIME type, else set it to @c NULL. + */ +svn_error_t * +svn_stream_detect_binary_mimetype(const char **mimetype, + svn_stream_t *stream); + /** @} */ /** Set @a *result to a string containing the contents of @a Index: subversion/libsvn_wc/props.c =================================================================== --- subversion/libsvn_wc/props.c (revision 881392) +++ subversion/libsvn_wc/props.c (arbetskopia) @@ -939,9 +939,9 @@ apr_pool_t *scratch_pool) { svn_wc_conflict_result_t *result = NULL; - svn_string_t *mime_propval = NULL; apr_pool_t *filepool = svn_pool_create(scratch_pool); svn_wc_conflict_description2_t *cdesc; + const char *mimetype = NULL; const char *dirpath = svn_dirent_dirname(local_abspath, filepool); if (cancel_func) @@ -1046,17 +1046,20 @@ (mergestream, diff, the_val, working_val, new_val, NULL, NULL, NULL, NULL, svn_diff_conflict_display_modified_latest, filepool)); + + /* We read the stream from the beginning again. */ + SVN_ERR(svn_stream_reset(mergestream)); + SVN_ERR(svn_stream_detect_binary_mimetype(&mimetype, mergestream)); + + svn_stream_close(mergestream); } } /* Build the rest of the description object: */ - if (!is_dir && working_props) - mime_propval = apr_hash_get(working_props, SVN_PROP_MIME_TYPE, - APR_HASH_KEY_STRING); - cdesc->mime_type = mime_propval ? mime_propval->data : NULL; - cdesc->is_binary = mime_propval ? - svn_mime_type_is_binary(mime_propval->data) : FALSE; + cdesc->mime_type = mimetype ? mimetype : NULL; + cdesc->is_binary = mimetype ? + svn_mime_type_is_binary(mimetype) : FALSE; if (!old_val && new_val) cdesc->action = svn_wc_conflict_action_add;