notmuch_threads_back and notmuch_messages_back

notmuch_threads_back and notmuch_messages_back

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

Is it possible to implement notmuch_threads_back and notmuch_messages_back?
And I guess will make sense to have also notmuch_tags_back.

This functions will do the oposite than notmuch_threads_advance and
notmuch_messages_advance. So I can use them as iterators going back and forward.

I didn't check the implementation of notmuch. I don't know if will be easy or
will need to redesign a lot of code.

-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
   La felicidad no es hacer lo que deseas
         es desear lo que haces.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: notmuch_threads_back and notmuch_messages_back

[Carl Worth]

On Thu, 26 Nov 2009 21:23:47 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> Is it possible to implement notmuch_threads_back and notmuch_messages_back?
> And I guess will make sense to have also notmuch_tags_back.
> 
> This functions will do the oposite than notmuch_threads_advance and
> notmuch_messages_advance. So I can use them as iterators going back and forward.
> 
> I didn't check the implementation of notmuch. I don't know if will be easy or
> will need to redesign a lot of code.

Most cases of these iterators are built on Xapian iterators, (C++ objects with
overloaded operators). So those are just using ++ for advance() and can
easily use -- for the new function, (I'd like a verb that pairs better
with "advance" than the non-verb "back"---any suggestions)?

So those won't need any new code. The one case that will need new code
is that for notmuch_message_get_replies and
notmuch_message_get_toplevel_messages the messages iterator is currently
built on a singly-linked list. Making it doubly linked would obviously
not be hard though.

-Carl

Re: notmuch_threads_back and notmuch_messages_back

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

On 19:57, Fri 27 Nov 09, Carl Worth wrote:
> (I'd like a verb that pairs better
> with "advance" than the non-verb "back"---any suggestions)?

What about regress?
I'm not a native English speaker, so maybe someone can suggest something better.

> So those won't need any new code. The one case that will need new code
> is that for notmuch_message_get_replies and
> notmuch_message_get_toplevel_messages the messages iterator is currently
> built on a singly-linked list. Making it doubly linked would obviously
> not be hard though.

I'm trying to convert it to a doubly linked list, but I'm not sure if I
understand well how they work.

I don't see how _notmuch_message_list_append works. It says "node can of course
point to an arbitrarily long list of nodes", but the code is:

void
_notmuch_message_list_append (notmuch_message_list_t *list,                
                  notmuch_message_node_t *node)                            
{   
    *(list->tail) = node;
    list->tail = &node->next;
}

Should not be something like:

void
_notmuch_message_list_append (notmuch_message_list_t *list,                
                  notmuch_message_node_t *node)                            
{   
    notmuch_message_node_t *next_node;

    *(list->tail) = node;
	for (next_node = node->next; next_node->next != NULL;
	     next_node = next_node->next);
	list->tail = &next_node->next;
}

Do I miss something? Or the function is just designed to work on a single node
not a list of them?


-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
         No vamos a reivindicar nada,
            no vamos a pedir nada.
            Tomaremos, okuparemos.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: notmuch_threads_back and notmuch_messages_back

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

On Sun, 6 Dec 2009 18:34:37 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> Do I miss something? Or the function is just designed to work on a single node
> not a list of them?

The latter. The documentation for the function is wrong. Sorry about that.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

regress option to messages iterator

[meskio]

Two patches for implement the regress functions on messages. With them 
notmuch_messages_t can be use as an iterator forwards and backwards.

Up to now not really useful, nothing use them. I send them mainly to review.
I'll do the same to threads and tags, so the UIs can use them.

PS: It's the first time I'm using git-send-email, I hope I do everything well.

[PATCH 1/2] Convert notmuch_message_list_t in a doubly linked

[meskio]

From: Ruben Pollan <meskio@sindominio.net>

The messages list now have pointers to previous nodes, so it is possible to
iterate backwards.
---
 lib/messages.c        |   18 +++++++++++++-----
 lib/notmuch-private.h |    3 ++-
 lib/thread.cc         |    4 ++--
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/messages.c b/lib/messages.c
index aa92535..5414f87 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -37,20 +37,28 @@ _notmuch_message_list_create (const void *ctx)
 	return NULL;
 
     list->head = NULL;
-    list->tail = &list->head;
+    list->tail = NULL;
 
     return list;
 }
 
-/* Append 'node' (which can of course point to an arbitrarily long
- * list of nodes) to the end of 'list'.
+/* Append 'node' to the end of 'list'.
  */
 void
 _notmuch_message_list_append (notmuch_message_list_t *list,
 			      notmuch_message_node_t *node)
 {
-    *(list->tail) = node;
-    list->tail = &node->next;
+    node->prev = list->tail;
+    if (list->head)
+    {
+        list->tail->next = node;
+    }
+    else
+    {
+        list->head = node;
+        list->tail = node;
+    }
+    list->tail = node;
 }
 
 /* Allocate a new node for 'message' and append it to the end of
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 0c340a7..133ed0e 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -285,11 +285,12 @@ notmuch_message_file_get_header (notmuch_message_file_t *message,
 typedef struct _notmuch_message_node {
     notmuch_message_t *message;
     struct _notmuch_message_node *next;
+    struct _notmuch_message_node *prev;
 } notmuch_message_node_t;
 
 typedef struct _notmuch_message_list {
     notmuch_message_node_t *head;
-    notmuch_message_node_t **tail;
+    notmuch_message_node_t *tail;
 } notmuch_message_list_t;
 
 /* There's a rumor that there's an alternate struct _notmuch_messages
diff --git a/lib/thread.cc b/lib/thread.cc
index 321937b..24dbe2c 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -169,8 +169,8 @@ _resolve_thread_relationships (unused (notmuch_thread_t *thread))
 					  (void **) &parent))
 	{
 	    *prev = node->next;
-	    if (thread->message_list->tail == &node->next)
-		thread->message_list->tail = prev;
+	    if (thread->message_list->tail == node)
+		thread->message_list->tail = node->prev;
 	    node->next = NULL;
 	    _notmuch_message_add_reply (parent, node);
 	} else {
-- 
1.6.5.4

[PATCH 2/2] Added regress option to messages iterator

[meskio]

From: Ruben Pollan <meskio@sindominio.net>

Added the functions notmuch_messages_regress and notmuch_messages_is_first to
notmuch library. With them is possible to iterate backwards on messages.

* notmuch_messages_regress do the opposite than notmuch_messages_advance,
  getting the messages iterator one position backwards.

* notmuch_messages_is_first return TRUE if the iterator is in the first
  message.
---
 lib/messages.c        |   27 +++++++++++++++++++++++++++
 lib/notmuch-private.h |    7 +++++++
 lib/notmuch.h         |    8 ++++++++
 lib/query.cc          |   24 ++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/lib/messages.c b/lib/messages.c
index 5414f87..2c28738 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -90,6 +90,7 @@ _notmuch_messages_create (notmuch_message_list_t *list)
 
     messages->is_of_list_type = TRUE;
     messages->iterator = list->head;
+    messages->previous_node = NULL;
 
     return messages;
 }
@@ -121,6 +122,18 @@ notmuch_messages_has_more (notmuch_messages_t *messages)
     return (messages->iterator != NULL);
 }
 
+notmuch_bool_t
+notmuch_messages_is_first (notmuch_messages_t *messages)
+{
+    if (messages == NULL)
+	return TRUE;
+
+    if (! messages->is_of_list_type)
+	return _notmuch_mset_messages_is_first (messages);
+
+    return (messages->previous_node == NULL);
+}
+
 notmuch_message_t *
 notmuch_messages_get (notmuch_messages_t *messages)
 {
@@ -142,10 +155,24 @@ notmuch_messages_advance (notmuch_messages_t *messages)
     if (messages->iterator == NULL)
 	return;
 
+    messages->previous_node = messages->iterator;
     messages->iterator = messages->iterator->next;
 }
 
 void
+notmuch_messages_regress (notmuch_messages_t *messages)
+{
+    if (! messages->is_of_list_type)
+	return _notmuch_mset_messages_regress (messages);
+
+    if (messages->previous_node == NULL)
+	return;
+
+    messages->iterator = messages->previous_node;
+    messages->previous_node = messages->iterator->prev;
+}
+
+void
 notmuch_messages_destroy (notmuch_messages_t *messages)
 {
     talloc_free (messages);
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 133ed0e..5852c00 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -299,6 +299,7 @@ typedef struct _notmuch_message_list {
  */
 struct _notmuch_messages {
     notmuch_bool_t is_of_list_type;
+    notmuch_message_node_t *previous_node;
     notmuch_message_node_t *iterator;
 };
 
@@ -321,12 +322,18 @@ _notmuch_messages_create (notmuch_message_list_t *list);
 notmuch_bool_t
 _notmuch_mset_messages_has_more (notmuch_messages_t *messages);
 
+notmuch_bool_t
+_notmuch_mset_messages_is_first (notmuch_messages_t *messages);
+
 notmuch_message_t *
 _notmuch_mset_messages_get (notmuch_messages_t *messages);
 
 void
 _notmuch_mset_messages_advance (notmuch_messages_t *messages);
 
+void
+_notmuch_mset_messages_regress (notmuch_messages_t *messages);
+
 /* message.cc */
 
 void
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 60834fb..69bd98a 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -604,6 +604,10 @@ notmuch_thread_destroy (notmuch_thread_t *thread);
 notmuch_bool_t
 notmuch_messages_has_more (notmuch_messages_t *messages);
 
