On Fri, Apr 9, 2010 at 10:04 PM, Johan Corveleyn <jcor...@gmail.com> wrote:
> On Fri, Apr 9, 2010 at 10:37 PM, Hyrum K. Wright > <hyrum_wri...@mail.utexas.edu> wrote: > > > > > > On Fri, Apr 9, 2010 at 9:17 PM, Johan Corveleyn <jcor...@gmail.com> > wrote: > >> > >> Hi devs, > >> > >> Just a heads up, not really important, but I came across this in > >> blame_cmd.c (inside print_line_info): > >> > >> [[[ > >> const char *rev_str = SVN_IS_VALID_REVNUM(revision) > >> ? apr_psprintf(pool, "%6ld", revision) > >> : " -"; > >> > >> ]]] > >> > >> Which means that users of a repo with over 1 million revisions will > >> see misaligned blame output ... > >> > >> So I guess this is coming up for you guys when s.a.o reaches the 1 > >> million mark :-). > > > > Heh. We also have a similar problem when committer names are longer than > a > > given threshold. > > Indeed, 10 characters: > [[[ > return svn_stream_printf(out, pool, "%s %10s ", rev_str, > author ? author : " -"); > ]]] > > > I've thought about hacking a solution into the client, but > > have never really gotten around to it. Do you want to write a patch? > > Hm, yes. It might be a good opportunity for me to get my feet wet (I'm > actually looking at making blame faster, cf. mail thread of a couple > of weeks ago; but it might be a good idea to try something simpler > first :-)). > > However, not having thought long about this problem: what would be the > desired behavior? > > Should I first determine the maximum length of rev_str and author for > the entire file and then use that as the column width (maybe with a > minimum of 6 for rev_str and 10 for author, like it is now) ? That would be my initial guess. Blame itself has enough overhead that adding this calculation shouldn't be too difficult. It would also give you a bit of exposure to the blame code itself. This may be overkill; I'll let you do the exercise to find out. :) -Hyrum