Re: [PATCH] WIP: add all subjects to value.

[Daniel Kahn Gillmor <dkg@fifthhorseman.net>]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

I like the ideas behind this patch.  it's labeled WIP, presumably
because it doesn't handle ordering the subject lines, right?

further comments below inline:

On Tue 2017-12-19 09:15:40 -0400, David Bremner wrote:
> +/* Remove all values from a document; currently these are
> +   all regenerated during indexing */
> +
> +notmuch_private_status_t
> +_notmuch_message_remove_values (notmuch_message_t *message)
> +{
> +    try {
> +	message->doc.clear_values ();
> +	message->modified = TRUE;
> +    } catch (const Xapian::Error &error) {
> +	notmuch_database_t *notmuch = message->notmuch;
> +
> +	if (!notmuch->exception_reported) {
> +	    _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred creating message: %s\n",
> +				      error.get_msg().c_str());

is this the right exception message?  seems like it should talk about
removing values rather than "creating message"

> @@ -1114,7 +1144,13 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
>      message->doc.add_value (NOTMUCH_VALUE_TIMESTAMP,
>  			    Xapian::sortable_serialise (time_value));
>      message->doc.add_value (NOTMUCH_VALUE_FROM, from);
> -    message->doc.add_value (NOTMUCH_VALUE_SUBJECT, subject);
> +
> +    old_subject = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
> +    if (old_subject.empty())
> +	message->doc.add_value (NOTMUCH_VALUE_SUBJECT, subject);
> +    else
> +	message->doc.add_value (NOTMUCH_VALUE_SUBJECT, old_subject + "\n" + subject);

here we're appending the subject to the tail -- so the first injected
subject line stays at the top.

it looks like it will re-add the same subject line multiple times,
even if already present.  so if i get 3 copies of a message with subject
"foo" then the value slot will be "foo\nfoo\nfoo".  is that desirable?

i think either we care about careful ordering (which strikes me as
delicate and potentially difficult to handle, esp. when you get into the
headers changing depending on whether you index the cleartext or not),
or we should avoid injecting duplicates.

wdyt?

        --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]