[PATCH v2 2/3] lib: Add support for automatically excluding tags from queries
Quoth Jameson Graef Rollins on Jan 14 at 3:38 pm:
> It looks like something in this patch is causing the following build
> warning:
>
> CXX -O2 lib/query.o
> lib/query.cc:26:8: warning: ?_notmuch_query? declared with greater visibility
> than the type of its field
> ?_notmuch_query::exclude_terms? [-Wattributes]
>
> However, I can't quite figure out what's causing it.
The problem is that notmuch_query_t is a "visible" symbol because the
type is declared (though not defined) in lib/notmuch.h. The actual
definition is tucked away in lib/query.cc, but GCC doesn't seem to
care. I added a field of type notmuch_string_list_t to the struct's
definition, but notmuch_string_list_t is declared between the "hidden"
pragmas in lib/notmuch-private.h because the type is private to the
library. This field with a hidden type in a visible type is what GCC
is complaining about.
I'm rather confused by the whole type visibility thing since type
symbols aren't linkable anyway. However, while puzzling over how you
could possibly use hidden types if they can only be used in other
hidden types, I discovered that Carl had solved this exact problem
last May in d5523ead by adding a visibility("default") attribute to
the offending hidden type.
> > +void
> > +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
> > +{
> > +char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"),
> > tag);
> > +_notmuch_string_list_append (query->exclude_terms, term);
> > +}
>
> This is really not an issue with this patch at all, and it should NOT
> prevent it from being applied, but this came up briefly on IRC and I'm
> curious, so I'll ask about it here.
>
> Are terms ALWAYS lower cased? If not, it seems to me it's possible to
> have an indexed term 'Kspam' that would get confused with the term
> 'spam' prefixed with the keyword prefix 'K' (which we use for tags).
> Maybe this degeneracy is broken by the query parser somehow (or maybe by
> the fact that tags are boolean terms?), but I wonder if it's not safer
> to use the built-in xapian prefix separator ':', ie:
>
> ... talloc_asprintf (query, "%s:%s", _find_prefix ("tag"), tag);
>
> I guess fixing that globally would require a database rebuild...
We discussed this on IRC, but to summarize for the list, the tag
prefix is a single character, so Xapian's ':' rule doesn't apply.
There are several places where we *do* get this wrong and use a
multi-character term prefix with a term that may start with a capital
letter but they're all terms you can't search anyway and, unless I'm
mistaken, we're completely consistent about where we violate or do not
violate the ':' rule.
> Ok, that's totally just an aside, and should not be a blocker for this
> patch.
>
> jamie.
[PATCH v2 2/3] lib: Add support for automatically excluding tags from queries
It looks like something in this patch is causing the following build
warning:
CXX -O2 lib/query.o
lib/query.cc:26:8: warning: ?_notmuch_query? declared with greater visibility
than the type of its field
?_notmuch_query::exclude_terms? [-Wattributes]
However, I can't quite figure out what's causing it.
> +void
> +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
> +{
> +char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
> +_notmuch_string_list_append (query->exclude_terms, term);
> +}
This is really not an issue with this patch at all, and it should NOT
prevent it from being applied, but this came up briefly on IRC and I'm
curious, so I'll ask about it here.
Are terms ALWAYS lower cased? If not, it seems to me it's possible to
have an indexed term 'Kspam' that would get confused with the term
'spam' prefixed with the keyword prefix 'K' (which we use for tags).
Maybe this degeneracy is broken by the query parser somehow (or maybe by
the fact that tags are boolean terms?), but I wonder if it's not safer
to use the built-in xapian prefix separator ':', ie:
... talloc_asprintf (query, "%s:%s", _find_prefix ("tag"), tag);
I guess fixing that globally would require a database rebuild...
Ok, that's totally just an aside, and should not be a blocker for this
patch.
jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL:
Re: [PATCH v2 2/3] lib: Add support for automatically excluding tags from queries
It looks like something in this patch is causing the following build
warning:
CXX -O2 lib/query.o
lib/query.cc:26:8: warning: ‘_notmuch_query’ declared with greater visibility
than the type of its field
‘_notmuch_query::exclude_terms’ [-Wattributes]
However, I can't quite figure out what's causing it.
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
+{
+char *term = talloc_asprintf (query, %s%s, _find_prefix (tag), tag);
+_notmuch_string_list_append (query-exclude_terms, term);
+}
This is really not an issue with this patch at all, and it should NOT
prevent it from being applied, but this came up briefly on IRC and I'm
curious, so I'll ask about it here.
Are terms ALWAYS lower cased? If not, it seems to me it's possible to
have an indexed term 'Kspam' that would get confused with the term
'spam' prefixed with the keyword prefix 'K' (which we use for tags).
Maybe this degeneracy is broken by the query parser somehow (or maybe by
the fact that tags are boolean terms?), but I wonder if it's not safer
to use the built-in xapian prefix separator ':', ie:
... talloc_asprintf (query, %s:%s, _find_prefix (tag), tag);
I guess fixing that globally would require a database rebuild...
Ok, that's totally just an aside, and should not be a blocker for this
patch.
jamie.
pgpAlplL5N69N.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 2/3] lib: Add support for automatically excluding tags from queries
Quoth Jameson Graef Rollins on Jan 14 at 3:38 pm:
It looks like something in this patch is causing the following build
warning:
CXX -O2 lib/query.o
lib/query.cc:26:8: warning: ‘_notmuch_query’ declared with greater visibility
than the type of its field
‘_notmuch_query::exclude_terms’ [-Wattributes]
However, I can't quite figure out what's causing it.
The problem is that notmuch_query_t is a visible symbol because the
type is declared (though not defined) in lib/notmuch.h. The actual
definition is tucked away in lib/query.cc, but GCC doesn't seem to
care. I added a field of type notmuch_string_list_t to the struct's
definition, but notmuch_string_list_t is declared between the hidden
pragmas in lib/notmuch-private.h because the type is private to the
library. This field with a hidden type in a visible type is what GCC
is complaining about.
I'm rather confused by the whole type visibility thing since type
symbols aren't linkable anyway. However, while puzzling over how you
could possibly use hidden types if they can only be used in other
hidden types, I discovered that Carl had solved this exact problem
last May in d5523ead by adding a visibility(default) attribute to
the offending hidden type.
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
+{
+char *term = talloc_asprintf (query, %s%s, _find_prefix (tag),
tag);
+_notmuch_string_list_append (query-exclude_terms, term);
+}
This is really not an issue with this patch at all, and it should NOT
prevent it from being applied, but this came up briefly on IRC and I'm
curious, so I'll ask about it here.
Are terms ALWAYS lower cased? If not, it seems to me it's possible to
have an indexed term 'Kspam' that would get confused with the term
'spam' prefixed with the keyword prefix 'K' (which we use for tags).
Maybe this degeneracy is broken by the query parser somehow (or maybe by
the fact that tags are boolean terms?), but I wonder if it's not safer
to use the built-in xapian prefix separator ':', ie:
... talloc_asprintf (query, %s:%s, _find_prefix (tag), tag);
I guess fixing that globally would require a database rebuild...
We discussed this on IRC, but to summarize for the list, the tag
prefix is a single character, so Xapian's ':' rule doesn't apply.
There are several places where we *do* get this wrong and use a
multi-character term prefix with a term that may start with a capital
letter but they're all terms you can't search anyway and, unless I'm
mistaken, we're completely consistent about where we violate or do not
violate the ':' rule.
Ok, that's totally just an aside, and should not be a blocker for this
patch.
jamie.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 2/3] lib: Add support for automatically excluding tags from queries
This is useful for tags like "deleted" and "spam" that people
generally want to exclude from query results. These exclusions will
be overridden if a tag is explicitly mentioned in a query.
---
lib/notmuch.h |6 ++
lib/query.cc | 35 +++
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9f23a10..7929fe7 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -457,6 +457,12 @@ notmuch_query_set_sort (notmuch_query_t *query,
notmuch_sort_t sort);
notmuch_sort_t
notmuch_query_get_sort (notmuch_query_t *query);
+/* Add a tag that will be excluded from the query results by default.
+ * This exclusion will be overridden if this tag appears explicitly in
+ * the query. */
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag);
+
/* Execute a query for threads, returning a notmuch_threads_t object
* which can be used to iterate over the results. The returned threads
* object is owned by the query and as such, will only be valid until
diff --git a/lib/query.cc b/lib/query.cc
index b6c0f12..0b36602 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -27,6 +27,7 @@ struct _notmuch_query {
notmuch_database_t *notmuch;
const char *query_string;
notmuch_sort_t sort;
+notmuch_string_list_t *exclude_terms;
};
typedef struct _notmuch_mset_messages {
@@ -76,6 +77,8 @@ notmuch_query_create (notmuch_database_t *notmuch,
query->sort = NOTMUCH_SORT_NEWEST_FIRST;
+query->exclude_terms = _notmuch_string_list_create (query);
+
return query;
}
@@ -97,6 +100,13 @@ notmuch_query_get_sort (notmuch_query_t *query)
return query->sort;
}
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
+{
+char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
+_notmuch_string_list_append (query->exclude_terms, term);
+}
+
/* We end up having to call the destructors explicitly because we had
* to use "placement new" in order to initialize C++ objects within a
* block that we allocated with talloc. So C++ is making talloc
@@ -112,6 +122,27 @@ _notmuch_messages_destructor (notmuch_mset_messages_t
*messages)
return 0;
}
+/* Return a query that does not match messages with the excluded tags
+ * registered with the query. Any tags that explicitly appear in
+ * xquery will not be excluded. */
+static Xapian::Query
+_notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
+{
+for (notmuch_string_node_t *term = query->exclude_terms->head; term;
+term = term->next) {
+ Xapian::TermIterator it = xquery.get_terms_begin ();
+ Xapian::TermIterator end = xquery.get_terms_end ();
+ for (; it != end; it++) {
+ if ((*it).compare (term->string) == 0)
+ break;
+ }
+ if (it == end)
+ xquery = Xapian::Query (Xapian::Query::OP_AND_NOT,
+ xquery, Xapian::Query (term->string));
+}
+return xquery;
+}
+
notmuch_messages_t *
notmuch_query_search_messages (notmuch_query_t *query)
{
@@ -157,6 +188,8 @@ notmuch_query_search_messages (notmuch_query_t *query)
mail_query, string_query);
}
+ final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme (Xapian::BoolWeight());
switch (query->sort) {
@@ -436,6 +469,8 @@ notmuch_query_count_messages (notmuch_query_t *query)
mail_query, string_query);
}
+ final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme(Xapian::BoolWeight());
enquire.set_docid_order(Xapian::Enquire::ASCENDING);
--
1.7.7.3
[PATCH v2 2/3] lib: Add support for automatically excluding tags from queries
This is useful for tags like deleted and spam that people
generally want to exclude from query results. These exclusions will
be overridden if a tag is explicitly mentioned in a query.
---
lib/notmuch.h |6 ++
lib/query.cc | 35 +++
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9f23a10..7929fe7 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -457,6 +457,12 @@ notmuch_query_set_sort (notmuch_query_t *query,
notmuch_sort_t sort);
notmuch_sort_t
notmuch_query_get_sort (notmuch_query_t *query);
+/* Add a tag that will be excluded from the query results by default.
+ * This exclusion will be overridden if this tag appears explicitly in
+ * the query. */
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag);
+
/* Execute a query for threads, returning a notmuch_threads_t object
* which can be used to iterate over the results. The returned threads
* object is owned by the query and as such, will only be valid until
diff --git a/lib/query.cc b/lib/query.cc
index b6c0f12..0b36602 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -27,6 +27,7 @@ struct _notmuch_query {
notmuch_database_t *notmuch;
const char *query_string;
notmuch_sort_t sort;
+notmuch_string_list_t *exclude_terms;
};
typedef struct _notmuch_mset_messages {
@@ -76,6 +77,8 @@ notmuch_query_create (notmuch_database_t *notmuch,
query-sort = NOTMUCH_SORT_NEWEST_FIRST;
+query-exclude_terms = _notmuch_string_list_create (query);
+
return query;
}
@@ -97,6 +100,13 @@ notmuch_query_get_sort (notmuch_query_t *query)
return query-sort;
}
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
+{
+char *term = talloc_asprintf (query, %s%s, _find_prefix (tag), tag);
+_notmuch_string_list_append (query-exclude_terms, term);
+}
+
/* We end up having to call the destructors explicitly because we had
* to use placement new in order to initialize C++ objects within a
* block that we allocated with talloc. So C++ is making talloc
@@ -112,6 +122,27 @@ _notmuch_messages_destructor (notmuch_mset_messages_t
*messages)
return 0;
}
+/* Return a query that does not match messages with the excluded tags
+ * registered with the query. Any tags that explicitly appear in
+ * xquery will not be excluded. */
+static Xapian::Query
+_notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
+{
+for (notmuch_string_node_t *term = query-exclude_terms-head; term;
+term = term-next) {
+ Xapian::TermIterator it = xquery.get_terms_begin ();
+ Xapian::TermIterator end = xquery.get_terms_end ();
+ for (; it != end; it++) {
+ if ((*it).compare (term-string) == 0)
+ break;
+ }
+ if (it == end)
+ xquery = Xapian::Query (Xapian::Query::OP_AND_NOT,
+ xquery, Xapian::Query (term-string));
+}
+return xquery;
+}
+
notmuch_messages_t *
notmuch_query_search_messages (notmuch_query_t *query)
{
@@ -157,6 +188,8 @@ notmuch_query_search_messages (notmuch_query_t *query)
mail_query, string_query);
}
+ final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme (Xapian::BoolWeight());
switch (query-sort) {
@@ -436,6 +469,8 @@ notmuch_query_count_messages (notmuch_query_t *query)
mail_query, string_query);
}
+ final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme(Xapian::BoolWeight());
enquire.set_docid_order(Xapian::Enquire::ASCENDING);
--
1.7.7.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
