From: Jonathan Tan [mailto:jonathantanmy@xxxxxxxxxx] > On 01/12/2017 01:20 AM, Matthew Wilcox wrote: > A test exercising the new functionality would be nice. Roger. > Also, maybe a more descriptive title like "mailinfo: also respect > keep_cr after base64 decode" (50 characters) is better. No problem. > > @@ -143,6 +144,7 @@ static void am_state_init(struct am_state *state, > const char *dir) > > > > memset(state, 0, sizeof(*state)); > > > > + state->keep_cr = -1; > > Maybe query the git config here (instead of later) so that we never have > to worry about state->keep_cr being neither 0 nor 1? This function > already queries the git config anyway. I wondered why the existing code didn't do that, but I wanted to make a minimal change rather than clean up an older mistake. I'm happy to do it that way. > > diff --git a/mailinfo.h b/mailinfo.h > > index 04a25351d..9fddcf684 100644 > > --- a/mailinfo.h > > +++ b/mailinfo.h > > @@ -12,6 +12,7 @@ struct mailinfo { > > struct strbuf email; > > int keep_subject; > > int keep_non_patch_brackets_in_subject; > > + int keep_cr; > > int add_message_id; > > int use_scissors; > > int use_inbody_headers; > > I personally would write "unsigned keep_cr : 1" to further emphasize > that this can only be 0 or 1, but I don't know if it's better to keep > with the style existing in the file (that is, using int). Probably best to stick to the existing file ... someone can always do a cleanup patch later, and having this match the others will make that easier, not harder. Thanks for the review.