Re: [PATCHv5 4/8] pretty.c: teach format_commit_message() to reencode the output

[Junio C Hamano <gitster@pobox.com>]

"Pat Notz" <patnotz@gmail.com> writes:

> diff --git a/pretty.c b/pretty.c
> index a607fd6..e5ce7fb 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1009,16 +1009,47 @@ void userformat_find_requirements(const char *fmt, struct userformat_want *w)
>  
>  void format_commit_message(const struct commit *commit,
>  			   const char *format, struct strbuf *sb,
> -			   const struct pretty_print_context *pretty_ctx)
> +			   const struct pretty_print_context *pretty_ctx,
> +			   const char *output_encoding)
>  {
>  	struct format_commit_context context;
> +	static char utf8[] = "UTF-8";
> +	char *enc;
> +	char *buffer;
> +	char *enc_buffer;
> +	struct strbuf scratch_sb = STRBUF_INIT;
> +	struct strbuf *sb_ptr;
> +
> +	enc = get_header(commit, "encoding");
> +	enc = enc ? enc : utf8;
> +	if(output_encoding && strcmp(enc,output_encoding)) {
> +		sb_ptr = &scratch_sb;
> +	} else {
> +		sb_ptr = sb;
> +	}

Style...

>  	memset(&context, 0, sizeof(context));
>  	context.commit = commit;
>  	context.pretty_ctx = pretty_ctx;
>  	context.wrap_start = sb->len;
> -	strbuf_expand(sb, format, format_commit_item, &context);
> -	rewrap_message_tail(sb, &context, 0, 0, 0);
> +	strbuf_expand(sb_ptr, format, format_commit_item, &context);
> +	rewrap_message_tail(sb_ptr, &context, 0, 0, 0);
> +
> +	if(sb_ptr != sb) {
> +		/* if re-encoding fails, take the content byte-for-byte */
> +		buffer = strbuf_detach(sb_ptr, 0);
> +		enc_buffer = reencode_string(buffer, output_encoding, enc);
> +		enc_buffer = enc_buffer ? enc_buffer : buffer;
> +
> +		strbuf_addstr(sb,enc_buffer);
> +
> +		if(enc_buffer != buffer)
> +			free(enc_buffer);
> +		free(buffer);
> +	}
> +
> +	if(enc != utf8)
> +		free(enc);
>  }

You are expanding and wrapping commit->buf before re-encoding, but I am
not sure it is the right thing to do.  Wouldn't it become much simpler and
more consistent if you re-encode first and then give the result to later
expansion and wrapping process?  format_commit_one() would need to take
"msg" not from c->commit->buffer but from a new field to hold reencoded
result you will add in your patch to the structure, if you do so, of
course.

Besides, I am a bit lost as to what this patch has to do with the stated
goal of the series, "Add commit message options for rebase --autosquash".