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

Re: [PATCH v4 4/6] Emit a whole line once a time

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

Bo Yang [off-list ref] writes:
Since the graph prefix will be printed when calling
emit_line, so the functions should be used to emit a
complete line out once a time. No one should call
emit_line to just output some strings instead of a
complete line.
Use a strbuf to compose the whole line, and then
call emit_line to output it once.
"once a time" in your title doesn't sound quite right.  I would say "in
one go" instead.
quoted hunk ↗ jump to hunk
Signed-off-by: Bo Yang <redacted>
---
 diff.c |   34 +++++++++++++++++++++++++++++-----
 1 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/diff.c b/diff.c
index 7f2538d..bffaedc 100644
--- a/diff.c
+++ b/diff.c
@@ -370,6 +370,18 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
 	const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
 	static const char atat[2] = { '@', '@' };
 	const char *cp, *ep;
+	struct strbuf msgbuf = STRBUF_INIT;
+	int org_len = len;
+
+	/*
+	 * trailing \r\n
+	 */
+	int i = 1;
+	for (; i < 3; i++) {
+		if (line[len - i] == '\r' || line[len - i] == '\n') {
+			len --;
+		}
+	}
I am not very happy with this logic.  The existing code (just outside the
post-context of this hunk) is being defensive and returns early when len
is shorter than what we expect, but this new code blindly assumes that len
is at least 2 bytes long, and also it would eat a line that ends with \r\r.

Can the partial line at the end be on this line?  IOW, can line[len-1] be
different from '\n' in some cases?

What's the reason to strip trailing "\r" at the end of the line to begin
with?
 	/*
 	 * As a hunk header must begin with "@@ -<old>, +<new> @@",
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help