[PATCH] Reordering of thread authors to list matching authors first
On Fri, 23 Apr 2010 17:21:53 -0700, Carl Worth wrote:
> On Wed, 21 Apr 2010 20:58:27 -0700, Dirk Hohndel
> wrote:
> > When displaying threads as result of a search it makes sense to list those
> > authors first who match the search. The matching authors are separated from
> > the
> > non-matching ones with a '|' instead of a ','
>
> It seems a reasonable feature to me.
I switched back to origin/master to help get ready for 0.3 and
tremendously miss this feature :-)
> Some notes on the patch:
>
> > +void
> > +notmuch_message_set_author (notmuch_message_t *message,
> > + const char *author)
> > +{
> > +message->author = talloc_strdup(message, author);
> > +return;
> > +}
>
> This is leaking any previously set author value, (admittedly, it's only
> a "talloc leak" so it will still get cleaned up when the message is
> cleaned up, but still.
Fixed in forthcoming revision of this patch
>
> > +/* Set the author member of 'message' - this is the representation used
> > + * when displaying the message
> > + */
> > +void
> > +notmuch_message_set_author (notmuch_message_t *message, const char
> > *author);
> > +
> > +/* Get the author member of 'message'
> > + */
> > +const char *
> > +notmuch_message_get_author (notmuch_message_t *message);
>
> The notmuch.h file is our publicly installed header file for the library
> interface. I don't think the feature here requires any new library
> interface. Even if it did, we wouldn't want a public function like
> set_author that could simply scramble internal state and change the
> result of future calls to get_author.
My mistake - moved them to notmuch-private.h
> > +/*
> > + * move authors of matched messages in the thread to
> > + * the front of the authors list, but keep them in
> > + * existing order within their group
> > + */
> > +static void
> > +_thread_move_matched_author (notmuch_thread_t *thread,
> > +const char *author)
>
> The implementation here seems a bit fiddly.
>
> We already have two passes over the messages, (first all messages, and
> then all matched messages). And we're currently calling add_author from
> the first pass.
>
> How about simply calling a new add_matched_author from the second pass
> which would look very much like the existing add_author. Then we could
> change add_author to accumulate authors into an array rather than a
> string. Then, finally, we would append any of these authors not already
> in the matched_authors hash tabled onto the final string.
>
> That should be less code and easier to understand I think.
>
> I can take a whack at that later if you don't beat me to it.
Maybe I'm misunderstanding your proposed algorithm - but it seems quite
a bit more complicated than what I'm doing today...
/D
Re: [PATCH] Reordering of thread authors to list matching authors first
On Fri, 23 Apr 2010 17:21:53 -0700, Carl Worth cwo...@cworth.org wrote:
On Wed, 21 Apr 2010 20:58:27 -0700, Dirk Hohndel hohn...@infradead.org
wrote:
When displaying threads as result of a search it makes sense to list those
authors first who match the search. The matching authors are separated from
the
non-matching ones with a '|' instead of a ','
It seems a reasonable feature to me.
I switched back to origin/master to help get ready for 0.3 and
tremendously miss this feature :-)
Some notes on the patch:
+void
+notmuch_message_set_author (notmuch_message_t *message,
+ const char *author)
+{
+message-author = talloc_strdup(message, author);
+return;
+}
This is leaking any previously set author value, (admittedly, it's only
a talloc leak so it will still get cleaned up when the message is
cleaned up, but still.
Fixed in forthcoming revision of this patch
+/* Set the author member of 'message' - this is the representation used
+ * when displaying the message
+ */
+void
+notmuch_message_set_author (notmuch_message_t *message, const char
*author);
+
+/* Get the author member of 'message'
+ */
+const char *
+notmuch_message_get_author (notmuch_message_t *message);
The notmuch.h file is our publicly installed header file for the library
interface. I don't think the feature here requires any new library
interface. Even if it did, we wouldn't want a public function like
set_author that could simply scramble internal state and change the
result of future calls to get_author.
My mistake - moved them to notmuch-private.h
+/*
+ * move authors of matched messages in the thread to
+ * the front of the authors list, but keep them in
+ * existing order within their group
+ */
+static void
+_thread_move_matched_author (notmuch_thread_t *thread,
+const char *author)
The implementation here seems a bit fiddly.
We already have two passes over the messages, (first all messages, and
then all matched messages). And we're currently calling add_author from
the first pass.
How about simply calling a new add_matched_author from the second pass
which would look very much like the existing add_author. Then we could
change add_author to accumulate authors into an array rather than a
string. Then, finally, we would append any of these authors not already
in the matched_authors hash tabled onto the final string.
That should be less code and easier to understand I think.
I can take a whack at that later if you don't beat me to it.
Maybe I'm misunderstanding your proposed algorithm - but it seems quite
a bit more complicated than what I'm doing today...
/D
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Reordering of thread authors to list matching authors first
On Wed, 21 Apr 2010 20:58:27 -0700, Dirk Hohndel
wrote:
> When displaying threads as result of a search it makes sense to list those
> authors first who match the search. The matching authors are separated from
> the
> non-matching ones with a '|' instead of a ','
It seems a reasonable feature to me.
Some notes on the patch:
> +void
> +notmuch_message_set_author (notmuch_message_t *message,
> + const char *author)
> +{
> +message->author = talloc_strdup(message, author);
> +return;
> +}
This is leaking any previously set author value, (admittedly, it's only
a "talloc leak" so it will still get cleaned up when the message is
cleaned up, but still.
> +/* Set the author member of 'message' - this is the representation used
> + * when displaying the message
> + */
> +void
> +notmuch_message_set_author (notmuch_message_t *message, const char *author);
> +
> +/* Get the author member of 'message'
> + */
> +const char *
> +notmuch_message_get_author (notmuch_message_t *message);
The notmuch.h file is our publicly installed header file for the library
interface. I don't think the feature here requires any new library
interface. Even if it did, we wouldn't want a public function like
set_author that could simply scramble internal state and change the
result of future calls to get_author.
> +/*
> + * move authors of matched messages in the thread to
> + * the front of the authors list, but keep them in
> + * existing order within their group
> + */
> +static void
> +_thread_move_matched_author (notmuch_thread_t *thread,
> + const char *author)
The implementation here seems a bit fiddly.
We already have two passes over the messages, (first all messages, and
then all matched messages). And we're currently calling add_author from
the first pass.
How about simply calling a new add_matched_author from the second pass
which would look very much like the existing add_author. Then we could
change add_author to accumulate authors into an array rather than a
string. Then, finally, we would append any of these authors not already
in the matched_authors hash tabled onto the final string.
That should be less code and easier to understand I think.
I can take a whack at that later if you don't beat me to it.
-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL:
Re: [PATCH] Reordering of thread authors to list matching authors first
On Wed, 21 Apr 2010 20:58:27 -0700, Dirk Hohndel hohn...@infradead.org wrote:
When displaying threads as result of a search it makes sense to list those
authors first who match the search. The matching authors are separated from
the
non-matching ones with a '|' instead of a ','
It seems a reasonable feature to me.
Some notes on the patch:
+void
+notmuch_message_set_author (notmuch_message_t *message,
+ const char *author)
+{
+message-author = talloc_strdup(message, author);
+return;
+}
This is leaking any previously set author value, (admittedly, it's only
a talloc leak so it will still get cleaned up when the message is
cleaned up, but still.
+/* Set the author member of 'message' - this is the representation used
+ * when displaying the message
+ */
+void
+notmuch_message_set_author (notmuch_message_t *message, const char *author);
+
+/* Get the author member of 'message'
+ */
+const char *
+notmuch_message_get_author (notmuch_message_t *message);
The notmuch.h file is our publicly installed header file for the library
interface. I don't think the feature here requires any new library
interface. Even if it did, we wouldn't want a public function like
set_author that could simply scramble internal state and change the
result of future calls to get_author.
+/*
+ * move authors of matched messages in the thread to
+ * the front of the authors list, but keep them in
+ * existing order within their group
+ */
+static void
+_thread_move_matched_author (notmuch_thread_t *thread,
+ const char *author)
The implementation here seems a bit fiddly.
We already have two passes over the messages, (first all messages, and
then all matched messages). And we're currently calling add_author from
the first pass.
How about simply calling a new add_matched_author from the second pass
which would look very much like the existing add_author. Then we could
change add_author to accumulate authors into an array rather than a
string. Then, finally, we would append any of these authors not already
in the matched_authors hash tabled onto the final string.
That should be less code and easier to understand I think.
I can take a whack at that later if you don't beat me to it.
-Carl
pgpeyTZQB4v2r.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Reordering of thread authors to list matching authors first
When displaying threads as result of a search it makes sense to list those
authors first who match the search. The matching authors are separated from the
non-matching ones with a '|' instead of a ','
Imagine the default "+inbox" query. Those mails in the thread that
match the query are actually "new" (whatever that means). And some
people seem to think that it would be much better to see those author
names first. For example, imagine a long and drawn out thread that once
was started by me; you have long read the older part of the thread and
removed the inbox tag. Whenever a new email comes in on this thread,
prior to this patch the author column in the search display will first show
"Dirk Hohndel" - I think it should first show the actual author(s) of the new
mail(s).
Signed-off-by: Dirk Hohndel
---
lib/message.cc | 16
lib/notmuch.h | 11
lib/thread.cc | 74
3 files changed, 101 insertions(+), 0 deletions(-)
diff --git a/lib/message.cc b/lib/message.cc
index 721c9a6..ac0afd9 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -35,6 +35,7 @@ struct _notmuch_message {
char *thread_id;
char *in_reply_to;
char *filename;
+char *author;
notmuch_message_file_t *message_file;
notmuch_message_list_t *replies;
unsigned long flags;
@@ -110,6 +111,7 @@ _notmuch_message_create (const void *talloc_owner,
message->in_reply_to = NULL;
message->filename = NULL;
message->message_file = NULL;
+message->author = NULL;
message->replies = _notmuch_message_list_create (message);
if (unlikely (message->replies == NULL)) {
@@ -533,6 +535,20 @@ notmuch_message_get_tags (notmuch_message_t *message)
return _notmuch_convert_tags(message, i, end);
}
+const char *
+notmuch_message_get_author (notmuch_message_t *message)
+{
+return message->author;
+}
+
+void
+notmuch_message_set_author (notmuch_message_t *message,
+ const char *author)
+{
+message->author = talloc_strdup(message, author);
+return;
+}
+
void
_notmuch_message_set_date (notmuch_message_t *message,
const char *date)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index bae48a6..769f747 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -773,6 +773,17 @@ notmuch_message_set_flag (notmuch_message_t *message,
time_t
notmuch_message_get_date (notmuch_message_t *message);
+/* Set the author member of 'message' - this is the representation used
+ * when displaying the message
+ */
+void
+notmuch_message_set_author (notmuch_message_t *message, const char *author);
+
+/* Get the author member of 'message'
+ */
+const char *
+notmuch_message_get_author (notmuch_message_t *message);
+
/* Get the value of the specified header from 'message'.
*
* The value will be read from the actual message file, not from the
diff --git a/lib/thread.cc b/lib/thread.cc
index e514bf8..baa0d7f 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -32,6 +32,7 @@ struct _notmuch_thread {
char *subject;
GHashTable *authors_hash;
char *authors;
+char *nonmatched_authors;
GHashTable *tags;
notmuch_message_list_t *message_list;
@@ -73,6 +74,76 @@ _thread_add_author (notmuch_thread_t *thread,
thread->authors = talloc_strdup (thread, author);
}
+/*
+ * move authors of matched messages in the thread to
+ * the front of the authors list, but keep them in
+ * existing order within their group
+ */
+static void
+_thread_move_matched_author (notmuch_thread_t *thread,
+const char *author)
+{
+char *authorscopy;
+char *currentauthor;
+char *lastpipe,*nextpipe;
+int idx,nmstart,author_len,authors_len;
+
+if (thread->authors == NULL || author == NULL)
+ return;
+if (thread->nonmatched_authors == NULL)
+ thread->nonmatched_authors = thread->authors;
+author_len = strlen(author);
+authors_len = strlen(thread->authors);
+if (author_len == authors_len) {
+ /* just one author */
+ thread->nonmatched_authors += author_len;
+ return;
+}
+currentauthor = strcasestr(thread->authors, author);
+if (currentauthor == NULL)
+ return;
+/* how far inside the nonmatched authors is our author? */
+idx = currentauthor - thread->nonmatched_authors;
+if (idx < 0) {
+ /* already among matched authors */
+ return;
+}
+/* are there any authors in the list after our author? */
+if (thread->nonmatched_authors + author_len < thread->authors +
authors_len) {
+ /* we have to make changes, so let's get a temp copy */
+ authorscopy = xstrdup(thread->authors);
+ /* nmstart is the offset into where the non-matched authors start */
+ nmstart = thread->nonmatched_authors - thread->authors;
+ /* copy this author and add the "| " - the if clause above tells us
there's more */
+
[PATCH] Reordering of thread authors to list matching authors first
When displaying threads as result of a search it makes sense to list those
authors first who match the search. The matching authors are separated from the
non-matching ones with a '|' instead of a ','
Imagine the default +inbox query. Those mails in the thread that
match the query are actually new (whatever that means). And some
people seem to think that it would be much better to see those author
names first. For example, imagine a long and drawn out thread that once
was started by me; you have long read the older part of the thread and
removed the inbox tag. Whenever a new email comes in on this thread,
prior to this patch the author column in the search display will first show
Dirk Hohndel - I think it should first show the actual author(s) of the new
mail(s).
Signed-off-by: Dirk Hohndel hohn...@infradead.org
---
lib/message.cc | 16
lib/notmuch.h | 11
lib/thread.cc | 74
3 files changed, 101 insertions(+), 0 deletions(-)
diff --git a/lib/message.cc b/lib/message.cc
index 721c9a6..ac0afd9 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -35,6 +35,7 @@ struct _notmuch_message {
char *thread_id;
char *in_reply_to;
char *filename;
+char *author;
notmuch_message_file_t *message_file;
notmuch_message_list_t *replies;
unsigned long flags;
@@ -110,6 +111,7 @@ _notmuch_message_create (const void *talloc_owner,
message-in_reply_to = NULL;
message-filename = NULL;
message-message_file = NULL;
+message-author = NULL;
message-replies = _notmuch_message_list_create (message);
if (unlikely (message-replies == NULL)) {
@@ -533,6 +535,20 @@ notmuch_message_get_tags (notmuch_message_t *message)
return _notmuch_convert_tags(message, i, end);
}
+const char *
+notmuch_message_get_author (notmuch_message_t *message)
+{
+return message-author;
+}
+
+void
+notmuch_message_set_author (notmuch_message_t *message,
+ const char *author)
+{
+message-author = talloc_strdup(message, author);
+return;
+}
+
void
_notmuch_message_set_date (notmuch_message_t *message,
const char *date)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index bae48a6..769f747 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -773,6 +773,17 @@ notmuch_message_set_flag (notmuch_message_t *message,
time_t
notmuch_message_get_date (notmuch_message_t *message);
+/* Set the author member of 'message' - this is the representation used
+ * when displaying the message
+ */
+void
+notmuch_message_set_author (notmuch_message_t *message, const char *author);
+
+/* Get the author member of 'message'
+ */
+const char *
+notmuch_message_get_author (notmuch_message_t *message);
+
/* Get the value of the specified header from 'message'.
*
* The value will be read from the actual message file, not from the
diff --git a/lib/thread.cc b/lib/thread.cc
index e514bf8..baa0d7f 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -32,6 +32,7 @@ struct _notmuch_thread {
char *subject;
GHashTable *authors_hash;
char *authors;
+char *nonmatched_authors;
GHashTable *tags;
notmuch_message_list_t *message_list;
@@ -73,6 +74,76 @@ _thread_add_author (notmuch_thread_t *thread,
thread-authors = talloc_strdup (thread, author);
}
+/*
+ * move authors of matched messages in the thread to
+ * the front of the authors list, but keep them in
+ * existing order within their group
+ */
+static void
+_thread_move_matched_author (notmuch_thread_t *thread,
+const char *author)
+{
+char *authorscopy;
+char *currentauthor;
+char *lastpipe,*nextpipe;
+int idx,nmstart,author_len,authors_len;
+
+if (thread-authors == NULL || author == NULL)
+ return;
+if (thread-nonmatched_authors == NULL)
+ thread-nonmatched_authors = thread-authors;
+author_len = strlen(author);
+authors_len = strlen(thread-authors);
+if (author_len == authors_len) {
+ /* just one author */
+ thread-nonmatched_authors += author_len;
+ return;
+}
+currentauthor = strcasestr(thread-authors, author);
+if (currentauthor == NULL)
+ return;
+/* how far inside the nonmatched authors is our author? */
+idx = currentauthor - thread-nonmatched_authors;
+if (idx 0) {
+ /* already among matched authors */
+ return;
+}
+/* are there any authors in the list after our author? */
+if (thread-nonmatched_authors + author_len thread-authors +
authors_len) {
+ /* we have to make changes, so let's get a temp copy */
+ authorscopy = xstrdup(thread-authors);
+ /* nmstart is the offset into where the non-matched authors start */
+ nmstart = thread-nonmatched_authors - thread-authors;
+ /* copy this author and add the | - the if clause above tells us
there's more */
+
