Hi,

Please find in attachment my (very first) patch addressing the issue
discussed in [1] and [2].

Log message (maybe a bit too verbose ...):
[[[
Fix alignment of blame output containing revision numbers >= 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 determine the column
   width for the revision number.
* subversion/libsvn_client/deprecated.c
  (blame_wrapper_receiver2): Implement the updated
   svn_client_blame_receiver3_t.
]]]

There are some caveats:
1) This patch changes the svn_client_blame_receiver3_t interface
(already new in 1.7). I think this breaks the bindings.

2) This patch causes the following test failures:
FAIL:  basic_tests.py 42: basic relative url target using current dir
FAIL:  basic_tests.py 43: basic relative url target using other targets
FAIL:  blame_tests.py 6: blame targets with peg-revisions
FAIL:  blame_tests.py 8: ignore whitespace when blaming
FAIL:  blame_tests.py 9: ignore eol styles when blaming
FAIL:  blame_tests.py 12: blame target not in HEAD with peg-revisions
FAIL:  blame_tests.py 14: blame -g output with inserted lines

That's because the revision number column in the blame output now only
has the minimum width necessary (previously this was always fixed to a
width of 6). So for low revision numbers this now comes out as:

[[[
1    jrandom This is the file 'iota'.
]]]

instead of (previous behavior):

[[[
     1    jrandom This is the file 'iota'.
]]]

I see two ways to fix this:
- Change the tests
- Change the patch, so that it is backwards compatible with the old
blame output for revision numbers <1000000 (i.e. always use minimum
width of 6, and larger if needed).

Comments, review, ... shoot.

Cheers,
-- 
Johan

[1] http://svn.haxx.se/dev/archive-2010-06/0175.shtml
[2] http://svn.haxx.se/dev/archive-2010-04/0463.shtml
Index: subversion/svn/blame-cmd.c
===================================================================
--- subversion/svn/blame-cmd.c  (revision 955339)
+++ 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,24 @@
                 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 = 0;
 
+  while (end_revnum != 0)
+    {
+      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 +174,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 +213,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 955339)
+++ 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 955339)
+++ 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 955339)
+++ 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;

Reply via email to