unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/3] ruby: get rid of FileNames object
@ 2023-03-27 21:59 Felipe Contreras
  2023-03-27 21:59 ` [PATCH 1/3] ruby: add filenames helper Felipe Contreras
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Felipe Contreras @ 2023-03-27 21:59 UTC (permalink / raw)
  To: notmuch

We don't need a FileNames enumerable object only for a small number of strings,
we can just get them directly.

This iterator is meant to be transient and works only once, so we better just
iterate it once.

This is the same approach I took with the Tags object, I was waiting for
feedback on that approach but since there isn't any and there's no reason this
shouldn't work, here's the same for Filenames.

Felipe Contreras (3):
  ruby: add filenames helper
  ruby: filenames: return string array directly
  ruby: remove FileNames object

 bindings/ruby/defs.h      |  9 +------
 bindings/ruby/directory.c |  4 +--
 bindings/ruby/filenames.c | 51 +++------------------------------------
 bindings/ruby/init.c      | 14 -----------
 bindings/ruby/message.c   |  2 +-
 5 files changed, 8 insertions(+), 72 deletions(-)

-- 
2.39.2.13.g1fb56cf030

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] ruby: add filenames helper
  2023-03-27 21:59 [PATCH 0/3] ruby: get rid of FileNames object Felipe Contreras
@ 2023-03-27 21:59 ` Felipe Contreras
  2023-03-27 21:59 ` [PATCH 2/3] ruby: filenames: return string array directly Felipe Contreras
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Felipe Contreras @ 2023-03-27 21:59 UTC (permalink / raw)
  To: notmuch

Right now it doesn't do much, but it will help for further
reorganization.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 bindings/ruby/defs.h      | 3 +++
 bindings/ruby/directory.c | 4 ++--
 bindings/ruby/filenames.c | 6 ++++++
 bindings/ruby/message.c   | 2 +-
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index e2541e8f..216380d4 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -225,6 +225,9 @@ VALUE
 notmuch_rb_directory_get_child_directories (VALUE self);
 
 /* filenames.c */
+VALUE
+notmuch_rb_filenames_get (notmuch_filenames_t *fnames);
+
 VALUE
 notmuch_rb_filenames_destroy (VALUE self);
 
diff --git a/bindings/ruby/directory.c b/bindings/ruby/directory.c
index 910f0a99..f267d82f 100644
--- a/bindings/ruby/directory.c
+++ b/bindings/ruby/directory.c
@@ -87,7 +87,7 @@ notmuch_rb_directory_get_child_files (VALUE self)
 
     fnames = notmuch_directory_get_child_files (dir);
 
-    return Data_Wrap_Notmuch_Object (notmuch_rb_cFileNames, &notmuch_rb_filenames_type, fnames);
+    return notmuch_rb_filenames_get (fnames);
 }
 
 /*
@@ -106,5 +106,5 @@ notmuch_rb_directory_get_child_directories (VALUE self)
 
     fnames = notmuch_directory_get_child_directories (dir);
 
-    return Data_Wrap_Notmuch_Object (notmuch_rb_cFileNames, &notmuch_rb_filenames_type, fnames);
+    return notmuch_rb_filenames_get (fnames);
 }
diff --git a/bindings/ruby/filenames.c b/bindings/ruby/filenames.c
index 0dec1952..17873393 100644
--- a/bindings/ruby/filenames.c
+++ b/bindings/ruby/filenames.c
@@ -20,6 +20,12 @@
 
 #include "defs.h"
 
+VALUE
+notmuch_rb_filenames_get (notmuch_filenames_t *fnames)
+{
+    return Data_Wrap_Notmuch_Object (notmuch_rb_cFileNames, &notmuch_rb_filenames_type, fnames);
+}
+
 /*
  * call-seq: FILENAMES.destroy! => nil
  *
diff --git a/bindings/ruby/message.c b/bindings/ruby/message.c
index f45c95cc..0d83567c 100644
--- a/bindings/ruby/message.c
+++ b/bindings/ruby/message.c
@@ -120,7 +120,7 @@ notmuch_rb_message_get_filenames (VALUE self)
 
     fnames = notmuch_message_get_filenames (message);
 
-    return Data_Wrap_Notmuch_Object (notmuch_rb_cFileNames, &notmuch_rb_filenames_type, fnames);
+    return notmuch_rb_filenames_get (fnames);
 }
 
 /*
-- 
2.39.2.13.g1fb56cf030

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] ruby: filenames: return string array directly
  2023-03-27 21:59 [PATCH 0/3] ruby: get rid of FileNames object Felipe Contreras
  2023-03-27 21:59 ` [PATCH 1/3] ruby: add filenames helper Felipe Contreras
