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

Re: [PATCH] Add verbose support to blame

From: Ben Reser <ben_at_reser.org>
Date: 2004-02-16 07:31:20 CET

On Sun, Feb 15, 2004 at 06:37:16PM -0800, Ben Reser wrote:
> On Sat, Feb 14, 2004 at 07:11:29PM -0600, Ben Gollmer wrote:
> > I was reading the thread 'annotate support for emacs', and I thought
> > I'd give this patching thing a shot. Yes, the date format is verbose,
> > but that's the way libsvn_subr/time.c said to do it.
> >
> > Log:
> > * clients/cmdline/main.c: Add '-v | --verbose' switch support to
> > svn blame.
> > * clients/cmdline/blame-cmd.c
> > (svn__cl_blame): Pass the verbose argument to svn_client_blame().
> > (blame_reciever): If the verbose argument is present, format the
> > date and include it in the output.
> > * include/svn_client.h: Add 'svn_boolean_t verbose' argument to
> > the declaration of svn_client_blame().
> > * libsvn_client/blame.c
> > (svn_client_blame): Pass the verbose argument to blame receiver.
>
> You changed the function signature. So this isn't going to go into
> 1.0.x. You'd have to make a svn_client_blame_verbose() to put it in
> 1.0.x.

Actually on further thought this implementation is completely wrong.
The blame receiver already gets the date. Output options don't belong
in our API, we're calling a receiver for the client to implement the
output anyway it chooses. A client has two potential ways of handling
situations like this:

a) Two blame receiver implementations. One for verbose and non-verbose
output. It just passes a different receiver to blame based on which one
it wnats.

b) One blame receiver and it uses the baton we provide to pass the
verbose flag through.

The later of which is IMHO the better option in this case since the
implementation difference is tiny. Neither of these changes require any
change in our public API, they only change implementation details of our
client. Then the only questions are:

1) Is it okay to add an enhancement in a patch release. Which I think
the answer to depends on the enhancement.

2) Is it okay to add a command line option in a patch release? This is
stickier. Do we want to offer the same compatability guarantees to
scripts (forwards and backwards between patch releases)? I don't think
this is necessary, scripts can more easily be written to detect the
functionality available and to use what they can or can't.

I'm inclined to say that both of these would be okay in this
circumstance. The change would be relatively minor from the end user
standpoint, makes it possible to make the annotation feature in emacs
work better/right.

-- 
Ben Reser <ben@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Feb 16 07:31:36 2004

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.