From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Sat, 22 Jul 2017 13:02:27 +0100
Subject: [RFC PATCH 0/4] Add ui-blame
In-Reply-To: <20170608021810.12964-1-whydoubt@gmail.com>
References: <20170608021810.12964-1-whydoubt@gmail.com>
Message-ID: <20170722120227.GI1600@john.keeping.me.uk>

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.