On Mon, Oct 23, 2023 at 02:40:10PM -0400, Jeff King wrote:
On Fri, Oct 20, 2023 at 12:45:43PM +0200, Oswald Buddenhagen wrote:
that seems like a rather significant point, doesn't it?
Maybe. It depends on whether anybody is interested in adding
continuation support. Nobody has in the previous 18 years, and nobody
has asked for it.
dunno, it seems like a bug to me. so if i cared at all about this
functionality, i'd fix it just because. so at least it doesn't seem nice
to make it harder for a potential volunteer.
> So another option is to just fix the individual bugs separately.
>
... so that seems preferable to me, given that the necessary fixes
seem
rather trivial.
They're not too bad. Probably:
1. lc() the keys we put into the hash
2. match to/cc/bcc and dereference their arrays
3. maybe handle 'body' separately from headers to avoid confusion
with the header keys lowercased, one could simply use BODY as the key
and be done with it.
But there may be other similar bugs lurking.
One I didn't mention: the
hash-based version randomly reorders headers!
hmm, yeah, that would mean using Tie::IxHash if one wanted to do it
elegantly, at the cost of depending on another non-core module.
also, it means that another hash with non-lowercased versions of the
keys would have to be kept.
ok, that's stupid. it would be easier to just keep an additional array
of the original keys for iteration, and check the hash before emitting
them.
> I guess "readable" is up for debate here, but I find the inline handling
> a lot easier to follow
>
any particular reason for that?
For the reasons I gave in the commit message: namely that the matching
and logic is in one place and doesn't need to be duplicated (e.g., the
special handling of to/cc/bcc, which caused a bug here).
from what i can see, there isn't really anything to "match", apart from
agreeing on the data structure (which the code partially failed to do,
but that's trivial enough). and layering/abstracting things is usually
considered a good thing, unless the cost/benefit ratio is completely
backwards.
The "//" one would
work, but we support perl versions old enough that they don't have it.
according to my grepping, that ship has sailed.
also, why _would_ you support such ancient perl versions? that makes
even less sense to me than supporting ancient c compilers.
regards