@ 2023-03-27 21:59 ` Felipe Contreras
  2023-03-27 21:59 ` [PATCH 3/3] ruby: remove FileNames object Felipe Contreras
  2023-03-28  0:13 ` [PATCH 0/3] ruby: get rid of " David Bremner
  3 siblings, 0 replies; 8+ messages in thread
From: Felipe Contreras @ 2023-03-27 21:59 UTC (permalink / raw)
  To: notmuch

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

diff --git a/bindings/ruby/filenames.c b/bindings/ruby/filenames.c
index 17873393..17ec4406 100644
--- a/bindings/ruby/filenames.c
+++ b/bindings/ruby/filenames.c
@@ -23,7 +23,10 @@
 VALUE
 notmuch_rb_filenames_get (notmuch_filenames_t *fnames)
 {
-    return Data_Wrap_Notmuch_Object (notmuch_rb_cFileNames, &notmuch_rb_filenames_type, fnames);
+    VALUE rb_array = rb_ary_new();
+    for (; notmuch_filenames_valid (fnames); notmuch_filenames_move_to_next (fnames))
+	rb_ary_push (rb_array, rb_str_new2 (notmuch_filenames_get (fnames)));
+    return rb_array;
 }
 
 /*
-- 
2.39.2.13.g1fb56cf030

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] ruby: remove FileNames object
  2023-03-27 21:59 [PATCH 0/3] ruby: get rid of FileNames object Felipe Contreras
  2023-03-27 21:59 ` [PATCH 1/3] ruby: add filenames helper Felipe Contreras
  2023-03-27 21:59 ` [PATCH 2/3] ruby: filenames: return string array directly Felipe Contreras
@ 2023-03-27 21:59 ` Felipe Contreras
  2023-04-12 10:35   ` David Bremner
  2023-03-28  0:13 ` [PATCH 0/3] ruby: get rid of " David Bremner
  3 siblings, 1 reply; 8+ messages in thread
From: Felipe Contreras @ 2023-03-27 21:59 UTC (permalink / raw)
  To: notmuch

Not used anymore now that we return an array of strings directly.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 bindings/ruby/defs.h      | 10 --------
 bindings/ruby/filenames.c | 52 ---------------------------------------
 bindings/ruby/init.c      | 14 -----------
 3 files changed, 76 deletions(-)

diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index 216380d4..2fde8652 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -59,7 +59,6 @@ extern ID ID_db_mode;
 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;
-extern const rb_data_type_t notmuch_rb_filenames_type;
 extern const rb_data_type_t notmuch_rb_query_type;
 extern const rb_data_type_t notmuch_rb_threads_type;
 extern const rb_data_type_t notmuch_rb_thread_type;
@@ -92,9 +91,6 @@ extern const rb_data_type_t notmuch_rb_tags_type;
 #define Data_Get_Notmuch_Directory(obj, ptr) \
     Data_Get_Notmuch_Object ((obj), &notmuch_rb_directory_type, (ptr))
 
-#define Data_Get_Notmuch_FileNames(obj, ptr) \
-    Data_Get_Notmuch_Object ((obj), &notmuch_rb_filenames_type, (ptr))
-
 #define Data_Get_Notmuch_Query(obj, ptr) \
     Data_Get_Notmuch_Object ((obj), &notmuch_rb_query_type, (ptr))
 
@@ -228,12 +224,6 @@ notmuch_rb_directory_get_child_directories (VALUE self);
 VALUE
 notmuch_rb_filenames_get (notmuch_filenames_t *fnames);
 
-VALUE
-notmuch_rb_filenames_destroy (VALUE self);
-
-VALUE
-notmuch_rb_filenames_each (VALUE self);
-
 /* query.c */
 VALUE
 notmuch_rb_query_destroy (VALUE self);