+/* Is the given notmuch_messages_t object on the first message */
+notmuch_bool_t
+notmuch_messages_is_first (notmuch_messages_t *messages);
+
 /* Get the current message from 'messages' as a notmuch_message_t.
  *
  * Note: The returned message belongs to 'messages' and has a lifetime
@@ -626,6 +630,10 @@ notmuch_messages_get (notmuch_messages_t *messages);
 void
 notmuch_messages_advance (notmuch_messages_t *messages);
 
+/* Regress the 'messages' iterator to the previous result. */
+void
+notmuch_messages_regress (notmuch_messages_t *messages);
+
 /* Destroy a notmuch_messages_t object.
  *
  * It's not strictly necessary to call this function. All memory from
diff --git a/lib/query.cc b/lib/query.cc
index 9106b92..94a6860 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -35,6 +35,7 @@ typedef struct _notmuch_mset_messages {
     notmuch_messages_t base;
     notmuch_database_t *notmuch;
     Xapian::MSetIterator iterator;
+    Xapian::MSetIterator iterator_begin;
     Xapian::MSetIterator iterator_end;
 } notmuch_mset_messages_t;
 
@@ -86,6 +87,7 @@ static int
 _notmuch_messages_destructor (notmuch_mset_messages_t *messages)
 {
     messages->iterator.~MSetIterator ();
+    messages->iterator_begin.~MSetIterator ();
     messages->iterator_end.~MSetIterator ();
 
     return 0;
@@ -108,6 +110,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	messages->base.iterator = NULL;
 	messages->notmuch = notmuch;
 	new (&messages->iterator) Xapian::MSetIterator ();
+	new (&messages->iterator_begin) Xapian::MSetIterator ();
 	new (&messages->iterator_end) Xapian::MSetIterator ();
 
 	talloc_set_destructor (messages, _notmuch_messages_destructor);
@@ -155,6 +158,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	mset = enquire.get_mset (0, notmuch->xapian_db->get_doccount ());
 
 	messages->iterator = mset.begin ();
+	messages->iterator_end = mset.begin ();
 	messages->iterator_end = mset.end ();
 
     } catch (const Xapian::Error &error) {
@@ -177,6 +181,16 @@ _notmuch_mset_messages_has_more (notmuch_messages_t *messages)
     return (mset_messages->iterator != mset_messages->iterator_end);
 }
 
+notmuch_bool_t
+_notmuch_mset_messages_is_first (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    return (mset_messages->iterator == mset_messages->iterator_begin);
+}
+
 notmuch_message_t *
 _notmuch_mset_messages_get (notmuch_messages_t *messages)
 {
@@ -215,6 +229,16 @@ _notmuch_mset_messages_advance (notmuch_messages_t *messages)
     mset_messages->iterator++;
 }
 
+void
+_notmuch_mset_messages_regress (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    mset_messages->iterator--;
+}
+
 /* Glib objects force use to use a talloc destructor as well, (but not
  * nearly as ugly as the for messages due to C++ objects). At
  * this point, I'd really like to have some talloc-friendly
-- 
1.6.5.4

Re: regress option to messages iterator

[ruben pollan]

[-- Attachment #1.1: Type: text/plain, Size: 849 bytes --]

Attached to this email I add a small code that use this functions, as an
example.

On 10:41, Tue 08 Dec 09, meskio@sindominio.net wrote:
> Two patches for implement the regress functions on messages. With them 
> notmuch_messages_t can be use as an iterator forwards and backwards.
> 
> Up to now not really useful, nothing use them. I send them mainly to review.
> I'll do the same to threads and tags, so the UIs can use them.
> 
> PS: It's the first time I'm using git-send-email, I hope I do everything well.
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
> 

-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
      Si luchas puedes perder
     Si no luchas estas perdido


[-- Attachment #1.2: foo.c --]
[-- Type: text/x-csrc, Size: 886 bytes --]

#include <stdio.h>
#include "lib/notmuch.h"

void
show_message (notmuch_message_t *message)
{
	printf ("%s\n", notmuch_message_get_message_id (message));
}

int
main ()
{
	notmuch_database_t *database;
	notmuch_query_t *query;
	notmuch_messages_t *messages;
	notmuch_message_t *message;
	char ch = '\0';

	database = notmuch_database_open ("/home/meskio/Maildir", NOTMUCH_DATABASE_MODE_READ_ONLY);
	query = notmuch_query_create (database, "inbox");
	messages = notmuch_query_search_messages (query);

	while (ch != 'q')
	{
		message = notmuch_messages_get (messages);
		show_message (message);
		ch = getchar();
		if (ch == 'n')
		{
			notmuch_messages_advance (messages);
			if (!notmuch_messages_has_more (messages))
				notmuch_messages_regress (messages);
		}
		else if (ch == 'p')
		{
			if (!notmuch_messages_is_first (messages))
				notmuch_messages_regress (messages);
		}
	}
}

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] Added regress option to threads iterator

[Ruben Pollan]

Added the functions notmuch_threads_regress and notmuch_threads_is_first to
notmuch library. With them is possible to iterate backwards on threads.

* notmuch_threads_regress do the opposite than notmuch_threads_advance,
  getting the threads iterator one position backwards.

* notmuch_threads_is_first return TRUE if the iterator is in the first
  thread.
---
 lib/notmuch.h |    8 ++++++++
 lib/query.cc  |   28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 69bd98a..e28ce46 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -429,6 +429,10 @@ notmuch_query_destroy (notmuch_query_t *query);
 notmuch_bool_t
 notmuch_threads_has_more (notmuch_threads_t *threads);
 
+/* Is the given notmuch_threads_t object on the first threads */
+notmuch_bool_t
+notmuch_threads_is_first (notmuch_threads_t *threads);
+
 /* Get the current thread from 'threads' as a notmuch_thread_t.
  *
  * Note: The returned thread belongs to 'threads' and has a lifetime
@@ -451,6 +455,10 @@ notmuch_threads_get (notmuch_threads_t *threads);
 void
 notmuch_threads_advance (notmuch_threads_t *threads);
 
+/* Regress the 'threads' iterator to the previous result. */
+void
+notmuch_threads_regress (notmuch_threads_t *threads);
+
 /* Destroy a notmuch_threads_t object.
  *
  * It's not strictly necessary to call this function. All memory from
diff --git a/lib/query.cc b/lib/query.cc
index 94a6860..cade17b 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -310,6 +310,12 @@ notmuch_threads_has_more (notmuch_threads_t *threads)
     return FALSE;
 }
 
+notmuch_bool_t
+notmuch_threads_is_first (notmuch_threads_t *threads)
+{
+    return (g_hash_table_size (threads->threads) <= 1);
+}
+
 notmuch_thread_t *
 notmuch_threads_get (notmuch_threads_t *threads)
 {
@@ -329,6 +335,28 @@ notmuch_threads_advance (notmuch_threads_t *threads)
 }
 
 void
+notmuch_threads_regress (notmuch_threads_t *threads)
+{
+    notmuch_message_t *message;
+	const char *thread_id;
+
+	thread_id = threads->thread_id;
+
+    while (!notmuch_messages_is_first (threads->messages))
+    {
+	notmuch_messages_regress (threads->messages);
+	message = notmuch_messages_get (threads->messages);
+	threads->thread_id = notmuch_message_get_thread_id (message);
+
+	if (strcmp (threads->thread_id, thread_id))
+	{
+	    g_hash_table_remove (threads->threads, thread_id);
+	    return;
+	}
+    }
+}
+
+void
 notmuch_threads_destroy (notmuch_threads_t *threads)
 {
     talloc_free (threads);
-- 
1.6.5.4

[PATCH] Added regress option to tags iterator

[Ruben Pollan]

Added the functions notmuch_tags_regress and notmuch_tags_is_first to
notmuch library. With them is possible to iterate backwards on tags.

* notmuch_tags_regress do the opposite than notmuch_tags_advance,
  getting the tags iterator one position backwards.

* notmuch_tags_is_first return TRUE if the iterator is in the first
  tag.
---
 lib/notmuch.h |    8 ++++++++
 lib/tags.c    |   19 +++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index e28ce46..db051c8 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -917,6 +917,10 @@ notmuch_message_destroy (notmuch_message_t *message);
 notmuch_bool_t
 notmuch_tags_has_more (notmuch_tags_t *tags);
 
+/* Is the given notmuch_tags_t object on the first tags */
+notmuch_bool_t
+notmuch_tags_is_first (notmuch_tags_t *tags);
+
 /* Get the current tag from 'tags' as a string.
  *
  * Note: The returned string belongs to 'tags' and has a lifetime
@@ -936,6 +940,10 @@ notmuch_tags_get (notmuch_tags_t *tags);
 void
 notmuch_tags_advance (notmuch_tags_t *tags);
 
+/* Regress the 'tags' iterator to the previous result. */
+void
+notmuch_tags_regress (notmuch_tags_t *tags);
+
 /* Destroy a notmuch_tags_t object.
  *
  * It's not strictly necessary to call this function. All memory from
diff --git a/lib/tags.c b/lib/tags.c
index 85507e9..cf9e030 100644
--- a/lib/tags.c
+++ b/lib/tags.c
@@ -25,6 +25,7 @@
 struct _notmuch_tags {
     int sorted;
     GList *tags;
+    GList *previous_node;
     GList *iterator;
 };
 
@@ -55,6 +56,7 @@ _notmuch_tags_create (void *ctx)
 
     tags->sorted = 1;
     tags->tags = NULL;
+    tags->previous_node = NULL;
     tags->iterator = NULL;
 
     return tags;
@@ -94,6 +96,12 @@ notmuch_tags_has_more (notmuch_tags_t *tags)
     return tags->iterator != NULL;
 }
 
+notmuch_bool_t
+notmuch_tags_is_first (notmuch_tags_t *tags)
+{
+    return tags->previous_node == NULL;
+}
+
 const char *
 notmuch_tags_get (notmuch_tags_t *tags)
 {
@@ -109,10 +117,21 @@ notmuch_tags_advance (notmuch_tags_t *tags)
     if (tags->iterator == NULL)
 	return;
 
+    tags->previous_node = tags->iterator;
     tags->iterator = tags->iterator->next;
 }
 
 void
+notmuch_tags_regress (notmuch_tags_t *tags)
+{
+    if (tags->previous_node == NULL)
+	return;
+
+	tags->iterator = tags->previous_node;
+	tags->previous_node = tags->iterator->prev;
+}
+
+void
 notmuch_tags_destroy (notmuch_tags_t *tags)
 {
     talloc_free (tags);
-- 
1.6.5.4

Re: [PATCH] Added regress option to tags iterator

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

With the last 4 patches there is more or less all I wanted implemented.

Do you like to call them regress? Should I change that?

What about the functions notmuch_*_is_first? Is kind of reversed logic than
notmuch_*_has_more, the last are true when is not reach the limit but the
first ones are true when the limit is reached. But I think it make sense like
that.


On 14:10, Wed 09 Dec 09, Ruben Pollan wrote:
> Added the functions notmuch_tags_regress and notmuch_tags_is_first to
> notmuch library. With them is possible to iterate backwards on tags.
> 
[...]

-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Sed realistas, exigid lo imposible.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

On Wed, 9 Dec 2009 14:24:46 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> Do you like to call them regress? Should I change that?

I don't love the name, (since it's so close to the word "regression"
which has a totally different meaning in software context). But I also
don't have an immediate suggestion for an improved name yet either.

> What about the functions notmuch_*_is_first? Is kind of reversed logic than
> notmuch_*_has_more, the last are true when is not reach the limit but the
> first ones are true when the limit is reached. But I think it make sense like
> that.

I'd like a more symmetric API here. Anyone have a favorite set of names
for iterating a list in two directions?

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Mark Anderson]

Excerpts from Carl Worth's message of Wed Dec 09 13:08:43 -0700 2009:
> On Wed, 9 Dec 2009 14:24:46 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> > Do you like to call them regress? Should I change that?
> 
> I don't love the name, (since it's so close to the word "regression"
> which has a totally different meaning in software context). But I also
> don't have an immediate suggestion for an improved name yet either.
> 
> > What about the functions notmuch_*_is_first? Is kind of reversed logic than
> > notmuch_*_has_more, the last are true when is not reach the limit but the
> > first ones are true when the limit is reached. But I think it make sense like
> > that.
> 
> I'd like a more symmetric API here. Anyone have a favorite set of names
> for iterating a list in two directions?

I like vocabulary games:

fwd/bck
forward/reverse
next/prev
advance/retreat
inc/dec
iter_fwd/iter_back
earlier/later
younger/older

I think that changing has_more is going to be a requirement to come up with a consistent set of names.

> 
> -Carl

Re: [PATCH] Added regress option to tags iterator

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Wed, 9 Dec 2009 14:21:04 -0700, Mark Anderson <markr.anderson@amd.com> wrote:
> I think that changing has_more is going to be a requirement to come up with a consistent set of names.

What? You don't want to pair it with has_less ?

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

On 12:08, Wed 09 Dec 09, Carl Worth wrote:
> On Wed, 9 Dec 2009 14:24:46 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> > Do you like to call them regress? Should I change that?
> 
> I don't love the name, (since it's so close to the word "regression"
> which has a totally different meaning in software context). But I also
> don't have an immediate suggestion for an improved name yet either.

Me neither, but as I don't have any better idea I just use regress. But we can
try to come up with something better.

> 
> > What about the functions notmuch_*_is_first? Is kind of reversed logic than
> > notmuch_*_has_more, the last are true when is not reach the limit but the
> > first ones are true when the limit is reached. But I think it make sense like
> > that.
> 
> I'd like a more symmetric API here. Anyone have a favorite set of names
> for iterating a list in two directions?

Yes, but actually are a bit different somehow. When you advance the last
iterator you can reach is a non-valid (outside the list) iterator. When you
'regress' the iterator at the end is a valid iterator, I don't see the need on
get outside of the list like with advance.

So, maybe because of it have sense the functions notmuch_*_is_first. Anyway I
really don't mind, one or other. To change it is trivial. It's harder to find a
good pair of names.




-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
   La felicidad no es hacer lo que deseas
         es desear lo que haces.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On 14:21, Wed 09 Dec 09, Mark Anderson wrote:
> advance/retreat

Jeffrey already suggested retreat. I let you (English speakers) decide, my English
level is not enough for it.

-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Ésta es la historia de una sociedad que se hunde
y mientras cae se repite:
hasta ahora todo va bien, hasta ahora todo va bien ...
Pero lo importante no es la caida sino el aterrizaje.
                               el odio

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

On Wed, 09 Dec 2009 12:08:43 -0800, Carl Worth <cworth@cworth.org> wrote:
> On Wed, 9 Dec 2009 14:24:46 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> > Do you like to call them regress? Should I change that?
...
> > What about the functions notmuch_*_is_first? Is kind of reversed logic than
> > notmuch_*_has_more, the last are true when is not reach the limit but the
> > first ones are true when the limit is reached. But I think it make sense like
> > that.

It doesn't make sense in the case of trying to write a for loop that
iterates in the reverse order. The is_first semantic doesn't give you
what you want for the loop-control portion of the for loop.

> I'd like a more symmetric API here. Anyone have a favorite set of names
> for iterating a list in two directions?

In some recent coding I needed to implement a new iterator so I had the
chance to think about this some more. Here is what I came up with:

  New function		Corresponds to existing function (if any)
  ------------		-----------------------------------------
  move_to_first		<implicit in iterator creation>
  has_next		has_more
  move_to_next		advance

  move_to_last		<none>
  has_previous		<none>
  move_to_previous	<none>

  get			get

The semantics of those all seem clear enough to me. They provide what's
necessary for all three portions of a for loop, (in either direction),
and everything pairs nicely.

The only downside is that the function names are a bit long in some
cases, but I'm willing to live with that until someone comes up with
better.

Thoughts?

-Carl


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

On Mon, 21 Dec 2009 17:23:55 -0800, Carl Worth <cworth@cworth.org> wrote:
>   New function		Corresponds to existing function (if any)
>   ------------		-----------------------------------------
>   move_to_first		<implicit in iterator creation>
>   has_next		has_more
>   move_to_next		advance
> 
>   move_to_last		<none>
>   has_previous		<none>
>   move_to_previous	<none>
> 
>   get			get
> 
> The semantics of those all seem clear enough to me. They provide what's
> necessary for all three portions of a for loop, (in either direction),

Except that they don't. :-P

We don't want has_next and has_previous but something more like "has
current", (perhaps to pair with get_current?).

> The only downside is that the function names are a bit long in some
> cases, but I'm willing to live with that until someone comes up with
> better.

One option is to just drop the "move_ " prefix. Then everything will be
a two-word function. So the new proposal is:

to_first
has_current
to_next

to_last
has_current
to_previous

get_current

Better?

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

On 19:16, Mon 21 Dec 09, Carl Worth wrote:
> On Mon, 21 Dec 2009 17:23:55 -0800, Carl Worth <cworth@cworth.org> wrote:
> >   New function		Corresponds to existing function (if any)
> >   ------------		-----------------------------------------
> >   move_to_first		<implicit in iterator creation>
> >   has_next		has_more
> >   move_to_next		advance
> > 
> >   move_to_last		<none>
> >   has_previous		<none>
> >   move_to_previous	<none>
> > 
> >   get			get
> > 
> > The semantics of those all seem clear enough to me. They provide what's
> > necessary for all three portions of a for loop, (in either direction),
> 
> Except that they don't. :-P
> 
> We don't want has_next and has_previous but something more like "has
> current", (perhaps to pair with get_current?).

Not sure if I understand that. Let's see if I understand well. move_to_first
(or move_to_last) will put the iterator in the first (or last) valid item.
move_to_next (and move_to_previous) will be able to reach an invalid item
outside the list. Is it like that?

In some implementations of iterators (like C++ STD) you can reach invalid items
only in one side of the list, at the end, but not at the beginning. Some people
get use to this idea, but should not be a big deal to do it different.

So you are thinking in a function has_current showing if the current item is
valid. Am I right?

> > The only downside is that the function names are a bit long in some
> > cases, but I'm willing to live with that until someone comes up with
> > better.
> 
> One option is to just drop the "move_ " prefix.

I think that's a good option. The names of the functions are still clear like
that, and the original names are too long.


PS: Sorry for the late reply, Christmas is a busy time.


-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Ésta es la historia de una sociedad que se hunde
y mientras cae se repite:
hasta ahora todo va bien, hasta ahora todo va bien ...
Pero lo importante no es la caida sino el aterrizaje.
                               el odio

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

On Tue, 5 Jan 2010 16:33:32 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> Not sure if I understand that. Let's see if I understand well. move_to_first
> (or move_to_last) will put the iterator in the first (or last) valid item.
> move_to_next (and move_to_previous) will be able to reach an invalid item
> outside the list. Is it like that?

Yes, that's the idea.

> In some implementations of iterators (like C++ STD) you can reach invalid items
> only in one side of the list, at the end, but not at the beginning. Some people
> get use to this idea, but should not be a big deal to do it different.

Yes, I've seen interfaces like that. They don't make sense to me since
then one direction or the other is much harder to iterate, (the
interface won't afford an easy for-loop-based iteration for example).

> So you are thinking in a function has_current showing if the current item is
> valid. Am I right?

Right. So example code using this would be:

	for (notmuch_messages_to_first (messages);
             notmuch_messages_has_current (messages);
             notmuch_messages_to_next (messages))
	{
		notmuch_message_t *message;

		message = notmuch_messages_get_current (messages);

		...
	}

And for iterating in the opposite direction it's very similar:

	for (notmuch_messages_to_last (messages);
             notmuch_messages_has_current (messages);
             notmuch_messages_to_previous (messages))
	{
		notmuch_message_t *message;

		message = notmuch_messages_get_current (messages);

		...
	}

Note that if you couldn't get the iterator to point to an invalid item
before the first, then this second loop would have to look very
different.

> I think that's a good option. The names of the functions are still clear like
> that, and the original names are too long.
> 
> PS: Sorry for the late reply, Christmas is a busy time.

Thanks for the feedback. And no worries about the late reply---I've been
quite busy myself.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

On 11:39, Tue 05 Jan 10, Carl Worth wrote:
> Right. So example code using this would be:
> 
> 	for (notmuch_messages_to_first (messages);
>              notmuch_messages_has_current (messages);
>              notmuch_messages_to_next (messages))
> 	{
> 		notmuch_message_t *message;
> 
> 		message = notmuch_messages_get_current (messages);
> 
> 		...
> 	}
> 
> And for iterating in the opposite direction it's very similar:
> 
> 	for (notmuch_messages_to_last (messages);
>              notmuch_messages_has_current (messages);
>              notmuch_messages_to_previous (messages))
> 	{
> 		notmuch_message_t *message;
> 
> 		message = notmuch_messages_get_current (messages);
> 
> 		...
> 	}
> 
> Note that if you couldn't get the iterator to point to an invalid item
> before the first, then this second loop would have to look very
> different.

Yes, make sense like that.


I'm not sure what to do about the iterator when is on an invalid item, if you 
reach an invalid item doing to_next should be possible to do to_previous to 
recover the last valid item? Or is better to force the user to use to_last to 
go back?

In some cases implement to_previous will need to store the previous item 
(actually I did that on the patches I sent).

-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Ésta es la historia de una sociedad que se hunde
y mientras cae se repite:
hasta ahora todo va bien, hasta ahora todo va bien ...
Pero lo importante no es la caida sino el aterrizaje.
                               el odio

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On Mon, 21 Dec 2009 19:16:49 -0800, Carl Worth <cworth@cworth.org> wrote:
> One option is to just drop the "move_ " prefix. Then everything will be
> a two-word function. So the new proposal is:
> 
> to_first
> has_current
> to_next
> 
> to_last
> has_current
> to_previous
> 
> get_current
> 
> Better?

Looking back at this proposal now, (after a long delay), I found I
didn't like it at all. With function names like:

	notmuch_threads_to_first
	notmuch_threads_to_next

the missing verb in the name is really distracting. I ended up reading
these names as if they were conversion functions. So I've gone back to
preferring the names with the explicit verbs (even though quite long):

	notmuch_threads_move_to_first
	notmuch_threads_move_to_next

Meanwhile, I also decided that _current wasn't adding anything to the
names it showed up in, (we can never "get" anything other than the
current item, so why qualify as "get_current"?). So I'm changing from
"has_current" to "valid", and leaving "get" as it is.

So the final proposal for iteration in either direction is:

	move_to_first
        valid
        move_to_next

	move_to_last
        valid
        move_to_previous

        get

I've just pushed commits changing the existing functions (which allow
only forward iteration) to use this naming scheme. I haven't added any
of the reverse-iteration functions yet, so Ruben, if you'd like to do
those within this scheme, that would be find. (Or we could wait until we
have an actual use in mind for them.)

Thanks,

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Added regress option to tags iterator

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 1389 bytes --]

On 09:36, Tue 09 Mar 10, Carl Worth wrote:
> So the final proposal for iteration in either direction is:
> 
> 	move_to_first
>         valid
>         move_to_next
> 
> 	move_to_last
>         valid
>         move_to_previous
> 
>         get

Fine for me.

> I've just pushed commits changing the existing functions (which allow
> only forward iteration) to use this naming scheme. I haven't added any
> of the reverse-iteration functions yet, so Ruben, if you'd like to do
> those within this scheme, that would be find. (Or we could wait until we
> have an actual use in mind for them.)

I'll implemented them again and we'll decide if make sense to have them even if
there is no code now using them. I think is a useful feature to create a client
based on notmuch library, but I also think is bad behavior to keep code that is 
not in use.

I'm still thinking in a ncurses client (but I didn't hack on that on a while)
and I guess I'll use this functions on it. But I don't mind to have them just in
my own repository until I have something working.

I'm not having too much hacking time lately, so I don't know when I'll have
some time for it. I hope in few days.


-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 Cuando los que mandan pierden la vergüenza,
    los que obedecen pierden el respeto.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

reverse iterators

[Ruben Pollan]

Adds support to reverse iteration on messages, threads and tags. To revew and
think if makes sense to include them on notmuch or wait until they have a real
use.

The patch 3 "Move the logic of threads iterator out of 'valid'" is just a 
reorganization of the code, I think it makes sense to include it in notmuch even
if we decide that the reverse iterators are not included.

[PATCH 1/5] Convert notmuch_message_list_t in a doubly linked

[Ruben Pollan]

The messages list now have pointers to previous nodes, so it is possible
to iterate backwards.
---
 lib/messages.c        |   18 +++++++++++++-----
 lib/notmuch-private.h |    3 ++-
 lib/thread.cc         |    4 ++--
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/messages.c b/lib/messages.c
index db2b7a1..2a85774 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -37,20 +37,28 @@ _notmuch_message_list_create (const void *ctx)
 	return NULL;
 
     list->head = NULL;
-    list->tail = &list->head;
+    list->tail = NULL;
 
     return list;
 }
 
-/* Append 'node' (which can of course point to an arbitrarily long
- * list of nodes) to the end of 'list'.
+/* Append 'node' to the end of 'list'.
  */
 void
 _notmuch_message_list_append (notmuch_message_list_t *list,
 			      notmuch_message_node_t *node)
 {
-    *(list->tail) = node;
-    list->tail = &node->next;
+    node->prev = list->tail;
+    if (list->head)
+    {
+        list->tail->next = node;
+    }
+    else
+    {
+        list->head = node;
+        list->tail = node;
+    }
+    list->tail = node;
 }
 
 /* Allocate a new node for 'message' and append it to the end of
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index d52d84d..3b3f0eb 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -349,11 +349,12 @@ notmuch_message_file_get_header (notmuch_message_file_t *message,
 typedef struct _notmuch_message_node {
     notmuch_message_t *message;
     struct _notmuch_message_node *next;
+    struct _notmuch_message_node *prev;
 } notmuch_message_node_t;
 
 typedef struct _notmuch_message_list {
     notmuch_message_node_t *head;
-    notmuch_message_node_t **tail;
+    notmuch_message_node_t *tail;
 } notmuch_message_list_t;
 
 /* There's a rumor that there's an alternate struct _notmuch_messages
diff --git a/lib/thread.cc b/lib/thread.cc
index ec80f85..05d2c39 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -169,8 +169,8 @@ _resolve_thread_relationships (unused (notmuch_thread_t *thread))
 					  (void **) &parent))
 	{
 	    *prev = node->next;
-	    if (thread->message_list->tail == &node->next)
-		thread->message_list->tail = prev;
+	    if (thread->message_list->tail == node)
+		thread->message_list->tail = node->prev;
 	    node->next = NULL;
 	    _notmuch_message_add_reply (parent, node);
 	} else {
-- 
1.7.0

[PATCH 2/5] Added backwards iterator to messages

[Ruben Pollan]

Added the functions notmuch_messages_move_to_prevoius,
notmuch_messages_move_to_last and  notmuch_messages_move_to_first to
notmuch library. With them is possible to iterate backwards on messages.

* notmuch_messages_move_to_prevoius do the opposite than
  notmuch_messages_move_to_next, getting the messages iterator one
  position backwards.

* notmuch_messages_move_to_last move the iterator to the first last
  message.

* notmuch_messages_move_to_first move the iterator to the first valid
  message.
---
 lib/messages.c        |   31 +++++++++++++++++++++++++++++
 lib/notmuch-private.h |   10 +++++++++
 lib/notmuch.h         |   28 ++++++++++++++++++++++++++
 lib/query.cc          |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 121 insertions(+), 0 deletions(-)

diff --git a/lib/messages.c b/lib/messages.c
index 2a85774..975e4b1 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -90,6 +90,7 @@ _notmuch_messages_create (notmuch_message_list_t *list)
 
     messages->is_of_list_type = TRUE;
     messages->iterator = list->head;
+    messages->list = list;
 
     return messages;
 }
@@ -134,6 +135,15 @@ notmuch_messages_get (notmuch_messages_t *messages)
 }
 
 void
+notmuch_messages_move_to_first (notmuch_messages_t *messages)
+{
+    if (! messages->is_of_list_type)
+	return _notmuch_mset_messages_move_to_first (messages);
+
+    messages->iterator = messages->list->head;
+}
+
+void
 notmuch_messages_move_to_next (notmuch_messages_t *messages)
 {
     if (! messages->is_of_list_type)
@@ -146,6 +156,27 @@ notmuch_messages_move_to_next (notmuch_messages_t *messages)
 }
 
 void
+notmuch_messages_move_to_last (notmuch_messages_t *messages)
+{
+    if (! messages->is_of_list_type)
+	return _notmuch_mset_messages_move_to_last (messages);
+
+    messages->iterator = messages->list->tail;
+}
+
+void
+notmuch_messages_move_to_previous (notmuch_messages_t *messages)
+{
+    if (! messages->is_of_list_type)
+	return _notmuch_mset_messages_move_to_previous (messages);
+
+    if (messages->iterator == NULL)
+	return;
+
+    messages->iterator = messages->iterator->prev;
+}
+
+void
 notmuch_messages_destroy (notmuch_messages_t *messages)
 {
     talloc_free (messages);
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 3b3f0eb..2269d2b 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -364,6 +364,7 @@ typedef struct _notmuch_message_list {
 struct _notmuch_messages {
     notmuch_bool_t is_of_list_type;
     notmuch_message_node_t *iterator;
+    notmuch_message_list_t *list;
 };
 
 notmuch_message_list_t *
@@ -389,8 +390,17 @@ notmuch_message_t *
 _notmuch_mset_messages_get (notmuch_messages_t *messages);
 
 void
+_notmuch_mset_messages_move_to_first (notmuch_messages_t *messages);
+
+void
 _notmuch_mset_messages_move_to_next (notmuch_messages_t *messages);
 
+void
+_notmuch_mset_messages_move_to_last (notmuch_messages_t *messages);
+
+void
+_notmuch_mset_messages_move_to_previous (notmuch_messages_t *messages);
+
 /* message.cc */
 
 void
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0d9cb0f..753f3bb 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -645,6 +645,15 @@ notmuch_messages_valid (notmuch_messages_t *messages);
 notmuch_message_t *
 notmuch_messages_get (notmuch_messages_t *messages);
 
