Author: dannas
Date: Wed Jul 14 17:41:38 2010
New Revision: 964107
URL: http://svn.apache.org/viewvc?rev=964107&view=rev
Log:
Fix bug in the diff parser where only the first property hunk after
a property header was detected.
* subversion/libsvn_diff/parse-diff.c
(parse_next_hunk): Introduce a new out parameter 'is_property' for
determerning the type of the returned hunk.
(svn_diff_parse_next_patch): Store the last prop name we've parsed
to be able to identify what property a hunk belongs to.
* subversion/tests/libsvn_diff/parse-diff-test.c
(property_unidiff): Use two hunks for one property.
(test_parse_property_diff): Check two hunks for one property.
Modified:
subversion/trunk/subversion/libsvn_diff/parse-diff.c
subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c
Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/parse-diff.c?rev=964107&r1=964106&r2=964107&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original)
+++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Wed Jul 14 17:41:38
2010
@@ -500,14 +500,17 @@ parse_prop_name(const char **prop_name,
}
/* Return the next *HUNK from a PATCH, using STREAM to read data
- * from the patch file. If no hunk can be found, set *HUNK to NULL. If we
- * have a property hunk, PROP_NAME and PROP_OPERATION will be set. If we
- * have a text hunk, PROP_NAME will be NULL. If REVERSE is TRUE, invert the
- * hunk while parsing it. If IGNORE_WHiTESPACES is TRUE, let lines without
- * leading spaces be recognized as context lines. Allocate results in
+ * from the patch file. If no hunk can be found, set *HUNK to NULL. Set
+ * IS_PROPERTY to TRUE if we have a property hunk. If the returned HUNK is
+ * the first belonging to a certain property, then PROP_NAME and
+ * PROP_OPERATION will be set too. If we have a text hunk, PROP_NAME will be
+ * NULL. If REVERSE is TRUE, invert the hunk while parsing it. If
+ * IGNORE_WHiTESPACES is TRUE, let lines without leading spaces be
+ * recognized as context lines. Allocate results in
* RESULT_POOL. Use SCRATCH_POOL for all other allocations. */
static svn_error_t *
parse_next_hunk(svn_hunk_t **hunk,
+ svn_boolean_t *is_property,
const char **prop_name,
svn_diff_operation_kind_t *prop_operation,
svn_patch_t *patch,
@@ -536,11 +539,9 @@ parse_next_hunk(svn_hunk_t **hunk,
*prop_operation = svn_diff_op_unchanged;
- /* We only set this if we have a property hunk.
- * ### prop_name acts as both a state flag inside this function and a
- * ### qualifier to discriminate between props and text hunks. Is that
- * ### kind of overloading ok? */
+ /* We only set this if we have a property hunk header. */
*prop_name = NULL;
+ *is_property = FALSE;
if (apr_file_eof(patch->patch_file) == APR_EOF)
{
@@ -669,10 +670,10 @@ parse_next_hunk(svn_hunk_t **hunk,
{
original_lines = (*hunk)->original_length;
modified_lines = (*hunk)->modified_length;
- *prop_name = NULL;
+ *is_property = FALSE;
}
}
- else if (starts_with(line->data, prop_atat) && *prop_name)
+ else if (starts_with(line->data, prop_atat))
{
/* Looks like we have a property hunk header, try to rip it
* apart. */
@@ -682,6 +683,7 @@ parse_next_hunk(svn_hunk_t **hunk,
{
original_lines = (*hunk)->original_length;
modified_lines = (*hunk)->modified_length;
+ *is_property = TRUE;
}
}
else if (starts_with(line->data, "Added: "))
@@ -1213,6 +1215,8 @@ svn_diff_parse_next_patch(svn_patch_t **
else
{
svn_hunk_t *hunk;
+ svn_boolean_t is_property;
+ const char *last_prop_name;
const char *prop_name;
svn_diff_operation_kind_t prop_operation;
@@ -1223,21 +1227,25 @@ svn_diff_parse_next_patch(svn_patch_t **
{
svn_pool_clear(iterpool);
- SVN_ERR(parse_next_hunk(&hunk, &prop_name, &prop_operation,
- *patch, stream, reverse,
+ SVN_ERR(parse_next_hunk(&hunk, &is_property, &prop_name,
+ &prop_operation, *patch, stream, reverse,
ignore_whitespace,
result_pool, iterpool));
- /* We have a property hunk. */
- if (hunk && prop_name)
+ if (hunk && is_property)
{
+ if (! prop_name)
+ prop_name = last_prop_name;
+ else
+ last_prop_name = prop_name;
SVN_ERR(add_property_hunk(*patch, prop_name, hunk,
prop_operation,
result_pool));
}
-
- /* We have a regular text hunk. */
else if (hunk)
- APR_ARRAY_PUSH((*patch)->hunks, svn_hunk_t *) = hunk;
+ {
+ APR_ARRAY_PUSH((*patch)->hunks, svn_hunk_t *) = hunk;
+ last_prop_name = NULL;
+ }
}
while (hunk);
Modified: subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c?rev=964107&r1=964106&r2=964107&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c Wed Jul 14
17:41:38 2010
@@ -148,9 +148,19 @@ static const char *bad_git_diff_header =
"Property changes on: iota" NL
"___________________________________________________________________" NL
"Modified: prop_mod" NL
- "## -1 +1 ##" NL
+ "## -1,4 +1,4 ##" NL
"-value" NL
- "+new value" NL;
+ "+new value" NL
+ " context" NL
+ " context" NL
+ " context" NL
+ "## -10,4 +10,4 ##" NL
+ " context" NL
+ " context" NL
+ " context" NL
+ "-value" NL
+ "+new value" NL
+ "" NL;
/* ### Add edge cases like context lines stripped from leading whitespaces
* ### that starts with 'Added: ', 'Deleted: ' or 'Modified: '. */
@@ -550,14 +560,36 @@ test_parse_property_diff(apr_pool_t *poo
SVN_TEST_ASSERT(prop_patch->operation == svn_diff_op_modified);
hunks = prop_patch->hunks;
- SVN_TEST_ASSERT(hunks->nelts == 1);
+ SVN_TEST_ASSERT(hunks->nelts == 2);
hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *);
SVN_ERR(check_content(hunk, TRUE,
+ "value" NL
+ "context" NL
+ "context" NL
+ "context" NL,
+ pool));
+
+ SVN_ERR(check_content(hunk, FALSE,
+ "new value" NL
+ "context" NL
+ "context" NL
+ "context" NL,
+ pool));
+
+ hunk = APR_ARRAY_IDX(hunks, 1 , svn_hunk_t *);
+
+ SVN_ERR(check_content(hunk, TRUE,
+ "context" NL
+ "context" NL
+ "context" NL
"value" NL,
pool));
SVN_ERR(check_content(hunk, FALSE,
+ "context" NL
+ "context" NL
+ "context" NL
"new value" NL,
pool));