diff --git a/bindings/ruby/filenames.c b/bindings/ruby/filenames.c
index 17ec4406..7ff33dca 100644
--- a/bindings/ruby/filenames.c
+++ b/bindings/ruby/filenames.c
@@ -1,23 +1,3 @@
-/* The Ruby interface to the notmuch mail library
- *
- * Copyright © 2010 Ali Polatel
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see https://www.gnu.org/licenses/ .
- *
- * Author: Ali Polatel <alip@exherbo.org>
- */
-
 #include "defs.h"
 
 VALUE
@@ -28,35 +8,3 @@ notmuch_rb_filenames_get (notmuch_filenames_t *fnames)
 	rb_ary_push (rb_array, rb_str_new2 (notmuch_filenames_get (fnames)));
     return rb_array;
 }
-
-/*
- * call-seq: FILENAMES.destroy! => nil
- *
- * Destroys the filenames, freeing all resources allocated for it.
- */
-VALUE
-notmuch_rb_filenames_destroy (VALUE self)
-{
-    notmuch_rb_object_destroy (self, &notmuch_rb_filenames_type);
-
-    return Qnil;
-}
-
-/*
- * call-seq: FILENAMES.each {|item| block } => FILENAMES
- *
- * Calls +block+ once for each element in +self+, passing that element as a
- * parameter.
- */
-VALUE
-notmuch_rb_filenames_each (VALUE self)
-{
-    notmuch_filenames_t *fnames;
-
-    Data_Get_Notmuch_FileNames (self, fnames);
-
-    for (; notmuch_filenames_valid (fnames); notmuch_filenames_move_to_next (fnames))
-	rb_yield (rb_str_new2 (notmuch_filenames_get (fnames)));
-
-    return self;
-}
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index cd9f04cd..816b8cf5 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -22,7 +22,6 @@
 
 VALUE notmuch_rb_cDatabase;
 VALUE notmuch_rb_cDirectory;
-VALUE notmuch_rb_cFileNames;
 VALUE notmuch_rb_cQuery;
 VALUE notmuch_rb_cThreads;
 VALUE notmuch_rb_cThread;
