Thread (3 messages) flat view 3 messages, 2 authors, 2016-06-15

Re: [PATCH v3] Give the hunk comment its own color

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:47

Bert Wesarg [off-list ref] writes:
quoted hunk
 diff.c                   |   64 +++++++++++++++++++++++++++++++++++++++++++--
...
@@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,
 	}
 }
 
+static void emit_hunk_line(struct emit_callback *ecbdata,
+			   const char *line, int len)
+{
+	const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
+	const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
+	const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
+	const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
+	const char *orig_line = line;
+	int orig_len = len;
+	const char *frag_start;
+	int frag_len;
+	const char *part_end = NULL;
+	int part_len = 0;
+
+	/* determine length of @ */
+	while (part_len < len && line[part_len] == '@')
+		part_len++;
+
+	/* find end of frag, (Ie. find second @@) */
+	part_end = memmem(line + part_len, len - part_len,
+			  line, part_len);
This is not incorrect per-se, but probably is overkill; this codepath only
deals with two-way diff and we know we are looking at "@@ -..., +... @@"
at this point.

	part_end = memmem(line + 2, len - 2, "@@", 2);

would be sufficient.
+	if (!part_end)
+		return emit_line(ecbdata->file, frag, reset, line, len);
+	/* calculate total length of frag */
+	part_len = (part_end + part_len) - line;
+
+	/* remember frag part, we emit only if we find a space separator */
+	frag_start = line;
+	frag_len = part_len;
+
+	/* consume hunk header */
+	len -= part_len;
+	line += part_len;
+
+	/*
+	 * for empty reminder or empty space sequence (exclusive any newlines
+	 * or carriage returns) emit complete original line as FRAGINFO
+	 */
+	if (!len || !(part_len = strspn(line, " \t")))
Slightly worrisome is what guarantees this strspn() won't step outside
len.

I would probably write the function like this instead.

-- >8 --

static void emit_hunk_header(struct emit_callback *ecbdata,
			     const char *line, int len)
{
	const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
	const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
	const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
	const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
	static const char atat[2] = { '@', '@' };
	const char *cp, *ep;

	/*
	 * As a hunk header must begin with "@@ -<old>, +<new> @@",
	 * it always is at least 10 bytes long.
	 */
	if (len < 10 ||
	    memcmp(line, atat, 2) ||
	    !(ep = memmem(line + 2, len - 2, atat, 2))) {
		emit_line(ecbdata->file, plain, reset, line, len);
		return;
	}
	ep += 2; /* skip over the second @@ */

	/* The hunk header in fraginfo color */
	emit_line(ecbdata->file, frag, reset, line, ep - line);

	/* blank before the func header */
	for (cp = ep; ep - line < len; ep++)
		if (*ep != ' ' && *ep != 't')
			break;
	if (ep != cp)
		emit_line(ecbdata->file, plain, reset, cp, ep - cp);

	if (ep < line + len)
		emit_line(ecbdata->file, func, reset, ep, line + len - ep);
}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help