> + 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.
> + 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.)
> @@ -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...
> @@ -257,7 +395,7 @@ svn_diff__file_datasource_get_next_token(apr_uint3
... and other factors such as putting return types on the same
line (which some have to do), combined with the syntactic bloat
of languages like C++, make even 50 characters a less-than-ideal
limit.
In favour of retaining a limit of 50, though, diffs will fit
within 80 columns. But is that really a serious consideration?
Are there any other reasons not to attempt to spit out the entire
line, up to a few hundred characters' worth?
Bruce
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Feb 16 07:49:41 2006