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

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

Date: Fri, 04 May 2018 09:48:52 -0400

To: David Bremner, David Bremner, notmuch@notmuchmail.org

Cc:

From: Daniel Kahn Gillmor


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
signature.asc (application/pgp-signature)
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: