[RFC PATCH 0/4] Add ui-blame
John Keeping
john at keeping.me.uk
Sat Jul 22 14:02:27 CEST 2017
On Wed, Jun 07, 2017 at 09:18:06PM -0500, Jeff Smith wrote:
> I split git blame functionality into libgit, and the changes were
> accepted upstream a few days ago. Now that the git infrastructure is in
> place, it is time to get back to the cgit part.
>
> The first patch advances git to current master (which should be future
> v2.14), and makes changes made necessary by doing so. The remaining
> patches deal with adding the new blame functionality.
>
> Based on branch ch/git-2-13-1.
>
> Jeff Smith (4):
> git: update to v2.14
> ui-blame: create placeholder and links
> ui-blame: create needed html_ntxt_noellipsis function
> ui-blame: fill in the contents
This looks like a good start. I've commented on the individual patches,
but apart from the patch order and struct commit_info question they're
all minor.
I have a general question, possible more for any administrators of CGit
sites who happen to see this: Should we offer a configuration switch to
disable the blame function? Blame is a comparatively expensive
operation compared to everything else we do to display a page, so is
there a desire to disable this feature for sites worried about resource
usage?
Finally, some general comments on the output formatting in the browser:
- Should the commit SHA be fixed-width? I'm comparing the output with
http://repo.or.cz/git.git/blame_incremental/HEAD:/branch.c which I
think is a bit easier to read.
- The tooltip looks very confusing since the labels and values aren't
aligned. I'd simplify it to show either the author's name and date
(like GitWeb in the link above) or just the commit message.
More information about the CGit
mailing list