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);
}
On Sat, Nov 28, 2009 at 06:52, Junio C Hamano [off-list ref] wrote:
Bert Wesarg [off-list ref] writes:
quoted
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.
Thats right, I made it generic by purpose.
quoted
+ 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.
Thats a valid concern and should be addressed.
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);
}
Please check that its really an *ep != '\t'. Its wrong in this mail, I
see only an *ep != 't'. Otherwise:
Acked-by: Bert.Wesarg@googlemail.com
quoted
/* blank before the func header */
for (cp = ep; ep - line < len; ep++)
if (*ep != ' ' && *ep != 't')
Please check that its really an *ep != '\t'. Its wrong in this mail, I
see only an *ep != 't'.
Obviously, you have not checked it. Please squash this in:
diff --git i/diff.c w/diff.c
index eaa1983..e126304 100644
--- i/diff.c
+++ w/diff.c
@@ -376,7 +376,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
/* blank before the func header */
for (cp = ep; ep - line < len; ep++)
- if (*ep != ' ' && *ep != 't')
+ if (*ep != ' ' && *ep != '\t')
break;
if (ep != cp)
emit_line(ecbdata->file, plain, reset, cp, ep - cp);
Bert