Open bshannon opened 6 years ago
bshannon
commented
6 years ago Sorry, it's been way too long since I made these changes and I don't remember all the details, but as I remember it the spamify changes were part of making it work for iso-2022-jp because spamify was making assumptions about how characters were encoded that just weren't valid for iso-2022-jp.
jkbzh
commented
6 years ago On Thu, Jun 07, 2018 at 01:58:32PM -0700, Bill Shannon wrote:
Sorry, it's been way too long since I made these changes and I don't remember all the details, but as I remember it the spamify changes were part of making it work for iso-2022-jp because spamify was making assumptions about how characters were encoded that just weren't valid for iso-2022-jp.
@bshannon,
OK, fair enough for my having taken so long time to take care of your of your code contribution. I'm grateful you still take time and in good mood for helping so late on.
It's going to take me more time to finish analyzing this change. Ideally, it's better to limit a MR to a specific contribution. C'est la vie.
I'll end splitting your changes in two so that I can test each part separately.
By the way, I've been adding all your changes to hypermail/Changelog. Please tell me if you'd prefer if you'd like me to add something more to identify you, like an email address or @github_alias.
--josé
bshannon
commented
6 years ago Attributing these changes to "Bill Shannon" is fine.
bshannon
commented
6 years ago Yes, you're probably right. (Too much time working in Java.) Do you need me to update the PR? Or will you just fix this when you split the changes into two?
jkbzh
commented
6 years ago @bshannon, no it's ok. I'm going thru them.
jkbzh
commented
5 years ago Hi @bshannon, It took some time but all the changes you have proposed for this PR were merged into the branch I am temporarily using for development (applemail_hack). I did some modifications: spamify_replacedomain was replacing everything following a "@" char by antispamdomain. Now it's only doing so for the domain part of valid email addresses. I didn't apply the optimization change your PR proposed for struct.c. Doing so restricted limited the use of antispamdomain (and maybe the other antispam functions) to the headers; the body was being ignored. I also added the needed free(input) where needed (and fixed a related sigsev).
Thank you for your contributions! I will close this PR when I merge that dev branch into master.
bshannon
commented
5 years ago I didn't apply the optimization change your PR proposed for struct.c. Doing so restricted limited the use of antispamdomain (and maybe the other antispam functions) to the headers; the body was being ignored.
Is the body not being handled in parseemail, like my comment says?
jkbzh
commented
5 years ago I didn't apply the optimization change your PR proposed for struct.c. Doing so restricted limited the use of antispamdomain (and maybe the other antispam functions) to the headers; the body was being ignored.
Is the body not being handled in parseemail, like my comment says?
I didn't look into detail for the reasons but spamify_replacedomain wasn't being applied to body when I enabled that change in struct.h. Disabling the changet made it work again.
@bshannon, do you think it could be possible to split this PR into two, one that deals with the changes you did for spamify (you can include the spamifiy_replacedomain changes in this eet) and another one for the rest of the iso-2022-jp changes? It will make it easier to test each set separately. It will also make it easier to review if I need to ask our i18n specialist colleagues something specific. Thanks!