From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Sat, 21 Oct 2017 15:43:25 +0100
Subject: [PATCH 4/4] ui-blame: Allow syntax highlighting
In-Reply-To: <20171018041735.31592-5-whydoubt@gmail.com>
References: <20171018041735.31592-1-whydoubt@gmail.com>
 <20171018041735.31592-5-whydoubt@gmail.com>
Message-ID: <20171021144325.GF2393@john.keeping.me.uk>

On Tue, Oct 17, 2017 at 11:17:35PM -0500, Jeff Smith wrote:
> Place file contents into a single block so that syntax highlighting can
> be applied in the usual fashion.  Place the alternating color bars
> behind the file contents.  Force the default syntax highlighting
> background to transparent.
> 
> Signed-off-by: Jeff Smith <whydoubt at gmail.com>

Other than a couple of minor comments below, this looks reasonable to
me.

It does suffer the same drawback as the normal tree view with source
highlighting in that a little care is needed to avoid the line numbers
and content getting out of step (try adding "font-size: larger" to one
of the syntax highlighting CSS rules!), but since we already accept that
there I think we can accept it here as well.

> ---
>  cgit.css                       | 10 ++++++++
>  filters/syntax-highlighting.py |  2 +-
>  ui-blame.c                     | 55 +++++++++++++++++++++++++++++++++---------
>  3 files changed, 54 insertions(+), 13 deletions(-)
> 
> diff --git a/cgit.css b/cgit.css
> index 20b7e86..217a05a 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -353,6 +353,16 @@ div#cgit table.blame div.alt:nth-child(odd) {
>  	background: white;
>  }
>  
> +div#cgit table.blame td.lines > div {
> +	position: relative;
> +}
> +
> +div#cgit table.blame td.lines > div > pre {
> +	padding: 0 0 0 0.5em;
> +	position: absolute;
> +	top: 0;
> +}
> +
>  div#cgit table.bin-blob {
>  	margin-top: 0.5em;
>  	border: solid 1px black;
> diff --git a/filters/syntax-highlighting.py b/filters/syntax-highlighting.py
> index 5888b50..e912594 100755
> --- a/filters/syntax-highlighting.py
> +++ b/filters/syntax-highlighting.py
> @@ -34,7 +34,7 @@ sys.stdin = io.TextIOWrapper(sys.stdin.buffer, encoding='utf-8', errors='replace
>  sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8', errors='replace')
>  data = sys.stdin.read()
>  filename = sys.argv[1]
> -formatter = HtmlFormatter(style='pastie')
> +formatter = HtmlFormatter(style='pastie', nobackground=True)
>  
>  try:
>  	lexer = guess_lexer_for_filename(filename, data)
> diff --git a/ui-blame.c b/ui-blame.c
> index f506616..574c3ee 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -67,15 +67,21 @@ static void emit_blame_entry_linenumber(struct blame_entry *ent)
>  		htmlf(numberfmt, ++lineno);
>  }
>  
> -static void emit_blame_entry_line(struct blame_scoreboard *sb,
> -				  struct blame_entry *ent)
> +static void emit_blame_entry_line_background(struct blame_scoreboard *sb,
> +					     struct blame_entry *ent)
>  {
> -	const char *cp, *cpend;
> +	unsigned long line;
> +	size_t len, maxlen = 2;
>  
> -	cp = blame_nth_line(sb, ent->lno);
> -	cpend = blame_nth_line(sb, ent->lno + ent->num_lines);
> +	for (line = ent->lno; line < ent->lno + ent->num_lines; line++) {
> +		html("\n");
> +		len = blame_nth_line(sb, line + 1) - blame_nth_line(sb, line);

This doesn't account for tabs, which is noticable in long lines (I
happened to test with cgit.c where line 615 is quite a bit longer than
average).

It is fixed by using:

	const char *start = blame_nth_line(sb, line);
	const char *end = blame_nth_line(sb, line + 1);

	html("\n");
	len = end - start;
	while (start < end)
		if (*(start++) == '\t')
			len += 7;

> +		if (len > maxlen)
> +			maxlen = len;
> +	}
>  
> -	html_ntxt(cp, cpend - cp);
> +	for (len = 0; len < maxlen - 1; len++)
> +		html(" ");

Should this use &nbsp; or is a plain space guaranteed to be okay here?

>  }
>  
>  struct walk_tree_context {
> @@ -88,6 +94,7 @@ static void print_object(const unsigned char *sha1, const char *path,
>  			 const char *basename, const char *rev)
>  {
>  	enum object_type type;
> +	char *buf;
>  	unsigned long size;
>  	struct argv_array rev_argv = ARGV_ARRAY_INIT;
>  	struct rev_info revs;
> @@ -102,6 +109,13 @@ static void print_object(const unsigned char *sha1, const char *path,
>  		return;
>  	}
>  
> +	buf = read_sha1_file(sha1, &type, &size);
> +	if (!buf) {
> +		cgit_print_error_page(500, "Internal server error",
> +			"Error reading object %s", sha1_to_hex(sha1));
> +		return;
> +	}
> +
>  	argv_array_push(&rev_argv, "blame");
>  	argv_array_push(&rev_argv, rev);
>  	init_revisions(&revs, NULL);
> @@ -157,20 +171,37 @@ static void print_object(const unsigned char *sha1, const char *path,
>  		html("</td>\n");
>  	}
>  
> -	/* Lines */
> -	html("<td class='lines'>");
> +	html("<td class='lines'><div>");
> +
> +	/* Colored bars behind lines */
> +	html("<div>");
>  	for (ent = sb.ent; ent; ) {
>  		struct blame_entry *e = ent->next;
> -		html("<div class='alt'><pre><code>");
> -		emit_blame_entry_line(&sb, ent);
> -		html("</code></pre></div>");
> +		html("<div class='alt'><pre>");
> +		emit_blame_entry_line_background(&sb, ent);
> +		html("</pre></div>");
>  		free(ent);
>  		ent = e;
>  	}
> -	html("</td>\n");
> +	html("</div>");
>  
>  	free((void *)sb.final_buf);
>  
> +	/* Lines */
> +	html("<pre><code>");
> +	if (ctx.repo->source_filter) {
> +		char *filter_arg = xstrdup(basename);
> +		cgit_open_filter(ctx.repo->source_filter, filter_arg);
> +		html_raw(buf, size);
> +		cgit_close_filter(ctx.repo->source_filter);
> +		free(filter_arg);
> +	} else {
> +		html_txt(buf);
> +	}
> +	html("</code></pre>");
> +
> +	html("</div></td>\n");
> +
>  	html("</tr>\n</table>\n");
>  
>  	cgit_print_layout_end();