+/* Move the 'messages' iterator to the first message.
+ *
+ * After that the 'messages' iterator will be set to the first valid 
+ * message, so it can be use to iterate with 
+ * notmuch_messages_move_to_next.
+ */
+void
+notmuch_messages_move_to_first (notmuch_messages_t *messages);
+
 /* Move the 'messages' iterator to the next message.
  *
  * If 'messages' is already pointing at the last message then the
@@ -658,6 +667,25 @@ notmuch_messages_get (notmuch_messages_t *messages);
 void
 notmuch_messages_move_to_next (notmuch_messages_t *messages);
 
+/* Move the 'messages' iterator to the last message.
+ *
+ * After that the 'messages' iterator will be set to the last valid 
+ * message, so it can be use to iterate with 
+ * notmuch_messages_move_to_previous.
+ */
+void
+notmuch_messages_move_to_last (notmuch_messages_t *messages);
+
+/* Move the 'messages' iterator to the previous message.
+ *
+ * If 'messages' is already pointing at the first message then the
+ * iterator will be moved to a point just beyond that first message,
+ * (where notmuch_messages_valid will return FALSE and
+ * notmuch_messages_get will return NULL).
+ */
+void
+notmuch_messages_move_to_previous (notmuch_messages_t *messages);
+
 /* Destroy a notmuch_messages_t object.
  *
  * It's not strictly necessary to call this function. All memory from
diff --git a/lib/query.cc b/lib/query.cc
index 9266d35..970c35a 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -35,6 +35,7 @@ typedef struct _notmuch_mset_messages {
     notmuch_messages_t base;
     notmuch_database_t *notmuch;
     Xapian::MSetIterator iterator;
+    Xapian::MSetIterator iterator_begin;
     Xapian::MSetIterator iterator_end;
 } notmuch_mset_messages_t;
 
@@ -86,6 +87,7 @@ static int
 _notmuch_messages_destructor (notmuch_mset_messages_t *messages)
 {
     messages->iterator.~MSetIterator ();
+    messages->iterator_begin.~MSetIterator ();
     messages->iterator_end.~MSetIterator ();
 
     return 0;
@@ -108,6 +110,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	messages->base.iterator = NULL;
 	messages->notmuch = notmuch;
 	new (&messages->iterator) Xapian::MSetIterator ();
+	new (&messages->iterator_begin) Xapian::MSetIterator ();
 	new (&messages->iterator_end) Xapian::MSetIterator ();
 
 	talloc_set_destructor (messages, _notmuch_messages_destructor);
@@ -157,6 +160,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	mset = enquire.get_mset (0, notmuch->xapian_db->get_doccount ());
 
 	messages->iterator = mset.begin ();
+	messages->iterator_begin = mset.begin ();
 	messages->iterator_end = mset.end ();
 
     } catch (const Xapian::Error &error) {
@@ -208,6 +212,16 @@ _notmuch_mset_messages_get (notmuch_messages_t *messages)
 }
 
 void
+_notmuch_mset_messages_move_to_first (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    mset_messages->iterator = mset_messages->iterator_begin;
+}
+
+void
 _notmuch_mset_messages_move_to_next (notmuch_messages_t *messages)
 {
     notmuch_mset_messages_t *mset_messages;
@@ -217,6 +231,44 @@ _notmuch_mset_messages_move_to_next (notmuch_messages_t *messages)
     mset_messages->iterator++;
 }
 
+void
+_notmuch_mset_messages_move_to_last (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    mset_messages->iterator = mset_messages->iterator_end;
+    mset_messages->iterator--;
+}
+
+void
+_notmuch_mset_messages_move_to_previous (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    if (mset_messages->iterator == mset_messages->iterator_begin)
+    {
+        /*
+         * Xapian iterators can not be beyond the first element, so we
+         * assign the iterator_end to mark the iterator as invalid in case
+         * of move_to_previous with the iterator at the beginning
+         */
+        mset_messages->iterator = mset_messages->iterator_end;
+    }
+    else if (_notmuch_mset_messages_valid (messages))
+    {
+        /*
+         * If is valid move the iterator. To emulate the same behavior 
+         * than notmuch_messages_t the iterator won't be updated if is 
+         * not valid
+         */
+        mset_messages->iterator--;
+    }
+}
+
 /* Glib objects force use to use a talloc destructor as well, (but not
  * nearly as ugly as the for messages due to C++ objects). At
  * this point, I'd really like to have some talloc-friendly
-- 
1.7.0

[PATCH 3/5] Move the logic of threads iterator out of 'valid'

[Ruben Pollan]

The logic of notmuch_threads_move_to_next iterator was on
notmuch_threads_valid function. Now notmuch_threads_valid just check if
the iterator is valid and is notmuch_threads_move_to_next wich actually
update the iterator.
---
 lib/query.cc |   47 ++++++++++++++++++++++++-----------------------
 1 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/lib/query.cc b/lib/query.cc
index 970c35a..44950c1 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -285,6 +285,7 @@ notmuch_threads_t *
 notmuch_query_search_threads (notmuch_query_t *query)
 {
     notmuch_threads_t *threads;
+    notmuch_message_t *message;
 
     threads = talloc (query, notmuch_threads_t);
     if (threads == NULL)
@@ -296,7 +297,10 @@ notmuch_query_search_threads (notmuch_query_t *query)
 
     threads->messages = notmuch_query_search_messages (query);
 
-    threads->thread_id = NULL;
+    message = notmuch_messages_get (threads->messages);
+    threads->thread_id = notmuch_message_get_thread_id (message);
+    g_hash_table_insert (threads->threads,
+                         xstrdup (threads->thread_id), NULL);
 
     talloc_set_destructor (threads, _notmuch_threads_destructor);
 
@@ -312,10 +316,25 @@ notmuch_query_destroy (notmuch_query_t *query)
 notmuch_bool_t
 notmuch_threads_valid (notmuch_threads_t *threads)
 {
-    notmuch_message_t *message;
+    return (threads->thread_id != NULL);
+}
 
-    if (threads->thread_id)
-	return TRUE;
+notmuch_thread_t *
+notmuch_threads_get (notmuch_threads_t *threads)
+{
+    if (! notmuch_threads_valid (threads))
+	return NULL;
+
+    return _notmuch_thread_create (threads->query,
+				   threads->query->notmuch,
+				   threads->thread_id,
+				   threads->query->query_string);
+}
+
+void
+notmuch_threads_move_to_next (notmuch_threads_t *threads)
+{
+    notmuch_message_t *message;
 
     while (notmuch_messages_valid (threads->messages))
     {
@@ -330,33 +349,15 @@ notmuch_threads_valid (notmuch_threads_t *threads)
 	    g_hash_table_insert (threads->threads,
 				 xstrdup (threads->thread_id), NULL);
 	    notmuch_messages_move_to_next (threads->messages);
-	    return TRUE;
+	    return;
 	}
 
 	notmuch_messages_move_to_next (threads->messages);
     }
 
     threads->thread_id = NULL;
-    return FALSE;
-}
-
-notmuch_thread_t *
-notmuch_threads_get (notmuch_threads_t *threads)
-{
-    if (! notmuch_threads_valid (threads))
-	return NULL;
-
-    return _notmuch_thread_create (threads->query,
-				   threads->query->notmuch,
-				   threads->thread_id,
-				   threads->query->query_string);
 }
 
-void
-notmuch_threads_move_to_next (notmuch_threads_t *threads)
-{
-    threads->thread_id = NULL;
-}
 
 void
 notmuch_threads_destroy (notmuch_threads_t *threads)
-- 
1.7.0

[PATCH 4/5] Added backwards iterator to threads

[Ruben Pollan]

Added the functions notmuch_threads_move_to_prevoius,
notmuch_threads_move_to_last and  notmuch_threads_move_to_first to
notmuch library. With them is possible to iterate backwards on threads.

* notmuch_threads_move_to_prevoius do the opposite than
  notmuch_threads_move_to_next, getting the threads iterator one
  position backwards.

* notmuch_threads_move_to_last move the iterator to the first last thread.

* notmuch_threads_move_to_first move the iterator to the first valid
  thread.

For it has been implemented notmuch_thread_list_t structur that stores
the thread_ids so the backwards iteration gets the thread_id in the same
order that was show on forward iteration.
---
 lib/notmuch.h |   28 +++++++++++
 lib/query.cc  |  143 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 159 insertions(+), 12 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 753f3bb..b96b624 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -466,6 +466,15 @@ notmuch_threads_valid (notmuch_threads_t *threads);
 notmuch_thread_t *
 notmuch_threads_get (notmuch_threads_t *threads);
 
+/* Move the 'threads' iterator to the first thread.
+ *
+ * After that the 'threads' iterator will be set to the first valid 
+ * thread, so it can be use to iterate with 
+ * notmuch_threads_move_to_next.
+ */
+void
+notmuch_threads_move_to_first (notmuch_threads_t *threads);
+
 /* Move the 'threads' iterator to the next thread.
  *
  * If 'threads' is already pointing at the last thread then the
@@ -479,6 +488,25 @@ notmuch_threads_get (notmuch_threads_t *threads);
 void
 notmuch_threads_move_to_next (notmuch_threads_t *threads);
 
+/* Move the 'threads' iterator to the last thread.
+ *
+ * After that the 'threads' iterator will be set to the last valid 
+ * thread, so it can be use to iterate with 
+ * notmuch_threads_move_to_previous.
+ */
+void
+notmuch_threads_move_to_last (notmuch_threads_t *threads);
+
+/* Move the 'threads' iterator to the previous thread.
+ *
+ * If 'threads' is already pointing at the first thread then the
+ * iterator will be moved to a point just beyond that first thread,
+ * (where notmuch_threads_valid will return FALSE and
+ * notmuch_threads_get will return NULL).
+ */
+void
+notmuch_threads_move_to_previous (notmuch_threads_t *threads);
+
 /* Destroy a notmuch_threads_t object.
  *
  * It's not strictly necessary to call this function. All memory from
diff --git a/lib/query.cc b/lib/query.cc
index 44950c1..39985e7 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -39,13 +39,25 @@ typedef struct _notmuch_mset_messages {
     Xapian::MSetIterator iterator_end;
 } notmuch_mset_messages_t;
 
+typedef struct _notmuch_thread_node {
+    const char *thread_id;
+    struct _notmuch_thread_node *next;
+    struct _notmuch_thread_node *prev;
+} notmuch_thread_node_t;
+
+typedef struct _notmuch_thread_list {
+    notmuch_thread_node_t *head;
+    notmuch_thread_node_t *tail;
+    notmuch_thread_node_t *iterator;
+} notmuch_thread_list_t;
+
 struct _notmuch_threads {
     notmuch_query_t *query;
     GHashTable *threads;
     notmuch_messages_t *messages;
 
-    /* This thread ID is our iterator state. */
-    const char *thread_id;
+    /* thread list with the thread_id of the showed messages */
+    notmuch_thread_list_t *list;
 };
 
 notmuch_query_t *
