Hi Alexey, Thanks for working on this. I think the boolean prefix version makes more sense, and it seems to work OK. I have a few comments below
"Alexey I. Froloff" <raorn at raorn.name> writes: > + begin_list_id = strrchr (list_id_header, '<'); > + if (!begin_list_id) { > + fprintf (stderr, "Warning: Not indexing mailformed List-Id tag.\n"); > + return; > + } - I guess this should say "malformed". - I got about 1800 lines of such messages when indexing 280k messages. That might strike some people as excessive. On the otherhand I guess we need to re-think error reporting overall. What do you think about printing filename or message-id here its easier to double check that it is not a bug? > + end_list_id = strrchr(begin_list_id, '>'); > + if (!end_list_id || (end_list_id - begin_list_id < 2)) { > + fprintf (stderr, "Warning: Not indexing mailformed List-Id tag.\n"); > + return; > + } > + Same comments here. > + void *local = talloc_new (message); > + > + /* We extract the list id between the angle brackets */ > + const char *list_id = talloc_strndup (local, begin_list_id + 1, > + end_list_id - begin_list_id - 1); > + we should handle ENOMEM here, I think. > + /* _notmuch_message_add_term() may return > + * NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG here. We can't fix it, but > + * this is not a reason to exit with error... */ > + if (_notmuch_message_add_term (message, "list", list_id)) > + fprintf (stderr, "Warning: Not indexing List-Id: <%s>\n", list_id); This should say why the indexing failed. Other than that: - We need a couple tests for this code; tests/search should give some hints how to proceed. - We need a patch for NEWS, explaining what people need to do take advantage of the new functionality. I think that adding new prefixes to an existing database is OK, but I'd welcome confirmation. BTW, my not too scientific tests show no detectable bloat in the database, at least after running xapian-compact. I'd be curious what other people report.