[PATCH 0/3] ruby: add latest config API

[PATCH 0/3] ruby: add latest config API

[Felipe Contreras]

Currently the simplest way to open the notmuch database properly a
client must do:

  $config = IO.popen(%w[notmuch config list]) do |io|
    io.each(chomp: true).map { |e| e.split('=') }.to_h
  end
  $db_name = config['database.path']
  $db = Notmuch::Database.new($db_name)

While this works and it's not too overly complicated, the notmuch API
already has much better constucts.

This patch series allows the user to simply do:

  $db = Notmuch::Database.open_with_config
  $config = $db.config.to_h

And much more.

Felipe Contreras (3):
  ruby: add new Database.open_with_config
  ruby: add db.config
  ruby: make db.config return an enumerator

 bindings/ruby/database.c | 95 ++++++++++++++++++++++++++++++++++++++++
 bindings/ruby/defs.h     | 10 +++++
 bindings/ruby/init.c     |  2 +
 test/T395-ruby.sh        | 13 ++++++
 4 files changed, 120 insertions(+)

-- 
2.32.0.rc2

[PATCH 1/3] ruby: add new Database.open_with_config

[Felipe Contreras]

In order to make use of notmuch_database_open_with_config.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 bindings/ruby/database.c | 62 ++++++++++++++++++++++++++++++++++++++++
 bindings/ruby/defs.h     |  6 ++++
 bindings/ruby/init.c     |  1 +
 test/T395-ruby.sh        |  6 ++++
 4 files changed, 75 insertions(+)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index bb993d86..bc0c22cb 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -105,6 +105,68 @@ notmuch_rb_database_open (int argc, VALUE *argv, VALUE klass)
     return rb_ensure (rb_yield, obj, notmuch_rb_database_close, obj);
 }
 
+/*
+ * call-seq: Notmuch::Database.open_with_config([database_path:, mode:, config_path:, profile:]) [{|db| ... }]
+ *
+ * Opens a database with a configuration file.
+ *
+ */
+VALUE
+notmuch_rb_database_open_with_config (int argc, VALUE *argv, VALUE klass)
+{
+    VALUE obj;
+    notmuch_database_t *db;
+    notmuch_status_t ret;
+    VALUE opts;
+    const char *database_path = NULL;
+    notmuch_database_mode_t mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
+    const char *config_path = NULL;
+    const char *profile = NULL;
+
+    rb_scan_args (argc, argv, ":", &opts);
+
+    if (!NIL_P (opts)) {
+	VALUE rdatabase_path, rmode, rconfig_path, rprofile;
+	VALUE kwargs[4];
+	static ID keyword_ids[4];
+
+	if (!keyword_ids[0]) {
+	    keyword_ids[0] = rb_intern_const ("database_path");
+	    keyword_ids[1] = rb_intern_const ("mode");
+	    keyword_ids[2] = rb_intern_const ("config_path");
+	    keyword_ids[3] = rb_intern_const ("profile");
+	}
+
+	rb_get_kwargs (opts, keyword_ids, 0, 4, kwargs);
+
+	rdatabase_path = kwargs[0];
+	rmode = kwargs[1];
+	rconfig_path = kwargs[2];
+	rprofile = kwargs[3];
+
+	if (rdatabase_path != Qundef)
+	    database_path = nm_str (rdatabase_path);
+	if (rmode != Qundef)
+	    mode = FIX2INT (rmode);
+	if (rconfig_path != Qundef)
+	    config_path = nm_str (rconfig_path);
+	if (rprofile != Qundef)
+	    profile = nm_str (rprofile);
+    }
+
+    ret = notmuch_database_open_with_config (database_path, mode,
+					     config_path, profile, &db,
+					     NULL);
+    notmuch_rb_status_raise (ret);
+    obj = notmuch_rb_database_alloc (klass);
+    DATA_PTR (obj) = db;
+
+    if (!rb_block_given_p ())
+	return obj;
+
+    return rb_ensure (rb_yield, obj, notmuch_rb_database_close, obj);
+}
+
 /*
  * call-seq: DB.close => nil
  *
diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 9860ee17..78239229 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -55,6 +55,9 @@ extern ID ID_db_mode;
 # define RSTRING_PTR(v) (RSTRING((v))->ptr)
 #endif /* !defined (RSTRING_PTR) */
 
+/* Simple string helpers */
+#define nm_str(str) (str != Qnil ? RSTRING_PTR (str) : NULL)
+
 extern const rb_data_type_t notmuch_rb_object_type;
 extern const rb_data_type_t notmuch_rb_database_type;
 extern const rb_data_type_t notmuch_rb_directory_type;