@@ -269,6 +281,64 @@ _notmuch_mset_messages_move_to_previous (notmuch_messages_t *messages)
     }
 }
 
+static void
+_notmuch_thread_list_create (notmuch_thread_list_t *list, const char *thread_id)
+{
+    list->head = talloc (list, notmuch_thread_node_t);
+    list->tail = list->head;
+    list->iterator = list->head;
+    list->iterator->thread_id = thread_id;
+    list->iterator->next = NULL;
+    list->iterator->prev = NULL;
+}
+
+static void
+_notmuch_thread_list_append (notmuch_thread_list_t *list, const char *thread_id)
+{
+    list->tail->next = talloc (list, notmuch_thread_node_t);
+    list->iterator = list->tail->next;
+    list->iterator->thread_id = thread_id;
+    list->iterator->next = NULL;
+    list->iterator->prev = list->tail;
+    list->tail = list->iterator;
+}
+
+static const char *
+_notmuch_thread_list_get_id (notmuch_thread_list_t *list)
+{
+    return list->iterator->thread_id;
+}
+
+static notmuch_bool_t
+_notmuch_thread_list_valid (notmuch_thread_list_t *list)
+{
+    return (list->iterator != NULL);
+}
+
+static void
+_notmuch_thread_list_move_to_first (notmuch_thread_list_t *list)
+{
+    list->iterator = list->head;
+}
+
+static void
+_notmuch_thread_list_move_to_next (notmuch_thread_list_t *list)
+{
+    list->iterator = list->iterator->next;
+}
+
+static void
+_notmuch_thread_list_move_to_last (notmuch_thread_list_t *list)
+{
+    list->iterator = list->tail;
+}
+
+static void
+_notmuch_thread_list_move_to_previous (notmuch_thread_list_t *list)
+{
+    list->iterator = list->iterator->prev;
+}
+
 /* Glib objects force use to use a talloc destructor as well, (but not
  * nearly as ugly as the for messages due to C++ objects). At
  * this point, I'd really like to have some talloc-friendly
@@ -297,10 +367,13 @@ notmuch_query_search_threads (notmuch_query_t *query)
 
     threads->messages = notmuch_query_search_messages (query);
 
+    threads->list = talloc (threads, notmuch_thread_list_t);
     message = notmuch_messages_get (threads->messages);
-    threads->thread_id = notmuch_message_get_thread_id (message);
+    _notmuch_thread_list_create (threads->list,
+                                  notmuch_message_get_thread_id (message));
     g_hash_table_insert (threads->threads,
-                         xstrdup (threads->thread_id), NULL);
+                         xstrdup (_notmuch_thread_list_get_id (threads->list)),
+                         NULL);
 
     talloc_set_destructor (threads, _notmuch_threads_destructor);
 
@@ -316,7 +389,7 @@ notmuch_query_destroy (notmuch_query_t *query)
 notmuch_bool_t
 notmuch_threads_valid (notmuch_threads_t *threads)
 {
-    return (threads->thread_id != NULL);
+    return _notmuch_thread_list_valid (threads->list);
 }
 
 notmuch_thread_t *
@@ -327,37 +400,83 @@ notmuch_threads_get (notmuch_threads_t *threads)
 
     return _notmuch_thread_create (threads->query,
 				   threads->query->notmuch,
-				   threads->thread_id,
+				   _notmuch_thread_list_get_id (threads->list),
 				   threads->query->query_string);
 }
 
 void
+notmuch_threads_move_to_first (notmuch_threads_t *threads)
+{
+    _notmuch_thread_list_move_to_first (threads->list);
+}
+
+void
 notmuch_threads_move_to_next (notmuch_threads_t *threads)
 {
-    notmuch_message_t *message;
+    if (!_notmuch_thread_list_valid (threads->list))
+        return;
+
+    _notmuch_thread_list_move_to_next (threads->list);
+    if (_notmuch_thread_list_valid (threads->list))
+        return;
 
     while (notmuch_messages_valid (threads->messages))
     {
-	message = notmuch_messages_get (threads->messages);
+        notmuch_message_t *message;
+        const char *thread_id;
 
-	threads->thread_id = notmuch_message_get_thread_id (message);
+	message = notmuch_messages_get (threads->messages);
+	thread_id = notmuch_message_get_thread_id (message);
 
 	if (! g_hash_table_lookup_extended (threads->threads,
-					    threads->thread_id,
+					    thread_id,
 					    NULL, NULL))
 	{
 	    g_hash_table_insert (threads->threads,
-				 xstrdup (threads->thread_id), NULL);
+				 xstrdup (thread_id), NULL);
+            _notmuch_thread_list_append (threads->list, thread_id);
 	    notmuch_messages_move_to_next (threads->messages);
 	    return;
 	}
 
 	notmuch_messages_move_to_next (threads->messages);
     }
+}
+
+void
+notmuch_threads_move_to_last (notmuch_threads_t *threads)
+{
+    _notmuch_thread_list_move_to_last (threads->list);
+
+    while (notmuch_messages_valid (threads->messages))
+    {
+        notmuch_message_t *message;
+        const char *thread_id;
+
+	message = notmuch_messages_get (threads->messages);
+	thread_id = notmuch_message_get_thread_id (message);
+
+	if (! g_hash_table_lookup_extended (threads->threads,
+					    thread_id,
+					    NULL, NULL))
+	{
+	    g_hash_table_insert (threads->threads,
+				 xstrdup (thread_id), NULL);
+            _notmuch_thread_list_append (threads->list, thread_id);
+        }
 
-    threads->thread_id = NULL;
+	notmuch_messages_move_to_next (threads->messages);
+    }
 }
 
+void
+notmuch_threads_move_to_previous (notmuch_threads_t *threads)
+{
+    if (!_notmuch_thread_list_valid (threads->list))
+        return;
+
+    _notmuch_thread_list_move_to_previous (threads->list);
+}
 
 void
 notmuch_threads_destroy (notmuch_threads_t *threads)
-- 
1.7.0

[PATCH 5/5] Added backwards iterator to tags

[Ruben Pollan]

Added the functions notmuch_tags_move_to_prevoius,
notmuch_tags_move_to_last and  notmuch_tags_move_to_first to notmuch
library. With them is possible to iterate backwards on tags.

* notmuch_tags_move_to_prevoius do the opposite than
  notmuch_tags_move_to_next, getting the tags iterator one
  position backwards.

* notmuch_tags_move_to_last move the iterator to the first last tag.

* notmuch_tags_move_to_first move the iterator to the first valid tag.
---
 lib/notmuch.h |   28 ++++++++++++++++++++++++++++
 lib/tags.c    |   21 +++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index b96b624..dc668dc 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1022,6 +1022,15 @@ notmuch_tags_valid (notmuch_tags_t *tags);
 const char *
 notmuch_tags_get (notmuch_tags_t *tags);
 
+/* Move the 'tags' iterator to the first tag.
+ *
+ * After that the 'tags' iterator will be set to the first valid 
+ * tag, so it can be use to iterate with 
+ * notmuch_tags_move_to_next.
+ */
+void
+notmuch_tags_move_to_first (notmuch_tags_t *tags);
+
 /* Move the 'tags' iterator to the next tag.
  *
  * If 'tags' is already pointing at the last tag then the iterator
@@ -1035,6 +1044,25 @@ notmuch_tags_get (notmuch_tags_t *tags);
 void
 notmuch_tags_move_to_next (notmuch_tags_t *tags);
 
+/* Move the 'tags' iterator to the last tag.
+ *
+ * After that the 'tags' iterator will be set to the last valid 
+ * tag, so it can be use to iterate with 
+ * notmuch_tags_move_to_previous.
+ */
+void
+notmuch_tags_move_to_last (notmuch_tags_t *tags);
+
+/* Move the 'tags' iterator to the previous tag.
+ *
+ * If 'tags' is already pointing at the first tag then the
+ * iterator will be moved to a point just beyond that first tag,
+ * (where notmuch_tags_valid will return FALSE and
+ * notmuch_tags_get will return NULL).
+ */
+void
+notmuch_tags_move_to_previous (notmuch_tags_t *tags);
+
 /* Destroy a notmuch_tags_t object.
  *
  * It's not strictly necessary to call this function. All memory from
diff --git a/lib/tags.c b/lib/tags.c
index 8fe4a3f..9c9a897 100644
--- a/lib/tags.c
+++ b/lib/tags.c
@@ -105,6 +105,12 @@ notmuch_tags_get (notmuch_tags_t *tags)
 }
 
 void
+notmuch_tags_move_to_first (notmuch_tags_t *tags)
+{
+    tags->iterator = g_list_first (tags->tags);
+}
+
+void
 notmuch_tags_move_to_next (notmuch_tags_t *tags)
 {
     if (tags->iterator == NULL)
@@ -114,6 +120,21 @@ notmuch_tags_move_to_next (notmuch_tags_t *tags)
 }
 
 void
+notmuch_tags_move_to_last (notmuch_tags_t *tags)
+{
+    tags->iterator = g_list_last (tags->tags);
+}
+
+void
+notmuch_tags_move_to_previous (notmuch_tags_t *tags)
+{
+    if (tags->iterator == NULL)
+	return;
+
+    tags->iterator = tags->iterator->prev;
+}
+
+void
 notmuch_tags_destroy (notmuch_tags_t *tags)
 {
     talloc_free (tags);
-- 
1.7.0

Re: reverse iterators

[Sebastian Spaeth]

On Sat, 20 Mar 2010 11:23:20 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> 
> Adds support to reverse iteration on messages, threads and tags. To revew and
> think if makes sense to include them on notmuch or wait until they have a real
> use.

/me raises arm. Real use case here! Personally, I don't need the
backwards operator (and I don't see the case where I would need to go
back very often). But a _move_to_first() for threads, messages (and
probably tags), would be really useful to me now:

Having bound notmuch.so to python, I am in a situation, where I can only
offer iteration once, with subsequent iterations failing. This is very
unexpected and very un-pythonesque. I fell myself over it:

#the below len() iterates and exhausts msgs
if len(msgs) > 0:
   for msg in msgs: print msg
   #above line fails horribly already...
else: print "No message for me!"

So, having a way to reset the iterator would be a real boon already now.

Sebastian

[PATCH] sending again patches 3 & 4

[Ruben Pollan]

I send again the patches 3 and 4, beacuse I found a bug on them. The previous 
implementation did a segfault when the search didn't have results.

[PATCH 1/2] Move the logic of threads iterator out of 'valid'

[Ruben Pollan]

The logic of notmuch_threads_move_to_next iterator was on
notmuch_threads_valid function. Now notmuch_threads_valid just check if
the iterator is valid and is notmuch_threads_move_to_next wich actually
update the iterator.
---
 lib/query.cc |   54 ++++++++++++++++++++++++++++++------------------------
 1 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/lib/query.cc b/lib/query.cc
index 9266d35..514a156 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -233,6 +233,7 @@ notmuch_threads_t *
 notmuch_query_search_threads (notmuch_query_t *query)
 {
     notmuch_threads_t *threads;
+    notmuch_message_t *message;
 
     threads = talloc (query, notmuch_threads_t);
     if (threads == NULL)
@@ -243,8 +244,17 @@ notmuch_query_search_threads (notmuch_query_t *query)
 					      free, NULL);
 
     threads->messages = notmuch_query_search_messages (query);
+    if (!notmuch_messages_valid (threads->messages))
+    {
+        threads->thread_id = NULL;
+        return threads;
+    }
 
-    threads->thread_id = NULL;
+    message = notmuch_messages_get (threads->messages);
+    threads->thread_id = notmuch_message_get_thread_id (message);
+    g_hash_table_insert (threads->threads,
+                         xstrdup (threads->thread_id),
+                         NULL);
 
     talloc_set_destructor (threads, _notmuch_threads_destructor);
 
@@ -260,10 +270,25 @@ notmuch_query_destroy (notmuch_query_t *query)
 notmuch_bool_t
 notmuch_threads_valid (notmuch_threads_t *threads)
 {
-    notmuch_message_t *message;
+    return (threads->thread_id != NULL);
+}
+
+notmuch_thread_t *
+notmuch_threads_get (notmuch_threads_t *threads)
+{
+    if (! notmuch_threads_valid (threads))
+	return NULL;
+
+    return _notmuch_thread_create (threads->query,
+				   threads->query->notmuch,
+				   threads->thread_id,
+				   threads->query->query_string);
+}
 
-    if (threads->thread_id)
-	return TRUE;
+void
+notmuch_threads_move_to_next (notmuch_threads_t *threads)
+{
+    notmuch_message_t *message;
 
     while (notmuch_messages_valid (threads->messages))
     {
@@ -278,32 +303,13 @@ notmuch_threads_valid (notmuch_threads_t *threads)
 	    g_hash_table_insert (threads->threads,
 				 xstrdup (threads->thread_id), NULL);
 	    notmuch_messages_move_to_next (threads->messages);
-	    return TRUE;
+	    return;
 	}
 
 	notmuch_messages_move_to_next (threads->messages);
     }
 
     threads->thread_id = NULL;
-    return FALSE;
-}
-
-notmuch_thread_t *
-notmuch_threads_get (notmuch_threads_t *threads)
-{
-    if (! notmuch_threads_valid (threads))
-	return NULL;
-
-    return _notmuch_thread_create (threads->query,
-				   threads->query->notmuch,
-				   threads->thread_id,
-				   threads->query->query_string);
-}
-
-void
-notmuch_threads_move_to_next (notmuch_threads_t *threads)
-{
-    threads->thread_id = NULL;
 }
 
 void
-- 
1.7.0

[PATCH 2/2] Added backwards iterator to threads

[Ruben Pollan]

Added the functions notmuch_threads_move_to_prevoius,
notmuch_threads_move_to_last and  notmuch_threads_move_to_first to
notmuch library. With them is possible to iterate backwards on threads.

* notmuch_threads_move_to_prevoius do the opposite than
  notmuch_threads_move_to_next, getting the threads iterator one
  position backwards.

* notmuch_threads_move_to_last move the iterator to the first last
thread.

* notmuch_threads_move_to_first move the iterator to the first valid
  thread.

For it has been implemented notmuch_thread_list_t structur that stores
the thread_ids so the backwards iteration gets the thread_id in the same
order that was show on forward iteration.
---
 lib/notmuch.h |   28 ++++++++
 lib/query.cc  |  209 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 222 insertions(+), 15 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0d9cb0f..62f4ad4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -466,6 +466,15 @@ notmuch_threads_valid (notmuch_threads_t *threads);
 notmuch_thread_t *
 notmuch_threads_get (notmuch_threads_t *threads);
 
+/* Move the 'threads' iterator to the first thread.          
+ *
+ * After that the 'threads' iterator will be set to the first valid
+ * thread, so it can be use to iterate with
+ * notmuch_threads_move_to_next.
+ */
+void
+notmuch_threads_move_to_first (notmuch_threads_t *threads);
+
 /* Move the 'threads' iterator to the next thread.
  *
  * If 'threads' is already pointing at the last thread then the
@@ -479,6 +488,25 @@ notmuch_threads_get (notmuch_threads_t *threads);
 void
 notmuch_threads_move_to_next (notmuch_threads_t *threads);
 
+/* Move the 'threads' iterator to the last thread.
+ *
+ * After that the 'threads' iterator will be set to the last valid
+ * thread, so it can be use to iterate with
+ * notmuch_threads_move_to_previous.
+ */
+void
+notmuch_threads_move_to_last (notmuch_threads_t *threads);
+
+/* Move the 'threads' iterator to the previous thread.
+ *
+ * If 'threads' is already pointing at the first thread then the
+ * iterator will be moved to a point just beyond that first thread,
+ * (where notmuch_threads_valid will return FALSE and
+ * notmuch_threads_get will return NULL).
+ */
+void
+notmuch_threads_move_to_previous (notmuch_threads_t *threads);
+
 /* Destroy a notmuch_threads_t object.
  *
  * It's not strictly necessary to call this function. All memory from
diff --git a/lib/query.cc b/lib/query.cc
index 514a156..727f449 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -35,16 +35,29 @@ typedef struct _notmuch_mset_messages {
     notmuch_messages_t base;
     notmuch_database_t *notmuch;
     Xapian::MSetIterator iterator;
+    Xapian::MSetIterator iterator_begin;
     Xapian::MSetIterator iterator_end;
 } notmuch_mset_messages_t;
 
+typedef struct _notmuch_thread_node {
+    const char *thread_id;
+    struct _notmuch_thread_node *next;
+    struct _notmuch_thread_node *prev;
+} notmuch_thread_node_t;
+
+typedef struct _notmuch_thread_list {
+    notmuch_thread_node_t *head;
+    notmuch_thread_node_t *tail;
+    notmuch_thread_node_t *iterator;
+} notmuch_thread_list_t;
+
 struct _notmuch_threads {
     notmuch_query_t *query;
     GHashTable *threads;
     notmuch_messages_t *messages;
 
-    /* This thread ID is our iterator state. */
-    const char *thread_id;
+    /* thread list with the thread_id of the showed messages */
+    notmuch_thread_list_t *list;
 };
 
 notmuch_query_t *