@@ -65,7 +64,6 @@ const rb_data_type_t notmuch_rb_object_type = {
 
 define_type (database);
 define_type (directory);
-define_type (filenames);
 define_type (query);
 define_type (threads);
 define_type (thread);
@@ -86,7 +84,6 @@ define_type (tags);
  * the user:
  *
  * - Notmuch::Database
- * - Notmuch::FileNames
  * - Notmuch::Query
  * - Notmuch::Threads
  * - Notmuch::Messages
@@ -297,17 +294,6 @@ Init_notmuch (void)
     rb_define_method (notmuch_rb_cDirectory, "child_files", notmuch_rb_directory_get_child_files, 0); /* in directory.c */
     rb_define_method (notmuch_rb_cDirectory, "child_directories", notmuch_rb_directory_get_child_directories, 0); /* in directory.c */
 
-    /*
-     * Document-class: Notmuch::FileNames
-     *
-     * Notmuch file names
-     */
-    notmuch_rb_cFileNames = rb_define_class_under (mod, "FileNames", rb_cObject);
-    rb_undef_method (notmuch_rb_cFileNames, "initialize");
-    rb_define_method (notmuch_rb_cFileNames, "destroy!", notmuch_rb_filenames_destroy, 0); /* in filenames.c */
-    rb_define_method (notmuch_rb_cFileNames, "each", notmuch_rb_filenames_each, 0); /* in filenames.c */
-    rb_include_module (notmuch_rb_cFileNames, rb_mEnumerable);
-
     /*
      * Document-class: Notmuch::Query
      *
-- 
2.39.2.13.g1fb56cf030
\r

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] ruby: get rid of FileNames object
  2023-03-27 21:59 [PATCH 0/3] ruby: get rid of FileNames object Felipe Contreras
                   ` (2 preceding siblings ...)
  2023-03-27 21:59 ` [PATCH 3/3] ruby: remove FileNames object Felipe Contreras
@ 2023-03-28  0:13 ` David Bremner
  2023-03-28  6:47   ` Felipe Contreras
  3 siblings, 1 reply; 8+ messages in thread
From: David Bremner @ 2023-03-28  0:13 UTC (permalink / raw)
  To: Felipe Contreras, notmuch

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

> We don't need a FileNames enumerable object only for a small number of strings,
> we can just get them directly.
>
> This iterator is meant to be transient and works only once, so we better just
> iterate it once.
>
> This is the same approach I took with the Tags object, I was waiting for
> feedback on that approach but since there isn't any and there's no reason this
> shouldn't work, here's the same for Filenames.

Hi Felipe;

I still haven't had a chance to look at the proposed changes, but I did
wonder what your plan was as far as migration. Usually with the library
itself we try to provide fairly smooth upgrades.

d

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] ruby: get rid of FileNames object
  2023-03-28  0:13 ` [PATCH 0/3] ruby: get rid of " David Bremner
@ 2023-03-28  6:47   ` Felipe Contreras
  2023-03-28  6:51     ` Felipe Contreras
  0 siblings, 1 reply; 8+ messages in thread
From: Felipe Contreras @ 2023-03-28  6:47 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

On Mon, Mar 27, 2023 at 6:13 PM David Bremner <david@tethera.net> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > We don't need a FileNames enumerable object only for a small number of strings,
> > we can just get them directly.
> >
> > This iterator is meant to be transient and works only once, so we better just
> > iterate it once.
> >
> > This is the same approach I took with the Tags object, I was waiting for
> > feedback on that approach but since there isn't any and there's no reason this
> > shouldn't work, here's the same for Filenames.
>
> Hi Felipe;
>
> I still haven't had a chance to look at the proposed changes, but I did
> wonder what your plan was as far as migration. Usually with the library
> itself we try to provide fairly smooth upgrades.

There's no migration, there's no functional changes from clients' point of view.

Previously the code returned a Notmuch::Tags object, but this class
included the Enumerable [1] module, making it an Enumerable. So
everything you could do with an Enumerable you could do with
Notmuch::Tags, for example: tags.count().

But we didn't implement all these methods, we only had to implement
`each()`. If `each()` works, then everything else works.

For example this class implements an Enumerable:

  class Foo
    include Enumerable
    def each
      yield 'inbox'
      yield 'unread'
    end
  end

And then all these work:

  foo = Foo.new
  foo.each { |e| puts e }
  puts 'first: %s' % foo.first
  puts 'count: %s' % foo.count

We can replace the Foo object any other kind of Enumerable, and the
code works just the same:

  foo = %w[inbox unread]
  foo.each { |e| puts e }
  puts 'first: %s' % foo.first
  puts 'count: %s' % foo.count

So replacing Notmuch::Tags with an array of strings doesn't change
anything because both are Enumerable, and both yield Strings.

Cheers.

[1] https://rubydoc.info/stdlib/core/Enumerable

-- 
Felipe Contreras\r

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] ruby: get rid of FileNames object
  2023-03-28  6:47   ` Felipe Contreras
@ 2023-03-28  6:51     ` Felipe Contreras
  0 siblings, 0 replies; 8+ messages in thread
From: Felipe Contreras @ 2023-03-28  6:51 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

On Tue, Mar 28, 2023 at 12:47 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:

> We can replace the Foo object any other kind of Enumerable, and the
> code works just the same:
>
>   foo = %w[inbox unread]

I realized this might be too idiomatic of Ruby, it's the same thing as:

foo = [ 'inbox', 'unread' ]

>   foo.each { |e| puts e }
>   puts 'first: %s' % foo.first
>   puts 'count: %s' % foo.count

-- 
Felipe Contreras\r

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] ruby: remove FileNames object
  2023-03-27 21:59 ` [PATCH 3/3] ruby: remove FileNames object Felipe Contreras
@ 2023-04-12 10:35   ` David Bremner
  0 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2023-04-12 10:35 UTC (permalink / raw)
  To: Felipe Contreras, notmuch

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

> Not used anymore now that we return an array of strings directly.

series applied to master, with one whitespace tweak in patch 2/3

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-04-12 10:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-27 21:59 [PATCH 0/3] ruby: get rid of FileNames object Felipe Contreras
2023-03-27 21:59 ` [PATCH 1/3] ruby: add filenames helper Felipe Contreras
2023-03-27 21:59 ` [PATCH 2/3] ruby: filenames: return string array directly Felipe Contreras
2023-03-27 21:59 ` [PATCH 3/3] ruby: remove FileNames object Felipe Contreras
2023-04-12 10:35   ` David Bremner
2023-03-28  0:13 ` [PATCH 0/3] ruby: get rid of " David Bremner
2023-03-28  6:47   ` Felipe Contreras
2023-03-28  6:51     ` Felipe Contreras

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).