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

> "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".