git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Bo Yang <struggleyb.nku@gmail.com>
Date
May 29, 2010, 14:19 UTC
Message-ID
<AANLkTin5KJs-3rEbPgLWRnEsTtlkmq7QTb-t0qEKAg78@mail.gmail.com>
In-Reply-To
<7vocfz5x80.fsf@alter.siamese.dyndns.org>
On Sat, May 29, 2010 at 9:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 43 quoted lines
> Bo Yang <struggleyb.nku@gmail.com> 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.
>
>> Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
>> ---
>>  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/
Previous: Junio C Hamano
Message 7 of 7 in “diff.c: Output the text graph padding before each diff line”
  1. 3/6 diff.c: Output the text graph padding before each diff lineBo Yang, May 26, 2010
  2. 4/6 Emit a whole line once a timeBo Yang, May 26, 2010
  3. 5/6 Register a callback for graph outputBo Yang, May 26, 2010
  4. 6/6 Make --color-words work well with --graphBo Yang, May 26, 2010
  5. Junio C HamanoMay 29, 2010
  6. Junio C HamanoMay 29, 2010
  7. Bo YangMay 29, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.