Re: [PATCH v2] gpg-interface: strip CR chars for Windows gpg2

From: Torsten Bögershausen <tboegi@web.de>
Date: Nov 12, 2017, 20:25 UTC
Message-ID: <20171112202505.GB2677@tor.lan>
In-Reply-To: <20171112130710.16000-1-jerzy.kozera@gmail.com>

On Sun, Nov 12, 2017 at 01:07:10PM +0000, Jerzy Kozera wrote:

Show 26 quoted lines

> This fixes the issue with newlines being \r\n and not being displayed
> correctly when using gpg2 for Windows, as reported at
> https://github.com/git-for-windows/git/issues/1249
> 
> Issues with non-ASCII characters remain for further investigation.
> 
> Signed-off-by: Jerzy Kozera <jerzy.kozera@gmail.com>
> ---
> 
> Addressed comments by Junio C Hamano (check for following \n, and
> updated the commit description).
> 
>  gpg-interface.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 4feacf16e5..ab592af7f2 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -145,6 +145,20 @@ const char *get_signing_key(void)
>  	return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
>  }
>  
> +/* Strip CR from the CRLF line endings, in case we are on Windows. */
> +static void strip_cr(struct strbuf *buffer, size_t bottom) {
> +	size_t i, j;

It is not wrong to say "Strip CR from the CRLF". In Git we often talk about "convert CRLF into LF",

The comment somewhat different to the function name. The function namd and the name of the parameters can be more in in line with existing strbuf functions: (And the opening '{' should go into it's own line:

static void convert_crlf_to_lf(struct strbuf *sb, size_t len)
{
	size_t i, j;

An even more generic approach (could be done in a seperate commit) would be to move the whole function into strbuf.c/strbuf.h, and it may be called like this.

void strbuf_crlf_to_lf(struct strbuf *sb, size_t len)
{
  /* I would even avoid "i" and "j", and use src and dst or so) */
  size_t src_pos, dst_idx;
}

Thanks for working on this.

[]