On Fri, 16 Apr 2010 13:51:40 -0700, Dirk Hohndel <hohndel@infradead.org> wrote: > The following two patches should address most of the concerns raised > to my previous series. Allow me to raise new concerns then. ;-) > The first patch simply adds an interface to obtain a concatenation of > all instances of a specific header from an email. I was hoping to see the "special-case value of NULL" go away with this change. And I like that there's a new function to get the concatenated header, (I would prefer an unabbreviated name of get_concatenated_header than get_header_concat), but I don't like seeing all the existing callers of get_header updated to pass an extra 0. Instead, I'd prefer to see those calls unchanged, and a tiny new get_header that passes the 0 and then make the actual implementing function be static and named something like notmuch_message_file_get_header_internal. Both patches have some trailing whitespace. I see these easily wince I have the following in my ~/.gitconfig: [core] whitespace = trailing-space,space-before-tab I'm sure there's a way to make git refuse to let you commit changes with trailing whitespace, but I don't know offhand what it is. Finally, I'd like to see some tests for this feature. (But we do have the feature already without tests, so I won't strictly block on that). If you can fix up any of the above before I make another pass through ym queue, that would be great. Thanks, -Carl