On Fri, Nov 10, 2023 at 10:36:22AM +0000, Simon Ser wrote:
> > I don't think that answering those questions needs to hold up your
> > patch. We can take it as a quick fix for a real bug, and then anybody
> > interested can dig further as a separate topic on top.
>
> These are good questions indeed. Unfortunately I don't hink I'll have time to
> work on this though.
That's OK. I think it's fine to stop here for now.
> > Some of these long lines (and the in-string newlines!) make this ugly
> > and hard to read. But it is also just copying the already-ugly style of
> > nearby tests. So I'm OK with that. But a prettier version might be:
> >
> > test_expect_success 'cover letter respects --encode-email-headers' '
> > test_config branch.rebuild-1.description "Café?" &&
> > git checkout rebuild-1 &&
> > git format-patch --stdout --encode-email-headers \
> > --cover-letter --cover-from-description=subject \
> > main >actual &&
> > ...
> > '
>
> Yeah, that sounds better indeed. Let me know if you want me to resend a cleaner
> version of the test.
I don't have a strong opinion, so I'd leave it up to you.
> > I also wondered if we could be just be testing this much more easily
> > with another header like "--to". But I guess that would be found in both
> > the cover letter and the actual patches (we also don't seem to encode
> > it even in the regular patches; is that a bug?).
>
> That sounds like another bug indeed… But maybe that'll be harder to fix. To
> Q-encode this field one needs to split off the full name and actual mail
> address ("André <andre@xxxxxxxxxxx>" would be split into "André" and
> "andre@xxxxxxxxxxx"), then Q-encode the full name, then join the strings
> together again. In particular, it's incorrect to Q-encode the full string.
Yeah, without even looking at the code, I had a suspicion that this
would be an issue. I doubt that format-patch is doing much parsing at
all of what we feed it via --to.
-Peff
