@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!
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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é
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Originally posted by: jkbzh
@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!
Originally posted by: bshannon
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.
Originally posted by: jkbzh
On Thu, Jun 07, 2018 at 01:58:32PM -0700, Bill Shannon wrote:
@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é
Originally posted by: bshannon
Attributing these changes to "Bill Shannon" is fine.
Originally posted by: bshannon
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?
Originally posted by: jkbzh
@bshannon, no it's ok. I'm going thru them.
Originally posted by: jkbzh
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.
Originally posted by: bshannon
Is the body not being handled in parseemail, like my comment says?
Originally posted by: jkbzh
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.