[PATCH v2 08/10] cli: address: Do not output duplicate addresses
On Tue, Nov 04 2014, David Bremner wrote:
> Michal Sojka writes:
>
>>
>> +/* Returns TRUE iff name and addr is duplicate. */
>
> If you're revising this patch, it would be good to mention the side
> effect of this function.
>
>> -process_address_list (const search_context_t *ctx, InternetAddressList
>> *list)
>> +process_address_list (const search_context_t *ctx,
>> + InternetAddressList *list)
>
> It probably doesn't make any difference, but this looks like a needless
> whitespace change.
>
> This function definitely needs some comment / pointer to
> documention. And probably not to have _my in the name.
>
>> +static void
>> +_my_talloc_free_for_g_hash (void *ptr)
>> +{
>> +talloc_free (ptr);
>> +}
>> +
>
> I don't understand the name of the next subtest
>
>> +test_begin_subtest "No --output"
>> +notmuch address --output=sender --output=recipients '*' >OUTPUT
This should be "notmuch address '*' >OUTPUT". I'll fix that.
>> +# Use EXPECTED from previous subtest
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>> +
>> +test_done
>
> nitpick, extra blank lines
>
> So, AIUI, this is all of the series proposed for 0.19.
Agreed.
> It looks close to OK to me, modulo some minor style nits. One
> anonymous commentator on IRC mentioned the use of module scope
> variables, I guess in patch 6/10. I'm not sure of a better solution,
> but it's true in a perfect world we wouldn't have module local state.
A possible solution would be fill in common_options structure
programmatically, but this would make the code much less readable. I can
think of a few other solutions but none of them would fit into "perfect
world" :)
I'll send updated patches in the evening (CET timezone).
Thanks
-Michal
[PATCH v2 08/10] cli: address: Do not output duplicate addresses
Michal Sojka writes:
>
> +/* Returns TRUE iff name and addr is duplicate. */
If you're revising this patch, it would be good to mention the side
effect of this function.
> -process_address_list (const search_context_t *ctx, InternetAddressList *list)
> +process_address_list (const search_context_t *ctx,
> + InternetAddressList *list)
It probably doesn't make any difference, but this looks like a needless
whitespace change.
This function definitely needs some comment / pointer to
documention. And probably not to have _my in the name.
> +static void
> +_my_talloc_free_for_g_hash (void *ptr)
> +{
> +talloc_free (ptr);
> +}
> +
I don't understand the name of the next subtest
> +test_begin_subtest "No --output"
> +notmuch address --output=sender --output=recipients '*' >OUTPUT
> +# Use EXPECTED from previous subtest
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +
> +test_done
nitpick, extra blank lines
So, AIUI, this is all of the series proposed for 0.19. It looks close to
OK to me, modulo some minor style nits. One anonymous commentator on
IRC mentioned the use of module scope variables, I guess in patch
6/10. I'm not sure of a better solution, but it's true in a perfect
world we wouldn't have module local state.
d
[PATCH v2 08/10] cli: address: Do not output duplicate addresses
This filters out duplicate addresses from address command output.
It also also adds tests for the address command.
The code here is an extended version of a patch from Jani Nikula.
---
doc/man1/notmuch-address.rst | 2 +-
notmuch-search.c | 40 -
test/T095-address.sh | 100 +++
3 files changed, 140 insertions(+), 2 deletions(-)
create mode 100755 test/T095-address.sh
diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index 8109f11..96512b7 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -11,7 +11,7 @@ DESCRIPTION
===
Search for messages matching the given search terms, and display the
-addresses from them.
+addresses from them. Duplicate addresses are filtered out.
See **notmuch-search-terms(7)** for details of the supported syntax for
.
diff --git a/notmuch-search.c b/notmuch-search.c
index 402e860..741702a 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -53,6 +53,7 @@ typedef struct {
int offset;
int limit;
int dupe;
+GHashTable *addresses;
} search_context_t;
typedef struct {
@@ -240,6 +241,27 @@ do_search_threads (search_context_t *ctx)
return 0;
}
+/* Returns TRUE iff name and addr is duplicate. */
+static notmuch_bool_t
+is_duplicate (const search_context_t *ctx, const char *name, const char *addr)
+{
+notmuch_bool_t duplicate;
+char *key;
+
+key = talloc_asprintf (ctx->format, "%s <%s>", name, addr);
+if (! key)
+ return FALSE;
+
+duplicate = g_hash_table_lookup_extended (ctx->addresses, key, NULL, NULL);
+
+if (! duplicate)
+ g_hash_table_insert (ctx->addresses, key, NULL);
+else
+ talloc_free (key);
+
+return duplicate;
+}
+
static void
print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
{
@@ -274,7 +296,8 @@ print_mailbox (const search_context_t *ctx, const mailbox_t
*mailbox)
/* Print addresses from InternetAddressList. */
static void
-process_address_list (const search_context_t *ctx, InternetAddressList *list)
+process_address_list (const search_context_t *ctx,
+ InternetAddressList *list)
{
InternetAddress *address;
int i;
@@ -298,6 +321,9 @@ process_address_list (const search_context_t *ctx,
InternetAddressList *list)
.addr = internet_address_mailbox_get_addr (mailbox),
};
+ if (is_duplicate (ctx, mbx.name, mbx.addr))
+ continue;
+
print_mailbox (ctx, );
}
}
@@ -321,6 +347,12 @@ process_address_header (const search_context_t *ctx, const
char *value)
g_object_unref (list);
}
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+talloc_free (ptr);
+}
+
static int
_count_filenames (notmuch_message_t *message)
{
@@ -669,8 +701,14 @@ notmuch_address_command (notmuch_config_t *config, int
argc, char *argv[])
argc - opt_index, argv + opt_index))
return EXIT_FAILURE;
+ctx->addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
+ _my_talloc_free_for_g_hash, NULL);
+
ret = do_search_messages (ctx);
+g_hash_table_unref (ctx->addresses);
+
+
_notmuch_search_cleanup (ctx);
return ret ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/test/T095-address.sh b/test/T095-address.sh
new file mode 100755
index 000..8a256d2
--- /dev/null
+++ b/test/T095-address.sh
@@ -0,0 +1,100 @@
+#!/usr/bin/env bash
+test_description='"notmuch address" in several variants'
+. ./test-lib.sh
+
+add_email_corpus
+
+test_begin_subtest "--output=sender"
+notmuch address --output=sender '*' >OUTPUT
+cat OUTPUT
+cat
Re: [PATCH v2 08/10] cli: address: Do not output duplicate addresses
On Tue, Nov 04 2014, David Bremner wrote:
Michal Sojka sojk...@fel.cvut.cz writes:
+/* Returns TRUE iff name and addr is duplicate. */
If you're revising this patch, it would be good to mention the side
effect of this function.
-process_address_list (const search_context_t *ctx, InternetAddressList
*list)
+process_address_list (const search_context_t *ctx,
+ InternetAddressList *list)
It probably doesn't make any difference, but this looks like a needless
whitespace change.
This function definitely needs some comment / pointer to
documention. And probably not to have _my in the name.
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+talloc_free (ptr);
+}
+
I don't understand the name of the next subtest
+test_begin_subtest No --output
+notmuch address --output=sender --output=recipients '*' OUTPUT
This should be notmuch address '*' OUTPUT. I'll fix that.
+# Use EXPECTED from previous subtest
+test_expect_equal_file OUTPUT EXPECTED
+
+
+test_done
nitpick, extra blank lines
So, AIUI, this is all of the series proposed for 0.19.
Agreed.
It looks close to OK to me, modulo some minor style nits. One
anonymous commentator on IRC mentioned the use of module scope
variables, I guess in patch 6/10. I'm not sure of a better solution,
but it's true in a perfect world we wouldn't have module local state.
A possible solution would be fill in common_options structure
programmatically, but this would make the code much less readable. I can
think of a few other solutions but none of them would fit into perfect
world :)
I'll send updated patches in the evening (CET timezone).
Thanks
-Michal
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 08/10] cli: address: Do not output duplicate addresses
This filters out duplicate addresses from address command output.
It also also adds tests for the address command.
The code here is an extended version of a patch from Jani Nikula.
---
doc/man1/notmuch-address.rst | 2 +-
notmuch-search.c | 40 -
test/T095-address.sh | 100 +++
3 files changed, 140 insertions(+), 2 deletions(-)
create mode 100755 test/T095-address.sh
diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index 8109f11..96512b7 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -11,7 +11,7 @@ DESCRIPTION
===
Search for messages matching the given search terms, and display the
-addresses from them.
+addresses from them. Duplicate addresses are filtered out.
See **notmuch-search-terms(7)** for details of the supported syntax for
search-terms.
diff --git a/notmuch-search.c b/notmuch-search.c
index 402e860..741702a 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -53,6 +53,7 @@ typedef struct {
int offset;
int limit;
int dupe;
+GHashTable *addresses;
} search_context_t;
typedef struct {
@@ -240,6 +241,27 @@ do_search_threads (search_context_t *ctx)
return 0;
}
+/* Returns TRUE iff name and addr is duplicate. */
+static notmuch_bool_t
+is_duplicate (const search_context_t *ctx, const char *name, const char *addr)
+{
+notmuch_bool_t duplicate;
+char *key;
+
+key = talloc_asprintf (ctx-format, %s %s, name, addr);
+if (! key)
+ return FALSE;
+
+duplicate = g_hash_table_lookup_extended (ctx-addresses, key, NULL, NULL);
+
+if (! duplicate)
+ g_hash_table_insert (ctx-addresses, key, NULL);
+else
+ talloc_free (key);
+
+return duplicate;
+}
+
static void
print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
{
@@ -274,7 +296,8 @@ print_mailbox (const search_context_t *ctx, const mailbox_t
*mailbox)
/* Print addresses from InternetAddressList. */
static void
-process_address_list (const search_context_t *ctx, InternetAddressList *list)
+process_address_list (const search_context_t *ctx,
+ InternetAddressList *list)
{
InternetAddress *address;
int i;
@@ -298,6 +321,9 @@ process_address_list (const search_context_t *ctx,
InternetAddressList *list)
.addr = internet_address_mailbox_get_addr (mailbox),
};
+ if (is_duplicate (ctx, mbx.name, mbx.addr))
+ continue;
+
print_mailbox (ctx, mbx);
}
}
@@ -321,6 +347,12 @@ process_address_header (const search_context_t *ctx, const
char *value)
g_object_unref (list);
}
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+talloc_free (ptr);
+}
+
static int
_count_filenames (notmuch_message_t *message)
{
@@ -669,8 +701,14 @@ notmuch_address_command (notmuch_config_t *config, int
argc, char *argv[])
argc - opt_index, argv + opt_index))
return EXIT_FAILURE;
+ctx-addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
+ _my_talloc_free_for_g_hash, NULL);
+
ret = do_search_messages (ctx);
+g_hash_table_unref (ctx-addresses);
+
+
_notmuch_search_cleanup (ctx);
return ret ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/test/T095-address.sh b/test/T095-address.sh
new file mode 100755
index 000..8a256d2
--- /dev/null
+++ b/test/T095-address.sh
@@ -0,0 +1,100 @@
+#!/usr/bin/env bash
+test_description='notmuch address in several variants'
+. ./test-lib.sh
+
+add_email_corpus
+
+test_begin_subtest --output=sender
+notmuch address --output=sender '*' OUTPUT
+cat EOF EXPECTED
+François Boulogne boulogn...@gmail.com
+Olivier Berger olivier.ber...@it-sudparis.eu
+Chris Wilson ch...@chris-wilson.co.uk
+Carl Worth cwo...@cworth.org
+Alexander Botero-Lowry alex.boterolo...@gmail.com
+Keith Packard kei...@keithp.com
+Jjgod Jiang gzjj...@gmail.com
+Rolland Santimano rollandsantim...@yahoo.com
+Jan Janak j...@ryngle.com
+Stewart Smith stew...@flamingspork.com
+Lars Kellogg-Stedman l...@seas.harvard.edu
+Alex Botero-Lowry alex.boterolo...@gmail.com
+Ingmar Vanhassel ing...@exherbo.org
+Aron Griffis agrif...@n01se.net
+Adrian Perez de Castro ape...@igalia.com
+Israel Herraiz i...@herraiz.org
+Mikhail Gusarov dotted...@dottedmag.net
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest --output=sender --format=json
+notmuch address --output=sender --format=json '*' OUTPUT
+cat EOF EXPECTED
+[{name: François Boulogne, address: boulogn...@gmail.com, name-addr:
François Boulogne boulogn...@gmail.com},
+{name: Olivier Berger, address: olivier.ber...@it-sudparis.eu,
name-addr: Olivier Berger olivier.ber...@it-sudparis.eu},
+{name: Chris Wilson, address: ch...@chris-wilson.co.uk, name-addr:
Chris Wilson ch...@chris-wilson.co.uk},
+{name: Carl Worth, address: cwo...@cworth.org,
Re: [PATCH v2 08/10] cli: address: Do not output duplicate addresses
Michal Sojka sojk...@fel.cvut.cz writes:
+/* Returns TRUE iff name and addr is duplicate. */
If you're revising this patch, it would be good to mention the side
effect of this function.
-process_address_list (const search_context_t *ctx, InternetAddressList *list)
+process_address_list (const search_context_t *ctx,
+ InternetAddressList *list)
It probably doesn't make any difference, but this looks like a needless
whitespace change.
This function definitely needs some comment / pointer to
documention. And probably not to have _my in the name.
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+talloc_free (ptr);
+}
+
I don't understand the name of the next subtest
+test_begin_subtest No --output
+notmuch address --output=sender --output=recipients '*' OUTPUT
+# Use EXPECTED from previous subtest
+test_expect_equal_file OUTPUT EXPECTED
+
+
+test_done
nitpick, extra blank lines
So, AIUI, this is all of the series proposed for 0.19. It looks close to
OK to me, modulo some minor style nits. One anonymous commentator on
IRC mentioned the use of module scope variables, I guess in patch
6/10. I'm not sure of a better solution, but it's true in a perfect
world we wouldn't have module local state.
d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
