From: john at keeping.me.uk (John Keeping)
Subject: [PATCH 4/4] ui-blame: Allow syntax highlighting
Date: Sat, 21 Oct 2017 15:43:25 +0100 [thread overview]
Message-ID: <20171021144325.GF2393@john.keeping.me.uk> (raw)
In-Reply-To: <20171018041735.31592-5-whydoubt@gmail.com>
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 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();
next prev parent reply other threads:[~2017-10-21 14:43 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-18 4:17 [PATCH 0/4] Add ui-blame " whydoubt
2017-10-18 4:17 ` [PATCH 1/4] ui-blame: Distinguish hashes column from lines column whydoubt
2017-10-21 14:33 ` john
2017-10-18 4:17 ` [PATCH 2/4] ui-blame: Break out emit_blame_entry into component methods whydoubt
2017-10-21 14:33 ` john
2017-10-18 4:17 ` [PATCH 3/4] ui-blame: Make each column into a single table cell whydoubt
2017-10-21 14:34 ` john
2017-10-18 4:17 ` [PATCH 4/4] ui-blame: Allow syntax highlighting whydoubt
2017-10-21 14:43 ` john [this message]
2017-10-21 21:53 ` whydoubt
2017-10-24 14:30 ` Jason
2017-10-29 2:23 ` [PATCH 4/4 v2] " whydoubt
2017-10-29 2:26 ` whydoubt
2017-10-29 2:43 ` [PATCH 4/4 v3] " whydoubt
2017-10-29 22:23 ` Jason
2017-11-05 12:36 ` john
2018-01-19 10:41 ` [PATCH 0/4] Add ui-blame " Jason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171021144325.GF2393@john.keeping.me.uk \
--to=cgit@lists.zx2c4.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).