@@ -134,6 +137,9 @@ notmuch_rb_database_initialize (int argc, VALUE *argv, VALUE klass);
 VALUE
 notmuch_rb_database_open (int argc, VALUE *argv, VALUE klass);
 
+VALUE
+notmuch_rb_database_open_with_config (int argc, VALUE *argv, VALUE klass);
+
 VALUE
 notmuch_rb_database_close (VALUE self);
 
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index bedfbf60..27c7eba3 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -259,6 +259,7 @@ Init_notmuch (void)
     notmuch_rb_cDatabase = rb_define_class_under (mod, "Database", rb_cObject);
     rb_define_alloc_func (notmuch_rb_cDatabase, notmuch_rb_database_alloc);
     rb_define_singleton_method (notmuch_rb_cDatabase, "open", notmuch_rb_database_open, -1); /* in database.c */
+    rb_define_singleton_method (notmuch_rb_cDatabase, "open_with_config", notmuch_rb_database_open_with_config, -1); /* in database.c */
     rb_define_method (notmuch_rb_cDatabase, "initialize", notmuch_rb_database_initialize, -1); /* in database.c */
     rb_define_method (notmuch_rb_cDatabase, "close", notmuch_rb_database_close, 0); /* in database.c */
     rb_define_method (notmuch_rb_cDatabase, "path", notmuch_rb_database_path, 0); /* in database.c */
diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index d36d4aff..27ac4cc8 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -82,4 +82,10 @@ q.search_threads.each do |t|
 end
 EOF
 
+test_begin_subtest "open with config"
+echo "$MAIL_DIR" > EXPECTED
+test_ruby <<EOF
+puts Notmuch::Database.open_with_config.path
+EOF
+
 test_done
-- 
2.32.0.rc2

Re: [PATCH 1/3] ruby: add new Database.open_with_config

[David Bremner]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> +    ret = notmuch_database_open_with_config (database_path, mode,
> +					     config_path, profile, &db,
> +					     NULL);

I'm curious why you ignore the error_message ouput parameter. Of course
it's valid and supported for library users to do this, but this way
there is no way for users of the ruby bindings to retrieve the
additional information about the error. Particularly in the case of
Xapian exceptions, this can be helpful for debugging.

Re: [PATCH 1/3] ruby: add new Database.open_with_config

[Felipe Contreras]

On Sun, Jun 27, 2021 at 1:02 PM David Bremner <david@tethera.net> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > +    ret = notmuch_database_open_with_config (database_path, mode,
> > +                                          config_path, profile, &db,
> > +                                          NULL);
>
> I'm curious why you ignore the error_message ouput parameter. Of course
> it's valid and supported for library users to do this, but this way
> there is no way for users of the ruby bindings to retrieve the
> additional information about the error. Particularly in the case of
> Xapian exceptions, this can be helpful for debugging.

Because the patch is complex enough as it is. It shouldn't be that
difficult to add a new notmuch_rb_status_raise function that takes an
error message and produces a proper Ruby exception with that message,
but that should be done in a separate patch.

-- 
Felipe Contreras

Re: [PATCH 1/3] ruby: add new Database.open_with_config

[David Bremner]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Sun, Jun 27, 2021 at 1:02 PM David Bremner <david@tethera.net> wrote:
>>
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>> > +    ret = notmuch_database_open_with_config (database_path, mode,
>> > +                                          config_path, profile, &db,
>> > +                                          NULL);
>>
>> I'm curious why you ignore the error_message ouput parameter. Of course
>> it's valid and supported for library users to do this, but this way
>> there is no way for users of the ruby bindings to retrieve the
>> additional information about the error. Particularly in the case of
>> Xapian exceptions, this can be helpful for debugging.
>
> Because the patch is complex enough as it is. It shouldn't be that
> difficult to add a new notmuch_rb_status_raise function that takes an
> error message and produces a proper Ruby exception with that message,
> but that should be done in a separate patch.
>

Agreed.

d

[PATCH 2/3] ruby: add db.config

[Felipe Contreras]

In order to use notmuch_config_get_pairs.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 bindings/ruby/database.c | 31 +++++++++++++++++++++++++++++++
 bindings/ruby/defs.h     |  4 ++++
 bindings/ruby/init.c     |  1 +
 test/T395-ruby.sh        |  7 +++++++
 4 files changed, 43 insertions(+)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index bc0c22cb..934cbfe8 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -479,3 +479,34 @@ notmuch_rb_database_query_create (VALUE self, VALUE qstrv)
 
     return Data_Wrap_Notmuch_Object (notmuch_rb_cQuery, &notmuch_rb_query_type, query);
 }
