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

Re: [PATCH] Add -p to show C function names was Re: [PATCH] Extra options for libsvn_diff

From: Bruce DeVisser <bmdmail_at_sympatico.ca>
Date: 2007-12-27 16:08:16 CET

On Wed, Dec 26, 2007 at 11:30:07AM -0800, Justin Erenkrantz wrote:
> (SVN_DIFF__EXTRA_CONTEXT_LENGTH): Define to 50.
[snip]
> +/* 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

Going back into my own time machine:

On Thu Feb 16, 2006, I wrote this in response to Peter Lundblad's
initial implementation:

> @@ -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...

The same day, Peter Samuelson replied:

> > +/* 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'd vote for a buffer of 64 chars, but then truncating the whole output
 line at 79 chars. After the @@...@@ you're left with 55 to 63 chars.

Peter's response seems to assume that the @@ lines should have a
length of less than 80 characters. I don't agree.

  * diff can put out long lines if the source code has lines
    longer than 80 characters (not everyone uses an 80 column
    convention). Since diff *can* put out lines longer than 80
    columns, it seems arbitrary to chop the -p output to ensure
    fewer than 80 columns.

  * The -p option is useful for C++ developers, but class names
    can take up a lot of characters, leaving the actual function
    name much more truncated than what you would ever see in
    straight C code. (This is a frustration with diff -up.)

  * A simple script can chop the output to 80 columns if
    required, but it is much more complex to resurrect a chopped
    function name. In other words, the frustration is manageable
    with a higher SVN_DIFF__EXTRA_CONTEXT_LENGTH than with a
    lower one.

I don't want to start a war about this, and will concede to just
about anything as long as this useful patch goes in; but from
this user's point of view, the higher the value of
SVN_DIFF__EXTRA_CONTEXT_LENGTH, the better.

Bruce

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Dec 27 16:08:43 2007

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