Re: [PATCH v2 1/2] ruby: add keyword arguments to db.query

2021-06-28 Thread Felipe Contreras
On Sun, Jun 27, 2021 at 12:18 PM David Bremner  wrote:
>
> Felipe Contreras  writes:
>
> > That way we don't need pass them to the query object ourselves.
>
> I have applied this change to master. As we discussed previously, I
> prefer to leave the sort order explicit in the tests.

OK. I still maintain that I see no point in doing this.

Just to find out what would happen if in the future the default
changed, I used NOTMUCH_SORT_UNSORTED instead of
NOTMUCH_SORT_NEWEST_FIRST in just two places and the result is 32
tests failing.

That does *not* include T395-ruby.sh.

In order for the ruby tests to fail the default in notmuch-search.c
has to be different from the default in lib/query.cc--I don't know why
notmuch-search.c doesn't use the true default.

But these are the tests that are *already* in the master branch. My
changes would not make the situation significantly worse.

I think if we really wanted to make the tests (and the binaries) truly
sort-agnostic, a lot more work is needed--and that's mostly orthogonal
to the changes I proposed to the ruby tests.

diff --git a/lib/query.cc b/lib/query.cc
index 792aba21..b618bf42 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -107,7 +107,7 @@ notmuch_query_create (notmuch_database_t *notmuch,

 query->query_string = talloc_strdup (query, query_string);

-query->sort = NOTMUCH_SORT_NEWEST_FIRST;
+query->sort = NOTMUCH_SORT_UNSORTED;

 query->exclude_terms = _notmuch_string_list_create (query);

diff --git a/notmuch-search.c b/notmuch-search.c
index 244817a9..83329a4d 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -770,7 +770,7 @@ _notmuch_search_cleanup (search_context_t *ctx)
 static search_context_t search_context = {
 .format_sel = NOTMUCH_FORMAT_TEXT,
 .exclude = NOTMUCH_EXCLUDE_TRUE,
-.sort = NOTMUCH_SORT_NEWEST_FIRST,
+.sort = NOTMUCH_SORT_UNSORTED,
 .output = 0,
 .offset = 0,
 .limit = -1, /* unlimited */


-- 
Felipe Contreras
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH v2 1/2] ruby: add keyword arguments to db.query

2021-06-27 Thread David Bremner
Felipe Contreras  writes:

> That way we don't need pass them to the query object ourselves.
>

I have applied this change to master. As we discussed previously, I
prefer to leave the sort order explicit in the tests.

d
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH v2 1/2] ruby: add keyword arguments to db.query

2021-05-23 Thread Felipe Contreras
That way we don't need pass them to the query object ourselves.

Signed-off-by: Felipe Contreras 
---
 bindings/ruby/database.c | 47 +---
 bindings/ruby/defs.h |  2 +-
 bindings/ruby/init.c |  2 +-
 test/T395-ruby.sh| 18 +++
 4 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index bb993d86..d6c804ac 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -395,17 +395,24 @@ notmuch_rb_database_get_all_tags (VALUE self)
 }
 
 /*
- * call-seq: DB.query(query) => QUERY
+ * call-seq:
+ *   DB.query(query) => QUERY
+ *   DB.query(query, sort:, excluded_tags:, omit_excluded:) => QUERY
  *
- * Retrieve a query object for the query string 'query'
+ * Retrieve a query object for the query string 'query'. When using keyword
+ * arguments they are passwed to the query object.
  */
 VALUE
-notmuch_rb_database_query_create (VALUE self, VALUE qstrv)
+notmuch_rb_database_query_create (int argc, VALUE *argv, VALUE self)
 {
+VALUE qstrv;
+VALUE opts;
 const char *qstr;
 notmuch_query_t *query;
 notmuch_database_t *db;
 
+rb_scan_args (argc, argv, "1:", , );
+
 Data_Get_Notmuch_Database (self, db);
 
 SafeStringValue (qstrv);
@@ -415,5 +422,39 @@ notmuch_rb_database_query_create (VALUE self, VALUE qstrv)
 if (!query)
 rb_raise (notmuch_rb_eMemoryError, "Out of memory");
 
+if (!NIL_P (opts)) {
+   VALUE sort, exclude_tags, omit_excluded;
+   VALUE kwargs[3];
+   static ID keyword_ids[3];
+
+   if (!keyword_ids[0]) {
+   keyword_ids[0] = rb_intern_const ("sort");
+   keyword_ids[1] = rb_intern_const ("exclude_tags");
+   keyword_ids[2] = rb_intern_const ("omit_excluded");
+   }
+
+   rb_get_kwargs (opts, keyword_ids, 0, 3, kwargs);
+
+   sort = kwargs[0];
+   exclude_tags = kwargs[1];
+   omit_excluded = kwargs[2];
+
+   if (sort != Qundef)
+   notmuch_query_set_sort (query, FIX2UINT (sort));
+
+   if (exclude_tags != Qundef) {
+   for (int i = 0; i < RARRAY_LEN (exclude_tags); i++) {
+   VALUE e = RARRAY_AREF (exclude_tags, i);
+   notmuch_query_add_tag_exclude (query, RSTRING_PTR (e));
+   }
+   }
+
+   if (omit_excluded != Qundef) {
+   notmuch_exclude_t omit;
+   omit = FIXNUM_P (omit_excluded) ? FIX2UINT (omit_excluded) : 
RTEST(omit_excluded);
+   notmuch_query_set_omit_excluded (query, omit);
+   }
+}
+
 return Data_Wrap_Notmuch_Object (notmuch_rb_cQuery, 
_rb_query_type, query);
 }
diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 9860ee17..995bcafd 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -174,7 +174,7 @@ VALUE
 notmuch_rb_database_get_all_tags (VALUE self);
 
 VALUE
-notmuch_rb_database_query_create (VALUE self, VALUE qstrv);
+notmuch_rb_database_query_create (int argc, VALUE *argv, VALUE self);
 
 /* directory.c */
 VALUE
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index bedfbf60..d421c601 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -275,7 +275,7 @@ Init_notmuch (void)
 rb_define_method (notmuch_rb_cDatabase, "find_message_by_filename",
  notmuch_rb_database_find_message_by_filename, 1); /* in 
database.c */
 rb_define_method (notmuch_rb_cDatabase, "all_tags", 
notmuch_rb_database_get_all_tags, 0); /* in database.c */
-rb_define_method (notmuch_rb_cDatabase, "query", 
notmuch_rb_database_query_create, 1); /* in database.c */
+rb_define_method (notmuch_rb_cDatabase, "query", 
notmuch_rb_database_query_create, -1); /* in database.c */
 
 /*
  * Document-class: Notmuch::Directory
diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index d36d4aff..e828efed 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -82,4 +82,22 @@ q.search_threads.each do |t|
 end
 EOF
 
+test_begin_subtest "check sort argument"
+notmuch search --sort=oldest-first --output=threads tag:inbox > EXPECTED
+test_ruby <<"EOF"
+q = db.query('tag:inbox', sort: Notmuch::SORT_OLDEST_FIRST)
+q.search_threads.each do |t|
+  puts 'thread:%s' % t.thread_id
+end
+EOF
+
+test_begin_subtest "check exclude_tags argument"
+notmuch search --output=threads --exclude=all tag:inbox > EXPECTED
+test_ruby <<"EOF"
+q = db.query('tag:inbox', exclude_tags: %w[deleted], omit_excluded: 
Notmuch::EXCLUDE_ALL)
+q.search_threads.each do |t|
+  puts 'thread:%s' % t.thread_id
+end
+EOF
+
 test_done
-- 
2.32.0.rc0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org