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

Re: [PATCH] v6. Fix issue 3459.

From: Daniel Näslund <daniel_at_longitudo.com>
Date: Thu, 10 Sep 2009 21:03:53 +0200

On Sun, Sep 06, 2009 at 09:42:04PM +0100, Stefan Sperling wrote:
> On Sun, Sep 06, 2009 at 09:48:03PM +0200, Daniel Näslund wrote:

> > +/** line-transformer callback to shave leading diff symbols. */
> > +static svn_error_t *
> > +remove_leading_char_filter(svn_stringbuf_t **buf,
>
> I'd call this something other than a 'filter', but whatever :)

Oups. Fixed.

> > @@ -358,8 +374,21 @@
> > }
> >
> > c = line->data[0];
> > - if (c == ' ' || c == '-' || c == '+')
> > - hunk_seen = TRUE;
> > + if (c == ' ' || c == '-')
> > + {
> > + hunk_seen = TRUE;
> > + original_lines--;
>
> What if original_lines wraps around?
> I think you need an 'if (original_lines > 0)' here.

Good point. Fixed.

> > + }
> > + else if (c == '+')
> > + {
> > + hunk_seen = TRUE;
> > + }
> > + /* Should tolerate chopped leading spaces on empty lines. */
>
> s/Should t/T/

Fixed.

> > @@ -418,8 +451,17 @@
> > modified_text = svn_stream_from_aprfile_range_readonly(f, FALSE,
> > start, end,
> > result_pool);
> > +
> > svn_stream_set_line_filter_callback(modified_text, modified_line_filter);
> > +
> > + svn_stream_set_line_transformer_callback(diff_text,
> > + remove_leading_char_filter);
>
> Is the diff text supposed to have chars stripped from it?
> I don't think so. I don't even remember what it's used for...
> Maybe we can just remove the diff_text (in another commit)?

I can't find a use for it either. I thought there were some secret
purpose. I removed the call to the transformer on this one

> > --- subversion/tests/libsvn_diff/parse-diff-test.c (revision 39156)
> > +++ subversion/tests/libsvn_diff/parse-diff-test.c (arbetskopia)
> > @@ -46,7 +46,7 @@
> > "===================================================================" NL
> > "--- A/D/gamma.orig" NL
> > "+++ A/D/gamma" NL
> > - "@@ -1 +1,2 @@" NL
> > + "@@ -1,2 +1 @@"
> > " This is the file 'gamma'." NL
> > "-some less bytes to 'gamma'" NL
> > "" NL
>
> why this change?

The hunk header was wrong. There was initially two lines and one was
removed. Not the other way around.

> > + svntest.actions.run_and_verify_patch(wc_dir, os.path.abspath(patch_file_path),
> > + expected_output,
> > + expected_disk,
> > + expected_status,
> > + expected_skip,
> > + None, # expected err
> > + 1, # check-props
> > + 0) # dry-run
>
> Also unrelated to your patch, but exposed by it: We need to clean up
> run_and_verify_patch(), e.g. the check-props param is probably unused now.

Something else to do!

> > @@ -648,13 +648,9 @@
> > {
> > if (line->len >= 1)
> > {
> > - char c = line->data[0];
> > -
> > - SVN_ERR_ASSERT(c == ' ' || c == '+' || c == '-');
> > - len = line->len - 1;
> > - SVN_ERR(svn_io_file_write_full(file, line->data + 1, len, &len,
> > + len = line->len;
>
> Maybe get rid of the 'len = line->len;' and ..
>
> > + SVN_ERR(svn_io_file_write_full(file, line->data, len, &len,
>
> .. pass (..., line->len, &len, ...) here?

New log message. Mentions the changed hunk header.

[[[
Fix issue 3459: svn patch does not tolerate empty lines of context in
unidiff.

As a side effect, libsvn_client is shielded from the notion of leading
diff symbols on lines.

* subversion/libsvn_diff/parse_diff.c
  (remove_leading_char_filter): New. Line-transformer callback that
    shaves leading diff symbols.
  (svn_diff__parse_next_hunk): Change logic for checking if line is in
    hunk. Set line-transformer callback on original_text and
    modified_text.

* subversion/tests/libsvn_diff/parse-diff-test.c
  (test_parse_unidiff): Change assertions to compare lines without
    leading diff symbols. Changed a faulty hunk header in one of the
    tests.

* subversion/tests/cmdline/patch_tests.py
  (patch_chopped_leading_spaces): New. Same as patch_unidiff() but with
    leading empty spaces removed on empty lines.
  (test_list): Add patch_chopped_leading_spaces().

* subversion/libsvn_client/patch.c
  (match_hunk): Compare whole lines since there is no leading diff
    symbols.
  (copy_hunk_text): Copy whole lines since there is no leading diff
    symbols.
]]]

/Daniel Näslund

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2393400
Received on 2009-09-10 21:04:27 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.