On Thu, 16 Feb 2006, Bruce DeVisser wrote:
> > + default:
> > + /* Some other whitespace character. */
> > + if (opts->ignore_space)
> > + {
> > + state = state_whitespace;
> > + if (opts->ignore_space
> > + == svn_diff_file_ignore_space_change)
> > + *newend++ = ' ';
> > + }
> > + break;
>
> This appears to implement a purist definition of space-change
> (whitespace at end-of-line is not folded into the newline),
> unlike GNU diff. Not that there's anything wrong with that; just
> pointing it out.
>
I first implemented ignore-space-at-eol-totally like GNU diff, but it
didn't work since I am normalizing in-place. If people think this is
important, I see no other way than using an extra buffer.
>
> > + case state_cr:
> > + if (*curp == '\n' && opts->ignore_eol_style)
> > + start = curp + 1;
> > + state = state_normal;
> > + }
>
> Mac text (CR-only) will foul up space-ignore logic for whitespace
> at the start of the next line, I think. (Sorry, I can't test it
> right now.)
>
Good catch! It would include a space at the beginning of the next line
unconditionally. I fixed this by (ab)using the C switch statement.
>
> > @@ -257,7 +395,7 @@ svn_diff__file_datasource_get_next_token(apr_uint3
>
> > +/* Maximum length of the extra context to show when show_c_function is set.
> > + GNU diff uses 40, let's be brave and use 50 instead. */
> > +#define SVN_DIFF__EXTRA_CONTEXT_LENGTH 50
>
> Consider going for at least 80. I imagine that short symbol names
> must have still been in vogue when diff -p was originally coded,
> but it's good to get with the times. You already have a
> 40-character function name here, in the inaugural use of the
> patch...
>
I choose a compromise between longer line and trying to not going over 80
columns, but hey, that's silly, since long filenames will go over 80
columns anyway. So just outputtting up to say, 200 characters seems
reasonable. This functionality (er, hack) only works in some special, but
common, cases anyway.
Thanks for the review,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Feb 16 09:45:14 2006