@@ -86,6 +99,7 @@ static int
 _notmuch_messages_destructor (notmuch_mset_messages_t *messages)
 {
     messages->iterator.~MSetIterator ();
+    messages->iterator_begin.~MSetIterator ();
     messages->iterator_end.~MSetIterator ();
 
     return 0;
@@ -108,6 +122,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	messages->base.iterator = NULL;
 	messages->notmuch = notmuch;
 	new (&messages->iterator) Xapian::MSetIterator ();
+	new (&messages->iterator_begin) Xapian::MSetIterator ();
 	new (&messages->iterator_end) Xapian::MSetIterator ();
 
 	talloc_set_destructor (messages, _notmuch_messages_destructor);
@@ -157,6 +172,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	mset = enquire.get_mset (0, notmuch->xapian_db->get_doccount ());
 
 	messages->iterator = mset.begin ();
+	messages->iterator_begin = mset.begin ();
 	messages->iterator_end = mset.end ();
 
     } catch (const Xapian::Error &error) {
@@ -208,6 +224,16 @@ _notmuch_mset_messages_get (notmuch_messages_t *messages)
 }
 
 void
+_notmuch_mset_messages_move_to_first (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    mset_messages->iterator = mset_messages->iterator_begin;
+}
+
+void
 _notmuch_mset_messages_move_to_next (notmuch_messages_t *messages)
 {
     notmuch_mset_messages_t *mset_messages;
@@ -217,6 +243,113 @@ _notmuch_mset_messages_move_to_next (notmuch_messages_t *messages)
     mset_messages->iterator++;
 }
 