+
+/*
+ * call-seq: DB.config(prefix) {|key, value| block} => nil
+ *
+ * Calls +block+ once for each key/value pair.
+ *
+ */
+VALUE
+notmuch_rb_database_config (int argc, VALUE *argv, VALUE self)
+{
+    VALUE prefix;
+    notmuch_database_t *db;
+    notmuch_config_pairs_t *list;
+    const char *cprefix;
+
+    Data_Get_Notmuch_Database (self, db);
+
+    rb_scan_args (argc, argv, "01", &prefix);
+
+    cprefix = prefix != Qnil ? RSTRING_PTR (prefix) : "";
+
+    list = notmuch_config_get_pairs (db, cprefix);
+    for (; notmuch_config_pairs_valid (list); notmuch_config_pairs_move_to_next (list)) {
+	const char *key = notmuch_config_pairs_key (list);
+	const char *value = notmuch_config_pairs_value (list);
+	rb_yield (rb_ary_new3(2, nm_str_new (key), nm_str_new (value)));
+    }
+    notmuch_config_pairs_destroy (list);
+
+    return Qnil;
+}
diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 78239229..f31e563b 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -57,6 +57,7 @@ extern ID ID_db_mode;
 
 /* Simple string helpers */
 #define nm_str(str) (str != Qnil ? RSTRING_PTR (str) : NULL)
+#define nm_str_new(str) (str ? rb_str_new_cstr (str) : Qnil)
 
 extern const rb_data_type_t notmuch_rb_object_type;
 extern const rb_data_type_t notmuch_rb_database_type;
@@ -182,6 +183,9 @@ notmuch_rb_database_get_all_tags (VALUE self);
 VALUE
 notmuch_rb_database_query_create (VALUE self, VALUE qstrv);
 
+VALUE
+notmuch_rb_database_config (int argc, VALUE *argv, VALUE self);
+
 /* directory.c */
 VALUE
 notmuch_rb_directory_destroy (VALUE self);
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index 27c7eba3..4b2f8655 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -277,6 +277,7 @@ Init_notmuch (void)
 		      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, "config", notmuch_rb_database_config, -1); /* in database.c */
 
     /*
      * Document-class: Notmuch::Directory
diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index 27ac4cc8..a7b09589 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -88,4 +88,11 @@ test_ruby <<EOF
 puts Notmuch::Database.open_with_config.path
 EOF
 
+test_begin_subtest "config"
+notmuch config list | grep -v '^built_with\.' > EXPECTED
+test_ruby <<"EOF"
+config_db = Notmuch::Database.open_with_config
+config_db.config { |e| puts '%s=%s' % e }
+EOF
+
 test_done
-- 
2.32.0.rc2

Re: [PATCH 2/3] ruby: add db.config

[Felipe Contreras]

On Thu, Jun 3, 2021 at 10:29 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:

> --- a/test/T395-ruby.sh
> +++ b/test/T395-ruby.sh
> @@ -88,4 +88,11 @@ test_ruby <<EOF
>  puts Notmuch::Database.open_with_config.path
>  EOF
>
> +test_begin_subtest "config"
> +notmuch config list | grep -v '^built_with\.' > EXPECTED
> +test_ruby <<"EOF"
> +config_db = Notmuch::Database.open_with_config
> +config_db.config { |e| puts '%s=%s' % e }
> +EOF
> +

I just noticed this might a little more idiomatic:

  Notmuch::Database.open_with_config do |db|
    db.config { |e| puts '%s=%s' % e }
  end

-- 
Felipe Contreras

[PATCH 3/3] ruby: make db.config return an enumerator

[Felipe Contreras]

Currently db.config requires a block to work:

  db.config { |k, v| puts '%s=%s' % [k, v] }

If you try to use it without a block you, get an error like:

  in `config': no block given (LocalJumpError)

In Ruby most methods should return an Enumerator if no block is given,
like:

  (1..10).each
  => #<Enumerator: ...>

This allows us to do:

  db.config.to_a
  db.config.to_h
  db.config.each { |k, v| ... }

And of course what is already possible:

  db.config { |k, v| ... }

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 bindings/ruby/database.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 934cbfe8..9316b32d 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -494,6 +494,8 @@ notmuch_rb_database_config (int argc, VALUE *argv, VALUE self)
     notmuch_config_pairs_t *list;
     const char *cprefix;
 
+    RETURN_ENUMERATOR(self, argc, argv);
+
     Data_Get_Notmuch_Database (self, db);
 
     rb_scan_args (argc, argv, "01", &prefix);
-- 
2.32.0.rc2