On Thu, Jun 17, 2010 at 5:54 PM, C. Michael Pilato <cmpil...@collab.net> wrote:
> Julian Foad wrote:
>> My recommendation: Let's strive for compatibility where we can, and
>> where the old behaviour is reasonable. I think a 6-character minimum
>> column width is reasonable for most purposes, even though it's not the
>> purest design.
>
> +1
Ok, thanks for the feedback, all.
In attachment a new version of the patch, which is compatible with the
old behavior for sub-1000000 end_revnum's. make check now passes. I've
also added a comment in the source explaining what's going on with the
6 and the 1000000 (feel free to edit/lose that comment if you want).
And an updated log message:
[[[
Make "svn blame" use a consistent column width even when revision
numbers are >= 1000000.
* subversion/include/svn_client.h
(svn_client_blame_receiver3_t): Add parameters start_revnum and end_revnum,
useful to the blame receiver in formatting its output.
* subversion/libsvn_client/blame.c
(svn_client_blame5): Pass start_revnum and end_revnum to the blame receiver.
* subversion/svn/blame-cmd.c
(blame_receiver_xml): Implement the updated svn_client_blame_receiver3_t.
(blame_receiver): Implement the updated svn_client_blame_receiver3_t, and
pass end_revnum to print_line_info.
(print_line_info): Add parameter end_revnum, use it to increase the column
width for the revision number if needed.
* subversion/libsvn_client/deprecated.c
(blame_wrapper_receiver2): Implement the updated
svn_client_blame_receiver3_t.
]]]
Cheers,
--
Johan
Index: subversion/svn/blame-cmd.c
===================================================================
--- subversion/svn/blame-cmd.c (revision 955714)
+++ subversion/svn/blame-cmd.c (working copy)
@@ -51,6 +51,8 @@
XML to stdout. */
static svn_error_t *
blame_receiver_xml(void *baton,
+ svn_revnum_t start_revnum,
+ svn_revnum_t end_revnum,
apr_int64_t line_no,
svn_revnum_t revision,
apr_hash_t *rev_props,
@@ -117,14 +119,27 @@
const char *date,
const char *path,
svn_boolean_t verbose,
+ svn_revnum_t end_revnum,
apr_pool_t *pool)
{
const char *time_utf8;
const char *time_stdout;
- const char *rev_str = SVN_IS_VALID_REVNUM(revision)
- ? apr_psprintf(pool, "%6ld", revision)
- : " -";
+ const char *rev_str;
+ int rev_maxlength;
+ /* The standard column width for the revision number is 6 characters.
+ If the revision number can potentially be larger (i.e. if the end_revnum
+ is larger than 1000000), we increase the column width as needed. */
+ rev_maxlength = 6;
+ while (end_revnum >= 1000000)
+ {
+ rev_maxlength++;
+ end_revnum = end_revnum / 10;
+ }
+ rev_str = SVN_IS_VALID_REVNUM(revision)
+ ? apr_psprintf(pool, "%*ld", rev_maxlength, revision)
+ : apr_psprintf(pool, "%*s", rev_maxlength, "-");
+
if (verbose)
{
if (date)
@@ -162,6 +177,8 @@
/* This implements the svn_client_blame_receiver3_t interface. */
static svn_error_t *
blame_receiver(void *baton,
+ svn_revnum_t start_revnum,
+ svn_revnum_t end_revnum,
apr_int64_t line_no,
svn_revnum_t revision,
apr_hash_t *rev_props,
@@ -199,14 +216,16 @@
SVN_PROP_REVISION_AUTHOR),
svn_prop_get_value(merged_rev_props,
SVN_PROP_REVISION_DATE),
- merged_path, opt_state->verbose, pool));
+ merged_path, opt_state->verbose, end_revnum,
+ pool));
else
SVN_ERR(print_line_info(out, revision,
svn_prop_get_value(rev_props,
SVN_PROP_REVISION_AUTHOR),
svn_prop_get_value(rev_props,
SVN_PROP_REVISION_DATE),
- NULL, opt_state->verbose, pool));
+ NULL, opt_state->verbose, end_revnum,
+ pool));
return svn_stream_printf(out, pool, "%s%s", line, APR_EOL_STR);
}
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 955714)
+++ subversion/include/svn_client.h (working copy)
@@ -681,7 +681,12 @@
* which has the revision properties @a rev_props, and that the contents were
* @a line.
*
- * If svn_client_blame4() was called with @a include_merged_revisions set to
+ * @a start_revnum and @a end_revnum contain the start and end revision
+ * number of the entire blame operation, as determined from the repository
+ * inside svn_client_blame5(). This can be useful for the blame receiver
+ * to format the blame output.
+ *
+ * If svn_client_blame5() was called with @a include_merged_revisions set to
* TRUE, @a merged_revision, @a merged_rev_props and @a merged_path will be
* set, otherwise they will be NULL. @a merged_path will be set to the
* absolute repository path.
@@ -697,6 +702,8 @@
*/
typedef svn_error_t *(*svn_client_blame_receiver3_t)(
void *baton,
+ svn_revnum_t start_revnum,
+ svn_revnum_t end_revnum,
apr_int64_t line_no,
svn_revnum_t revision,
apr_hash_t *rev_props,
Index: subversion/libsvn_client/deprecated.c
===================================================================
--- subversion/libsvn_client/deprecated.c (revision 955714)
+++ subversion/libsvn_client/deprecated.c (working copy)
@@ -119,6 +119,8 @@
static svn_error_t *
blame_wrapper_receiver2(void *baton,
+ svn_revnum_t start_revnum,
+ svn_revnum_t end_revnum,
apr_int64_t line_no,
svn_revnum_t revision,
apr_hash_t *rev_props,
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c (revision 955714)
+++ subversion/libsvn_client/blame.c (working copy)
@@ -787,13 +787,16 @@
if (!eof || sb->len)
{
if (walk->rev)
- SVN_ERR(receiver(receiver_baton, line_no, walk->rev->revision,
+ SVN_ERR(receiver(receiver_baton, start_revnum, end_revnum,
+ line_no, walk->rev->revision,
walk->rev->rev_props, merged_rev,
merged_rev_props, merged_path,
sb->data, FALSE, iterpool));
else
- SVN_ERR(receiver(receiver_baton, line_no, SVN_INVALID_REVNUM,
- NULL, SVN_INVALID_REVNUM, NULL, NULL,
+ SVN_ERR(receiver(receiver_baton, start_revnum, end_revnum,
+ line_no, SVN_INVALID_REVNUM,
+ NULL, SVN_INVALID_REVNUM,
+ NULL, NULL,
sb->data, TRUE, iterpool));
}
if (eof) break;
