git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Steffen Prohaska <prohaska@zib.de>
Cc: git@vger.kernel.org, "Jan H. Schoenherr" <schnhrr@cs.tu-berlin.de>
Subject: Re: [PATCH] shortlog: Fix wrapping lines of wraplen
Date: Sun, 09 Dec 2012 01:36:28 -0800	[thread overview]
Message-ID: <7v8v97efdv.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1354993767-7455-1-git-send-email-prohaska@zib.de> (Steffen Prohaska's message of "Sat, 8 Dec 2012 20:09:27 +0100")

Steffen Prohaska <prohaska@zib.de> writes:

> A recent commit [1] fixed a off-by-one wrapping error.  As
> a side-effect, add_wrapped_shortlog_msg() needs to be changed to always
> append a newline.

Could you clarify "As a side effect" a bit more?  Do you mean
something like this?

    Earlier strbuf_add_wrapped_text() ended its output with a
    newline only when the end of the text exactly fitted in wrap
    length, due to the off-by-one error fixed with 14e1a4e (utf8:
    fix off-by-one wrapping of text, 2012-10-18). There was a hack
    in add_wrapped_shortlog_msg() function to compensate for this
    bug.

    With the bug fixed, the function never ends its output with a
    newline, and the caller needs to unconditionally add one.


>
> [1] 14e1a4e1ff70aff36db3f5d2a8b806efd0134d50 utf8: fix off-by-one
>     wrapping of text
>
> Signed-off-by: Steffen Prohaska <prohaska@zib.de>
> ---
>  builtin/shortlog.c  |  3 +--
>  t/t4201-shortlog.sh | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/shortlog.c b/builtin/shortlog.c
> index b316cf3..db5b57d 100644
> --- a/builtin/shortlog.c
> +++ b/builtin/shortlog.c
> @@ -307,8 +307,7 @@ static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,
>  				     const struct shortlog *log)
>  {
>  	int col = strbuf_add_wrapped_text(sb, s, log->in1, log->in2, log->wrap);
> -	if (col != log->wrap)
> -		strbuf_addch(sb, '\n');
> +	strbuf_addch(sb, '\n');
>  }
>  
>  void shortlog_output(struct shortlog *log)
> diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
> index 6872ba1..02ac978 100755
> --- a/t/t4201-shortlog.sh
> +++ b/t/t4201-shortlog.sh
> @@ -120,6 +120,30 @@ test_expect_success 'shortlog from non-git directory' '
>  	test_cmp expect out
>  '
>  
> +test_expect_success 'shortlog should add newline when input line matches wraplen' '
> +	cat >expect <<\EOF &&
> +A U Thor (2):
> +      bbbbbbbbbbbbbbbbbb: bbbbbbbb bbb bbbb bbbbbbb bb bbbb bbb bbbbb bbbbbb
> +      aaaaaaaaaaaaaaaaaaaaaa: aaaaaa aaaaaaaaaa aaaa aaaaaaaa aa aaaa aa aaa
> +
> +EOF
> +	git shortlog -w >out <<\EOF &&
> +commit 0000000000000000000000000000000000000001
> +Author: A U Thor <author@example.com>
> +Date:   Thu Apr 7 15:14:13 2005 -0700
> +
> +    aaaaaaaaaaaaaaaaaaaaaa: aaaaaa aaaaaaaaaa aaaa aaaaaaaa aa aaaa aa aaa
> +    
> +commit 0000000000000000000000000000000000000002
> +Author: A U Thor <author@example.com>
> +Date:   Thu Apr 7 15:14:13 2005 -0700
> +
> +    bbbbbbbbbbbbbbbbbb: bbbbbbbb bbb bbbb bbbbbbb bb bbbb bbb bbbbb bbbbbb
> +    
> +EOF
> +	test_cmp expect out
> +'
> +
>  iconvfromutf8toiso88591() {
>  	printf "%s" "$*" | iconv -f UTF-8 -t ISO8859-1
>  }

  reply	other threads:[~2012-12-09  9:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-08 19:09 [PATCH] shortlog: Fix wrapping lines of wraplen (was broken since recent off-by-one fix) Steffen Prohaska
2012-12-09  9:36 ` Junio C Hamano [this message]
2012-12-11  5:59   ` [PATCH 0/2] Re: [PATCH] shortlog: Fix wrapping lines of wraplen Steffen Prohaska
2012-12-11  5:59     ` [PATCH 1/2] shortlog: Fix wrapping lines of wraplen (was broken since recent off-by-one fix) Steffen Prohaska
2012-12-11 10:24       ` "Jan H. Schönherr"
2012-12-11  5:59     ` [PATCH 2/2] strbuf_add_wrapped*(): Remove unused return value Steffen Prohaska

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7v8v97efdv.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=prohaska@zib.de \
    --cc=schnhrr@cs.tu-berlin.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).