Add additional multipart headers as an optional parameter #183
Conversation
lib/types/multipart.js
Outdated
| @@ -258,7 +265,9 @@ function Multipart(boy, cfg) { | |||
| curField = undefined; | |||
| if (buffer.length) | |||
| buffer = decodeText(buffer, 'binary', charset); | |||
| boy.emit('field', fieldname, buffer, false, truncated, encoding, contype); | |||
|
|
|||
| var hdrs = Object.keys(additionalHeaders).length ? additionalHeaders : null; | |||
There was a problem hiding this comment.
We can either use a simple boolean that is set in the for loop to avoid Object.keys() or we could just always pass additionalHeaders. I'm fine with either way.
There was a problem hiding this comment.
Or we could just pass header as-is. That would save us from having to do any additional work.
There was a problem hiding this comment.
Yeah, an empty object might be nice there, since it would avoid two null checks I suppose. I think I'll just pass in the object if that's cool with you.
lib/types/multipart.js
Outdated
| @@ -169,6 +170,10 @@ function Multipart(boy, cfg) { | |||
| else | |||
| encoding = '7bit'; | |||
|
|
|||
| for (var h in header) { | |||
There was a problem hiding this comment.
I think we should just iterate over Object.keys(header) to avoid traversing prototypes and the like.
lib/types/multipart.js
Outdated
| @@ -169,6 +170,10 @@ function Multipart(boy, cfg) { | |||
| else | |||
| encoding = '7bit'; | |||
|
|
|||
| for (var h in header) { | |||
| if (!isRequiredHeader(h)) additionalHeaders[h] = header[h][0]; | |||
There was a problem hiding this comment.
I think we should not be forcing the first value all the time, in case someone is interested in seeing all of the duplicate header values. Maybe at the very least we could just return the first value if header[h].length === 1 otherwise use the array value. Also just always assigning to header[h] so it's always an array for consistency is fine by me too.
|
Sounds good to me, added the changes in the above commit. |
|
Any idea if this will be merged any time in the future? |
|
@mscdex any update to add this additional header feature |
|
Hello guys, If not, when Cooper's changes will be merged with the master ? |
As we talked about previously, this commit adds any additional multipart headers as an optional record type parameter.