[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r31030 - in trunk/subversion: libsvn_diff tests/libsvn_diff

From: David Glasser <glasser_at_davidglasser.net>
Date: Mon, 5 May 2008 16:30:56 +0100

... so I wrote this revision while offline a few days ago. It's
actually problematic: the context is based purely on the "original"
file rather than on the diff3 resolution, and I think because of that
the line numbers might not be correct. I have a way to fix the former
issue that's half-implemented; for the latter, I might just ditch line
numbers for now, or add them to the marker lines rather than the @@
line. Does anyone know of prior art for a
diff3-showing-only-conflicts-with-context format?

--dave

On Mon, May 5, 2008 at 4:23 PM, <glasser_at_tigris.org> wrote:
> Author: glasser
> Date: Mon May 5 08:23:46 2008
> New Revision: 31030
>
> Log:
> Show context in the "conflict-only diff3" output.
>
> The header line for conflict-only context sections looks like:
>
> @@ <M,N |M,N >M,N @@
>
> If there is prior art that I should be mimicking, let me know.
>
> This revision implements it only for in-memory diffs (because it's
> easier to start here...), so the only visible way to see that it works
> is to see that the test XFails during the file-diff part, not the
> mem-diff part.
>
> * subversion/libsvn_diff/diff_memory.c
> (merge_output_baton_t): Add some fields only used by conflict-only
> mode: hunk, hunk_length, hunk_start, real_output_stream, and pool.
> (output_merge_token_range): Add a lines_printed_p arg.
> (output_common_modified, output_latest, output_conflict): Adjust.
> (output_conflicts_flush_hunk): New.
> (output_conflict_with_context): Actually have context.
> (svn_diff_mem_string_output_merge2): Initialize fields and clean up
> differently for conflict-only mode.
>
> * subversion/tests/libsvn_diff/diff-diff3-test.c
> (test_three_way_merge_conflict_styles): Add context to expected
> output.
> (test_funcs): Mark test_three_way_merge_conflict_styles as XFail.
>
> Modified:
> trunk/subversion/libsvn_diff/diff_memory.c
> trunk/subversion/tests/libsvn_diff/diff-diff3-test.c
>
> Modified: trunk/subversion/libsvn_diff/diff_memory.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_diff/diff_memory.c?pathrev=31030&r1=31029&r2=31030
> ==============================================================================
> --- trunk/subversion/libsvn_diff/diff_memory.c Mon May 5 08:23:09 2008 (r31029)
> +++ trunk/subversion/libsvn_diff/diff_memory.c Mon May 5 08:23:46 2008 (r31030)
> @@ -599,24 +599,38 @@ typedef struct merge_output_baton_t
> 2 = separator, 3 = latest (end) */
>
> svn_diff_conflict_display_style_t conflict_style;
> +
> + /* For svn_diff_conflict_display_only_conflicts only: */
> + svn_stringbuf_t *hunk; /* in-progress hunk data */
> + apr_size_t hunk_length[3];
> + apr_off_t hunk_start[3];
> + /* The actual output stream (OUTPUT_STREAM just writes to HUNK). */
> + svn_stream_t *real_output_stream;
> + apr_pool_t *pool;
> } merge_output_baton_t;
>
>
> static svn_error_t *
> -output_merge_token_range(merge_output_baton_t *btn,
> +output_merge_token_range(apr_size_t *lines_printed_p,
> + merge_output_baton_t *btn,
> int idx, apr_off_t first,
> apr_off_t length)
> {
> apr_array_header_t *tokens = btn->sources[idx].tokens;
> + apr_size_t lines_printed = 0;
>
> - for (; length > 0; length--, first++)
> + for (; length > 0 && first < tokens->nelts; length--, first++)
> {
> svn_string_t *token = APR_ARRAY_IDX(tokens, first, svn_string_t *);
> apr_size_t len = token->len;
>
> SVN_ERR(svn_stream_write(btn->output_stream, token->data, &len));
> + lines_printed++;
> }
>
> + if (lines_printed_p)
> + *lines_printed_p = lines_printed;
> +
> return SVN_NO_ERROR;
> }
>
> @@ -633,7 +647,7 @@ output_common_modified(void *baton,
> apr_off_t modified_start, apr_off_t modified_length,
> apr_off_t latest_start, apr_off_t latest_length)
> {
> - return output_merge_token_range(baton, 1/*modified*/,
> + return output_merge_token_range(NULL, baton, 1/*modified*/,
> modified_start, modified_length);
> }
>
> @@ -643,7 +657,7 @@ output_latest(void *baton,
> apr_off_t modified_start, apr_off_t modified_length,
> apr_off_t latest_start, apr_off_t latest_length)
> {
> - return output_merge_token_range(baton, 2/*latest*/,
> + return output_merge_token_range(NULL, baton, 2/*latest*/,
> latest_start, latest_length);
> }
>
> @@ -686,26 +700,26 @@ output_conflict(void *baton,
> style == svn_diff_conflict_display_modified_original_latest)
> {
> SVN_ERR(output_merge_marker(btn, 1/*modified*/));
> - SVN_ERR(output_merge_token_range(btn, 1/*modified*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 1/*modified*/,
> modified_start, modified_length));
>
> if (style == svn_diff_conflict_display_modified_original_latest)
> {
> SVN_ERR(output_merge_marker(btn, 0/*original*/));
> - SVN_ERR(output_merge_token_range(btn, 0/*original*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 0/*original*/,
> original_start, original_length));
> }
>
> SVN_ERR(output_merge_marker(btn, 2/*separator*/));
> - SVN_ERR(output_merge_token_range(btn, 2/*latest*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 2/*latest*/,
> latest_start, latest_length));
> SVN_ERR(output_merge_marker(btn, 3/*latest (end)*/));
> }
> else if (style == svn_diff_conflict_display_modified)
> - SVN_ERR(output_merge_token_range(btn, 1/*modified*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 1/*modified*/,
> modified_start, modified_length));
> else if (style == svn_diff_conflict_display_latest)
> - SVN_ERR(output_merge_token_range(btn, 2/*latest*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 2/*latest*/,
> latest_start, latest_length));
> else /* unknown style */
> abort();
> @@ -715,6 +729,60 @@ output_conflict(void *baton,
>
>
> static svn_error_t *
> +output_conflicts_flush_hunk(merge_output_baton_t *baton)
> +{
> + int i;
> + apr_size_t hunk_len;
> + apr_size_t trailing_len;
> + if (svn_stringbuf_isempty(baton->hunk))
> + return SVN_NO_ERROR;
> +
> + svn_pool_clear(baton->pool);
> +
> + /* Add trailing context to the hunk. */
> + SVN_ERR(output_merge_token_range(&trailing_len, baton, 0 /*original*/,
> + baton->hunk_start[0] + baton->hunk_length[0],
> + SVN_DIFF__UNIFIED_CONTEXT_SIZE));
> + baton->hunk_length[0] += trailing_len;
> + baton->hunk_length[1] += trailing_len;
> + baton->hunk_length[2] += trailing_len;
> +
> + /* Write the hunk header. */
> + SVN_ERR(svn_stream_printf(baton->real_output_stream, baton->pool, "@@"));
> +
> + for (i = 0; i < 3; i++)
> + {
> + static int sources[] = { 1, 0, 2 };
> + int src = sources[i];
> + char tag = "|<>"[src];
> + if (baton->hunk_length[src] > 0)
> + /* Convert our 0-based line numbers into 1-based numbers */
> + baton->hunk_start[src]++;
> + SVN_ERR(svn_stream_printf
> + (baton->real_output_stream, baton->pool,
> + /* Hunk length 1 is implied, don't show the
> + length field if we have a hunk that long */
> + (baton->hunk_length[src] == 1)
> + ? (" %c%" APR_OFF_T_FMT)
> + : (" %c%" APR_OFF_T_FMT ",%" APR_SIZE_T_FMT),
> + tag, baton->hunk_start[src], baton->hunk_length[src]));
> + }
> +
> + SVN_ERR(svn_stream_printf(baton->real_output_stream, baton->pool,
> + " @@" APR_EOL_STR));
> +
> + hunk_len = baton->hunk->len;
> + SVN_ERR(svn_stream_write(baton->real_output_stream,
> + baton->hunk->data, &hunk_len));
> +
> + baton->hunk_length[0] = baton->hunk_length[1] = baton->hunk_length[2] = 0;
> + svn_stringbuf_setempty(baton->hunk);
> +
> + return SVN_NO_ERROR;
> +}
> +
> +
> +static svn_error_t *
> output_conflict_with_context(void *baton,
> apr_off_t original_start,
> apr_off_t original_length,
> @@ -725,18 +793,51 @@ output_conflict_with_context(void *baton
> svn_diff_t *diff)
> {
> merge_output_baton_t *btn = baton;
> + apr_off_t targ_orig;
> +
> + /* Calculate beginning of context. */
> + targ_orig = original_start - SVN_DIFF__UNIFIED_CONTEXT_SIZE;
> + targ_orig = (targ_orig < 0) ? 0 : targ_orig;
> +
> + if (btn->hunk_start[0] + btn->hunk_length[0] + SVN_DIFF__UNIFIED_CONTEXT_SIZE
> + < targ_orig)
> + /* There's previous context, but we don't blend with it. */
> + SVN_ERR(output_conflicts_flush_hunk(btn));
> +
> + if (btn->hunk_length[0] == 0
> + && btn->hunk_length[1] == 0
> + && btn->hunk_length[2] == 0)
> + {
> + btn->hunk_start[0] = targ_orig;
> + btn->hunk_start[1] = modified_start + (targ_orig - original_start);
> + btn->hunk_start[2] = latest_start + (targ_orig - original_start);
> + }
> + else
> + /* There's context; make sure we're not repeating it. */
> + targ_orig = btn->hunk_start[0] + btn->hunk_length[0];
> +
> + /* Output leading context. */
> + SVN_ERR(output_merge_token_range(NULL, btn, 0/*original*/,
> + targ_orig, original_start - targ_orig));
> + btn->hunk_length[0] += original_start - targ_orig;
> + btn->hunk_length[1] += original_start - targ_orig;
> + btn->hunk_length[2] += original_start - targ_orig;
>
> + /* Output the conflict itself. */
> SVN_ERR(output_merge_marker(btn, 1/*modified*/));
> - SVN_ERR(output_merge_token_range(btn, 1/*modified*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 1/*modified*/,
> modified_start, modified_length));
> + btn->hunk_length[1] += modified_length;
>
> SVN_ERR(output_merge_marker(btn, 0/*original*/));
> - SVN_ERR(output_merge_token_range(btn, 0/*original*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 0/*original*/,
> original_start, original_length));
> + btn->hunk_length[0] += original_length;
>
> SVN_ERR(output_merge_marker(btn, 2/*separator*/));
> - SVN_ERR(output_merge_token_range(btn, 2/*latest*/,
> + SVN_ERR(output_merge_token_range(NULL, btn, 2/*latest*/,
> latest_start, latest_length));
> + btn->hunk_length[2] += latest_length;
> SVN_ERR(output_merge_marker(btn, 3/*latest (end)*/));
>
> return SVN_NO_ERROR;
> @@ -799,7 +900,16 @@ svn_diff_mem_string_output_merge2(svn_st
> ? &merge_only_conflicts_output_vtable : &merge_output_vtable;
>
> memset(&btn, 0, sizeof(btn));
> - btn.output_stream = output_stream;
> +
> + if (conflicts_only)
> + {
> + btn.hunk = svn_stringbuf_create("", pool);
> + btn.output_stream = svn_stream_from_stringbuf(btn.hunk, pool);
> + btn.real_output_stream = output_stream;
> + btn.pool = svn_pool_create(pool);
> + }
> + else
> + btn.output_stream = output_stream;
>
> fill_source_tokens(&(btn.sources[0]), original, pool);
> fill_source_tokens(&(btn.sources[1]), modified, pool);
> @@ -846,6 +956,11 @@ svn_diff_mem_string_output_merge2(svn_st
> pool));
>
> SVN_ERR(svn_diff_output(diff, &btn, vtable));
> + if (conflicts_only)
> + {
> + SVN_ERR(output_conflicts_flush_hunk(&btn));
> + svn_pool_destroy(btn.pool);
> + }
>
> return SVN_NO_ERROR;
> }
>
> Modified: trunk/subversion/tests/libsvn_diff/diff-diff3-test.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn_diff/diff-diff3-test.c?pathrev=31030&r1=31029&r2=31030
> ==============================================================================
> --- trunk/subversion/tests/libsvn_diff/diff-diff3-test.c Mon May 5 08:23:09 2008 (r31029)
> +++ trunk/subversion/tests/libsvn_diff/diff-diff3-test.c Mon May 5 08:23:46 2008 (r31030)
> @@ -1974,9 +1974,12 @@ test_three_way_merge_conflict_styles(con
> svn_diff_conflict_display_modified_original_latest,
> pool));
>
> - /* ### Really should have some context. */
> SVN_ERR(three_way_merge("style-only1", "style-only2", "style-only3",
> original, modified, latest,
> + "@@ <8,16 |8,13 >8,16 @@\n"
> + "h\n"
> + "i\n"
> + "j\n"
> "<<<<<<< style-only2\n"
> "k\n"
> "l\n"
> @@ -2007,7 +2010,10 @@ test_three_way_merge_conflict_styles(con
> "yay\n"
> "p\n"
> "q\n"
> - ">>>>>>> style-only3\n",
> + ">>>>>>> style-only3\n"
> + "r\n"
> + "s\n"
> + "t\n",
> NULL,
> svn_diff_conflict_display_only_conflicts,
> pool));
> @@ -2348,6 +2354,6 @@ struct svn_test_descriptor_t test_funcs[
> SVN_TEST_PASS(random_three_way_merge),
> SVN_TEST_PASS(merge_with_part_already_present),
> SVN_TEST_PASS(merge_adjacent_changes),
> - SVN_TEST_PASS(test_three_way_merge_conflict_styles),
> + SVN_TEST_XFAIL(test_three_way_merge_conflict_styles),
> SVN_TEST_NULL
> };
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>
>

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-05 17:31:24 CEST

This is an archived mail posted to the Subversion Dev mailing list.