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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Bo Yang <struggleyb.nku@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  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> @@",
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]