[RFC PATCH 4/4] ui-blame: fill in the contents

[john at keeping.me.uk (John Keeping)]

On Wed, Jun 07, 2017 at 09:18:10PM -0500, Jeff Smith wrote:
> Use the blame interface added in libgit to output the blame information
> of a file in the repository.
> 
> Signed-off-by: Jeff Smith <whydoubt at gmail.com>
> ---
>  cgit.css   |   8 +++
>  ui-blame.c | 214 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 220 insertions(+), 2 deletions(-)
> 
> diff --git a/cgit.css b/cgit.css
> index 1dc2c11..258991c 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -329,6 +329,14 @@ div#cgit table.ssdiff td.lineno a:hover {
>  	color: black;
>  }
>  
> +div#cgit table.blame tr:nth-child(even) {
> +	background: #f7f0e7;
> +}
> +
> +div#cgit table.blame tr:nth-child(odd) {
> +	background: white;
> +}
> +
>  div#cgit table.bin-blob {
>  	margin-top: 0.5em;
>  	border: solid 1px black;
> diff --git a/ui-blame.c b/ui-blame.c
> index 901ca89..6ad0009 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -10,6 +10,187 @@
>  #include "ui-blame.h"
>  #include "html.h"
>  #include "ui-shared.h"
> +#include "argv-array.h"
> +#include "blame.h"
> +
> +
> +/* Remember to update object flag allocation in object.h */

This comment doesn't really apply in CGit, but it looks like the define
is entirely unused, so we can just delete it.

> +#define MORE_THAN_ONE_PATH	(1u<<13)
> +
> +/*
> + * Information on commits, used for output.
> + */

Can we reuse our existing struct commitinfo here?  I can't see anything
that this function does that our existing cgit_parse_commit() doesn't do
and cgit_parse_commit() deals with encoding properly as well.

> +struct commit_info {
> +	struct strbuf author;
> +	struct strbuf author_mail;
> +	struct ident_split author_ident;
> +
> +	/* filled only when asked for details */
> +	struct strbuf committer;
> +	struct strbuf committer_mail;
> +	struct ident_split committer_ident;
> +
> +	struct strbuf summary;
> +};
> +
[snip parsing details]
> +static void get_commit_info(struct commit *commit,
> +			    struct commit_info *ret,
> +			    int detailed)

This is only ever called with detailed == 1.

> +{
> +	int len;
> +	const char *subject, *encoding;
> +	const char *message;
> +
> +	commit_info_init(ret);
> +
> +	encoding = get_log_output_encoding();
> +	message = logmsg_reencode(commit, NULL, encoding);
> +	get_ac_line(message, "\nauthor ",
> +		    &ret->author, &ret->author_mail,
> +		    &ret->author_ident);
> +
> +	if (!detailed) {
> +		unuse_commit_buffer(commit, message);
> +		return;
> +	}
> +
> +	get_ac_line(message, "\ncommitter ",
> +		    &ret->committer, &ret->committer_mail,
> +		    &ret->committer_ident);
> +
> +	len = find_commit_subject(message, &subject);
> +	if (len)
> +		strbuf_add(&ret->summary, subject, len);
> +	else
> +		strbuf_addf(&ret->summary, "(%s)", oid_to_hex(&commit->object.oid));
> +
> +	unuse_commit_buffer(commit, message);
> +}

[snip most of the code, which all looks entirely reasonable.]

> @@ -76,7 +278,15 @@ static void print_object(const unsigned char *sha1, const char *path, const char
>  		return;
>  	}
>  
> -	/* Output data here */
> +	html("<table class='blame blob'>");
> +	for (ent = sb.ent; ent; ) {
> +		struct blame_entry *e = ent->next;
> +		emit_blame_entry(&sb, ent);
> +		free(ent);
> +		ent = e;
> +	}
> +	html("</table>\n");
> +	free((void *)sb.final_buf);

There's no need to cast to void* in C, so drop the cast here.

>  
>  	cgit_print_layout_end();
>  	return;
> -- 
> 2.9.3