Re: MT 'blame' and 'log' Auditing - Design Specification

Mark Phippard wrote:
> I still do not agree with this part of the spec:
>
> svn blame
> Two additional columns for each line, with the original revision
> and author of that line. Unlike other commands, we do not need to
> worry about multiple source revisions, because each line can have at
> most one author.
>
>
> I object to the two additional columns. If I run svn blame
> --merge-sensitive I do not need to see any other information. I
> thought at one point we even said that were it not for the expected
> performance hit in getting the merge information that svn blame should
> just always return this information. When using blame it is not
> likely that someone would want any other information.
>
> The reason I object is just that all the GUI tools out there that
> already work with blame output will be able to do so easier if a
> command line option simply alters the data we get back as opposed to
> giving us an all new format to deal with.

I don't understand. Do you call the svn client from Subclipse and parse
its output? I don't think you do it that way, so why bother with how the
svn client will show the additional information? It shouldn't bother you
how many columns there are since you would be using the blame callback
which I think will simply have some more callback params to use.

Or am I completely wrong here?

Reading the specs, I guess the new blame callback will change from

svn_error_t* blameReceiver(void* baton, apr_off_t line_no, svn_revnum_t
revision, const char * author, const char * date, const char * line,
apr_pool_t * pool)

so something like this:
svn_error_t* blameReceiver(void* baton, apr_off_t line_no, svn_revnum_t
revision, const char * author, const char * date,
apr_off_t orig_line_no, svn_revnum_t orig_revision, const char *
orig_author, const char * orig_date, const char * line, apr_pool_t * pool)

Then it is up to the GUI clients to make use of that new information.
They can decide how to show it - maybe not as an additional column but
in a tooltip control or something else.

I have one request for the new API:
please change the 'date' param from a const char * to an apr_time_t - I
don't like to parse the date from a string just to get a 'real' date.

Now my comments to the specs:

"svn info, svn ls --verbose and svn status --show-updates are purposely
not included in this list, on the grounds that one would typically need
more information than they can reasonably provide. Adding more output to
these commands would clutter their interface, further reducing their
utility."

I think here I disagree. I'm ok with this if you're only referring to
the command line client. But the API should still provide that
information if it's available. You don't know how other (UI) clients
could make good use of such information. Always remember, an UI client
isn't limited to 80 chars screen width.

Especially the svn_client_status() function should return that
information if it is available.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org