+void
+_notmuch_mset_messages_move_to_last (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    mset_messages->iterator = mset_messages->iterator_end;
+    mset_messages->iterator--;
+}
+
+void
+_notmuch_mset_messages_move_to_previous (notmuch_messages_t *messages)
+{
+    notmuch_mset_messages_t *mset_messages;
+
+    mset_messages = (notmuch_mset_messages_t *) messages;
+
+    if (mset_messages->iterator == mset_messages->iterator_begin)
+    {
+        /*
+         * Xapian iterators can not be beyond the first element, so we
+         * assign the iterator_end to mark the iterator as invalid in case
+         * of move_to_previous with the iterator at the beginning
+         */
+        mset_messages->iterator = mset_messages->iterator_end;
+    }
+    else if (_notmuch_mset_messages_valid (messages))
+    {
+        /*
+         * If is valid move the iterator. To emulate the same behavior 
+         * than notmuch_messages_t the iterator won't be updated if is 
+         * not valid
+         */
+        mset_messages->iterator--;
+    }
+}
+
+static notmuch_thread_list_t *
+_notmuch_thread_list_create (void *ctx)
+{
+    notmuch_thread_list_t *list;
+
+    list = talloc (ctx, notmuch_thread_list_t);
+    list->tail = NULL;
+    list->head = NULL;
+    list->iterator = NULL;
+
+    return list;
+}
+
+static void
+_notmuch_thread_list_append (notmuch_thread_list_t *list, const char *thread_id)
+{
+    list->iterator = talloc (list, notmuch_thread_node_t);
+    if (list->head == NULL)
+    {
+        list->head = list->iterator;
+        list->iterator->prev = NULL;
+    }
+    else
+    {
+        list->tail->next = list->iterator;
+        list->iterator->prev = list->tail;
+    }
+
+    list->iterator->thread_id = thread_id;
+    list->iterator->next = NULL;
+    list->tail = list->iterator;
+}
+
+static const char *
+_notmuch_thread_list_get_id (notmuch_thread_list_t *list)
+{
+    return list->iterator->thread_id;
+}
+
+static notmuch_bool_t
+_notmuch_thread_list_valid (notmuch_thread_list_t *list)
+{
+    return (list->iterator != NULL);
+}
+
+static void
+_notmuch_thread_list_move_to_first (notmuch_thread_list_t *list)
+{
+    list->iterator = list->head;
+}
+
+static void
+_notmuch_thread_list_move_to_next (notmuch_thread_list_t *list)
+{
+    list->iterator = list->iterator->next;
+}
+
+static void
+_notmuch_thread_list_move_to_last (notmuch_thread_list_t *list)
+{
+    list->iterator = list->tail;
+}
+
+static void
+_notmuch_thread_list_move_to_previous (notmuch_thread_list_t *list)
+{
+    list->iterator = list->iterator->prev;
+}
+
 /* Glib objects force use to use a talloc destructor as well, (but not
  * nearly as ugly as the for messages due to C++ objects). At
  * this point, I'd really like to have some talloc-friendly
@@ -244,16 +377,15 @@ notmuch_query_search_threads (notmuch_query_t *query)
 					      free, NULL);
 
     threads->messages = notmuch_query_search_messages (query);
+    threads->list = _notmuch_thread_list_create (threads);
     if (!notmuch_messages_valid (threads->messages))
-    {
-        threads->thread_id = NULL;
         return threads;
-    }
 
     message = notmuch_messages_get (threads->messages);
-    threads->thread_id = notmuch_message_get_thread_id (message);
+    _notmuch_thread_list_append (threads->list,
+                                 notmuch_message_get_thread_id (message));
     g_hash_table_insert (threads->threads,
-                         xstrdup (threads->thread_id),
+                         xstrdup (_notmuch_thread_list_get_id (threads->list)),
                          NULL);
 
     talloc_set_destructor (threads, _notmuch_threads_destructor);
@@ -270,7 +402,7 @@ notmuch_query_destroy (notmuch_query_t *query)
 notmuch_bool_t
 notmuch_threads_valid (notmuch_threads_t *threads)
 {
-    return (threads->thread_id != NULL);
+    return _notmuch_thread_list_valid (threads->list);
 }
 
 notmuch_thread_t *
@@ -281,35 +413,82 @@ notmuch_threads_get (notmuch_threads_t *threads)
 
     return _notmuch_thread_create (threads->query,
 				   threads->query->notmuch,
-				   threads->thread_id,
+				   _notmuch_thread_list_get_id (threads->list),
 				   threads->query->query_string);
 }
 
 void
+notmuch_threads_move_to_first (notmuch_threads_t *threads)
+{
+    _notmuch_thread_list_move_to_first (threads->list);
+}
+
+void
 notmuch_threads_move_to_next (notmuch_threads_t *threads)
 {
-    notmuch_message_t *message;
+    if (!_notmuch_thread_list_valid (threads->list))
+        return;
+
+    _notmuch_thread_list_move_to_next (threads->list);
+    if (_notmuch_thread_list_valid (threads->list))
+        return;
 
     while (notmuch_messages_valid (threads->messages))
     {
-	message = notmuch_messages_get (threads->messages);
+        notmuch_message_t *message;
+        const char *thread_id;
 
-	threads->thread_id = notmuch_message_get_thread_id (message);
+	message = notmuch_messages_get (threads->messages);
+	thread_id = notmuch_message_get_thread_id (message);
 
 	if (! g_hash_table_lookup_extended (threads->threads,
-					    threads->thread_id,
+					    thread_id,
 					    NULL, NULL))
 	{
 	    g_hash_table_insert (threads->threads,
-				 xstrdup (threads->thread_id), NULL);
+				 xstrdup (thread_id), NULL);
+            _notmuch_thread_list_append (threads->list, thread_id);
 	    notmuch_messages_move_to_next (threads->messages);
 	    return;
 	}
 
 	notmuch_messages_move_to_next (threads->messages);
     }
+}
+
+void
+notmuch_threads_move_to_last (notmuch_threads_t *threads)
+{
+    _notmuch_thread_list_move_to_last (threads->list);
+
+    while (notmuch_messages_valid (threads->messages))
+    {
+        notmuch_message_t *message;
+        const char *thread_id;
+
+	message = notmuch_messages_get (threads->messages);
+	thread_id = notmuch_message_get_thread_id (message);
+
+	if (! g_hash_table_lookup_extended (threads->threads,
+					    thread_id,
+					    NULL, NULL))
+	{
+	    g_hash_table_insert (threads->threads,
+				 xstrdup (thread_id), NULL);
+            _notmuch_thread_list_append (threads->list, thread_id);
+        }
+
+	notmuch_messages_move_to_next (threads->messages);
+    }
+}
+
+void
+notmuch_threads_move_to_previous (notmuch_threads_t *threads)
+{
+    if (!_notmuch_thread_list_valid (threads->list))
+        return;
 
-    threads->thread_id = NULL;
+    _notmuch_thread_list_move_to_previous (threads->list);
 }
 
 void
-- 
1.7.0

Re: reverse iterators

[Ruben Pollan]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

On 21:07, Sun 21 Mar 10, Sebastian Spaeth wrote:
> On Sat, 20 Mar 2010 11:23:20 +0100, Ruben Pollan <meskio@sindominio.net> wrote:
> > 
> > Adds support to reverse iteration on messages, threads and tags. To revew and
> > think if makes sense to include them on notmuch or wait until they have a real
> > use.
> 
> /me raises arm. Real use case here! Personally, I don't need the
> backwards operator (and I don't see the case where I would need to go
> back very often).

For me is pretty useful because I use the iterator as a cursor on an ncurses
notmuch interface. So the selected thread is where the iterator is and the user
can to go forward or backwards.

> But a _move_to_first() for threads, messages (and
> probably tags), would be really useful to me now:

:) Happy to see there is someone else needing it.


-- 
Rubén Pollán  | jabber:meskio@jabber.org
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Lo hago para no volverme loco cuando noto
que solo me queda un demonio en un hombro
por que se ha cortado las venas
el ángel que había en el otro.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]