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

On Sat, May 29, 2010 at 9:10 AM, Junio C Hamano [off-list ref] wrote:

Bo Yang [off-list ref] writes:

quoted

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

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.

Hmm, yes, I will move the defensive code upper this check.

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

I think a line in Macintosh will end with '\r'.

What's the reason to strip trailing "\r" at the end of the line to begin
with?

Both '\r' and '\n' will be added back to the strbuf, what I do is
finding the len and make sure the '\r' and \n will not be surround by
the color escape sequence.

-- 
Regards!
Bo
----------------------------
My blog: http://blog.morebits.org
Why Git: http://www.whygitisbetterthanx.com/