* [v2 0/3] Avoid empty thread names if possible
@ 2014-10-29 20:51 Jesse Rosenthal
2014-10-29 20:51 ` [v2 1/3] thread.cc: " Jesse Rosenthal
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jesse Rosenthal @ 2014-10-29 20:51 UTC (permalink / raw)
To: notmuch
This is the second go at avoiding empty thread names. The differences
from the first version are:
* Compare empty strings by checking for '\0' in first character
(using a macro) as suggested by Tomi Ollila.
* Make sure that threads are titled correctly regardless of sorting
order. In this version we only add change the subject in
_thread_set_subject_from_message if the subject (after cleaning
"Re:") is non-empty. This is necessary for
oldest-first. newest-first works the same as before.
* Add tests. This means that we have to force the test suite to
accept a non-empty header. I called the dummy subject
`@FORCE_EMPTY` to differentiate from a normal string, but not
invoke any special shell-ness.
Jesse Rosenthal (3):
thread.cc: Avoid empty thread names if possible.
test-lib: Add dummy subject to force empty subject
thread-naming test: Test empty subject names.
lib/thread.cc | 16 +++++++++++-----
test/T200-thread-naming.sh | 32 ++++++++++++++++++++++++++++++++
test/test-lib.sh | 2 ++
3 files changed, 45 insertions(+), 5 deletions(-)
--
2.1.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [v2 1/3] thread.cc: Avoid empty thread names if possible.
2014-10-29 20:51 [v2 0/3] Avoid empty thread names if possible Jesse Rosenthal
@ 2014-10-29 20:51 ` Jesse Rosenthal
2015-01-10 9:18 ` David Bremner
2014-10-29 20:51 ` [v2 2/3] test-lib: Add dummy subject to force empty subject Jesse Rosenthal
2014-10-29 20:51 ` [v2 3/3] thread-naming test: Test empty subject names Jesse Rosenthal
2 siblings, 1 reply; 7+ messages in thread
From: Jesse Rosenthal @ 2014-10-29 20:51 UTC (permalink / raw)
To: notmuch
Currently the thread is named based on either the oldest or newest
matching message (depending on the search order). If this message has
an empty subject, though, the thread will show up with an empty
subject in the search results. (See the thread starting with
`id:1412371140-21051-1-git-send-email-david@tethera.net` for an
example.)
This changes the behavior so it will use a non-empty name for the
thread if possible. We name threads based on (a) non-empty matches for
the query, and (b) the search order. If the search order is
oldest-first (as in the default inbox) it chooses the oldest matching
non-empty message as the subject. If the search order is newest-first
it chooses the newest one.
---
lib/thread.cc | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/lib/thread.cc b/lib/thread.cc
index 8922403..267f351 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -24,6 +24,8 @@
#include <gmime/gmime.h>
#include <glib.h> /* GHashTable */
+#define EMPTY_STRING(s) ((s)[0] == '\0')
+
struct visible _notmuch_thread {
notmuch_database_t *notmuch;
char *thread_id;
@@ -330,11 +332,13 @@ _thread_set_subject_from_message (notmuch_thread_t *thread,
} else {
cleaned_subject = talloc_strdup (thread, subject);
}
+
+ if (! EMPTY_STRING(cleaned_subject)) {
+ if (thread->subject)
+ talloc_free (thread->subject);
- if (thread->subject)
- talloc_free (thread->subject);
-
- thread->subject = talloc_strdup (thread, cleaned_subject);
+ thread->subject = talloc_strdup (thread, cleaned_subject);
+ }
}
/* Add a message to this thread which is known to match the original
@@ -348,8 +352,10 @@ _thread_add_matched_message (notmuch_thread_t *thread,
{
time_t date;
notmuch_message_t *hashed_message;
+ const char *cur_subject;
date = notmuch_message_get_date (message);
+ cur_subject = notmuch_thread_get_subject(thread);
if (date < thread->oldest || ! thread->matched_messages) {
thread->oldest = date;
@@ -359,7 +365,7 @@ _thread_add_matched_message (notmuch_thread_t *thread,
if (date > thread->newest || ! thread->matched_messages) {
thread->newest = date;
- if (sort != NOTMUCH_SORT_OLDEST_FIRST)
+ if (sort != NOTMUCH_SORT_OLDEST_FIRST || EMPTY_STRING(cur_subject))
_thread_set_subject_from_message (thread, message);
}
--
2.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [v2 2/3] test-lib: Add dummy subject to force empty subject
2014-10-29 20:51 [v2 0/3] Avoid empty thread names if possible Jesse Rosenthal
2014-10-29 20:51 ` [v2 1/3] thread.cc: " Jesse Rosenthal
@ 2014-10-29 20:51 ` Jesse Rosenthal
2014-10-29 20:51 ` [v2 3/3] thread-naming test: Test empty subject names Jesse Rosenthal
2 siblings, 0 replies; 7+ messages in thread
From: Jesse Rosenthal @ 2014-10-29 20:51 UTC (permalink / raw)
To: notmuch
At the moment, the test-lib fills in any missing headers. This makes
it impossible to test our handling of empty subjects. This will
allow us to use a special dummy subject -- `@FORCE_EMPTY` -- to force
the subject to remain empty.
---
test/test-lib.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 53db9ca..6057238 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -368,6 +368,8 @@ generate_message ()
else
template[subject]="Test message #${gen_msg_cnt}"
fi
+ elif [ "${template[subject]}" = "@FORCE_EMPTY" ]; then
+ template[subject]=""
fi
if [ -z "${template[date]}" ]; then
--
2.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [v2 3/3] thread-naming test: Test empty subject names.
2014-10-29 20:51 [v2 0/3] Avoid empty thread names if possible Jesse Rosenthal
2014-10-29 20:51 ` [v2 1/3] thread.cc: " Jesse Rosenthal
2014-10-29 20:51 ` [v2 2/3] test-lib: Add dummy subject to force empty subject Jesse Rosenthal
@ 2014-10-29 20:51 ` Jesse Rosenthal
2015-01-10 9:28 ` David Bremner
2 siblings, 1 reply; 7+ messages in thread
From: Jesse Rosenthal @ 2014-10-29 20:51 UTC (permalink / raw)
To: notmuch
We test all empty subjects, and then empty subjects followed by
non-empty subjects (searching both oldest- and newest-first).
---
test/T200-thread-naming.sh | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/test/T200-thread-naming.sh b/test/T200-thread-naming.sh
index 1a1a48f..dcfc1b3 100755
--- a/test/T200-thread-naming.sh
+++ b/test/T200-thread-naming.sh
@@ -63,6 +63,38 @@ add_message '[subject]="Sv: thread-naming: Initial thread subject"' \
output=$(notmuch search --sort=newest-first thread-naming and tag:inbox | notmuch_search_sanitize)
test_expect_equal "$output" "thread:XXX 2001-01-12 [6/8] Notmuch Test Suite; thread-naming: Initial thread subject (inbox unread)"
+test_begin_subtest "Use empty subjects if necessary."
+add_message '[subject]="@FORCE_EMPTY"' \
+ '[date]="Sat, 13 Jan 2001 15:43:45 -0000"' \
+ '[from]="Empty Sender \<empty_test@notmuchmail.org\>"'
+empty_parent=${gen_msg_id}
+add_message '[subject]="@FORCE_EMPTY"' \
+ '[date]="Sun, 14 Jan 2001 15:43:45 -0000"' \
+ '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+ "[in-reply-to]=\<$empty_parent\>"
+output=$(notmuch search --sort=newest-first from:empty_test@notmuchmail.org | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX 2001-01-14 [2/2] Empty Sender; (inbox unread)"
+
+test_begin_subtest "Avoid empty subjects if possible (newest-first)."
+add_message '[subject]="Non-empty subject (1)"' \
+ '[date]="Mon, 15 Jan 2001 15:43:45 -0000"' \
+ '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+ "[in-reply-to]=\<$empty_parent\>"
+add_message '[subject]="Non-empty subject (2)"' \
+ '[date]="Mon, 16 Jan 2001 15:43:45 -0000"' \
+ '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+ "[in-reply-to]=\<$empty_parent\>"
+add_message '[subject]="@FORCE_EMPTY"' \
+ '[date]="Tue, 17 Jan 2001 15:43:45 -0000"' \
+ '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+ "[in-reply-to]=\<$empty_parent\>"
+output=$(notmuch search --sort=newest-first from:Empty | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX 2001-01-17 [5/5] Empty Sender; Non-empty subject (2) (inbox unread)"
+
+test_begin_subtest "Avoid empty subjects if possible (oldest-first)."
+output=$(notmuch search --sort=oldest-first from:Empty | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX 2001-01-13 [5/5] Empty Sender; Non-empty subject (1) (inbox unread)"
+
test_begin_subtest 'Test order of messages in "notmuch show"'
output=$(notmuch show thread-naming | notmuch_show_sanitize)
test_expect_equal "$output" "\fmessage{ id:msg-$(printf "%03d" $first)@notmuch-test-suite depth:0 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $first)
--
2.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [v2 1/3] thread.cc: Avoid empty thread names if possible.
2014-10-29 20:51 ` [v2 1/3] thread.cc: " Jesse Rosenthal
@ 2015-01-10 9:18 ` David Bremner
0 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2015-01-10 9:18 UTC (permalink / raw)
To: Jesse Rosenthal, notmuch
Jesse Rosenthal <jrosenthal@jhu.edu> writes:
>
> +#define EMPTY_STRING(s) ((s)[0] == '\0')
> +
Extra whitespace at end of line. If that's the only issue I can rebase
it away, but maybe you want to enable the standard pre-commit hook?
> struct visible _notmuch_thread {
> notmuch_database_t *notmuch;
> char *thread_id;
> @@ -330,11 +332,13 @@ _thread_set_subject_from_message (notmuch_thread_t *thread,
> } else {
> cleaned_subject = talloc_strdup (thread, subject);
> }
> +
Same here.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2 3/3] thread-naming test: Test empty subject names.
2014-10-29 20:51 ` [v2 3/3] thread-naming test: Test empty subject names Jesse Rosenthal
@ 2015-01-10 9:28 ` David Bremner
2015-01-17 12:57 ` David Bremner
0 siblings, 1 reply; 7+ messages in thread
From: David Bremner @ 2015-01-10 9:28 UTC (permalink / raw)
To: Jesse Rosenthal, notmuch
Jesse Rosenthal <jrosenthal@jhu.edu> writes:
>
> +test_begin_subtest "Use empty subjects if necessary."
> +add_message '[subject]="@FORCE_EMPTY"' \
> + '[date]="Sat, 13 Jan 2001 15:43:45 -0000"' \
> + '[from]="Empty Sender \<empty_test@notmuchmail.org\
It seems to me we should avoid notmuchmail.org addresses in the test
suite. Maybe this is too picky, but if you are going to fixup the white
space errors, how about also changing to example.{org,net,com}
addresses.
Cheers,
d
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2 3/3] thread-naming test: Test empty subject names.
2015-01-10 9:28 ` David Bremner
@ 2015-01-17 12:57 ` David Bremner
0 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2015-01-17 12:57 UTC (permalink / raw)
To: Jesse Rosenthal, notmuch
David Bremner <david@tethera.net> writes:
> Jesse Rosenthal <jrosenthal@jhu.edu> writes:
>
>>
>> +test_begin_subtest "Use empty subjects if necessary."
>> +add_message '[subject]="@FORCE_EMPTY"' \
>> + '[date]="Sat, 13 Jan 2001 15:43:45 -0000"' \
>> + '[from]="Empty Sender \<empty_test@notmuchmail.org\
>
> It seems to me we should avoid notmuchmail.org addresses in the test
> suite. Maybe this is too picky, but if you are going to fixup the white
> space errors, how about also changing to example.{org,net,com}
> addresses.
I realized we use such addresses everywhere, so this is at least
consistent with the existing code. I pushed the series with the
whitespace fixups.
d
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-17 12:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-29 20:51 [v2 0/3] Avoid empty thread names if possible Jesse Rosenthal
2014-10-29 20:51 ` [v2 1/3] thread.cc: " Jesse Rosenthal
2015-01-10 9:18 ` David Bremner
2014-10-29 20:51 ` [v2 2/3] test-lib: Add dummy subject to force empty subject Jesse Rosenthal
2014-10-29 20:51 ` [v2 3/3] thread-naming test: Test empty subject names Jesse Rosenthal
2015-01-10 9:28 ` David Bremner
2015-01-17 12:57 ` David Bremner
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).