Re: [PATCH] format-patch: teach --no-encode-headers

[Emma Brooks <me@pluvano.com>]

On 2020-04-05 20:29:57-0700, Junio C Hamano wrote:
> Is the RFC2047 the only thing we do to message headers?  What I am
> trying is to figure out if "encode-headers - yes/no?" would be a
> stable (iow, would we be gaining other kinds of encoding over time?)
> and well-defined (iow, is there a case where one kind of 'encoding'
> is still desirable while disabling other kinds of 'encoding' is
> wanted?) question.  If there is any doubt in your answers to the
> above question, we may have to make sure the name of the option
> makes it clear to users what kind of encoding we're talking about.

It's also too vague and it's not entirely clear from the option itself
what sort of encoding it refers to. I will change it to
--[no-]q-encode-headers and format.qEncodeHeaders in v2 unless there are
other suggestions.

> > +--[no-]encode-headers::
> 
> I think we'd want to standadize on writing these out, i.e.
> 
>         --encode-headers::
>         --no-encode-headers::
> 
> so let's follow that when adding a new option.

OK.

> > +	Encode email headers that have non-ASCII characters with
> > +	"Q-encoding", instead of outputting the headers verbatim. The
> 
> I wonder if calling RFC2047 out helps readers here, when they wonder
> what Q is and how they can decipher it.

I'll reference the RFC directly in v2.

> > +	default is `--encode-headers` unless the `format.encodeHeaders`
> > +	configuration variable is set.
> 
> I am wondering if we can go even shorter, e.g.
> 
> 	The default is set to the value of `format.encodeHeaders`
> 	configuration variable.

OK, I'll go with that.

> > -		if (needs_rfc2047_encoding(namebuf, namelen)) {
> > +		if (pp->encode_headers &&
> > +				needs_rfc2047_encoding(namebuf, namelen)) {
> 
> Don't overly indent the second line like this.  The same comment
> applies to the next hunk (not quoted).
> 
> Thanks.

OK.