unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 1/2] config: Add 'config list' command
@ 2012-03-20 22:31 Peter Wang
  2012-03-20 22:31 ` [PATCH 2/2] man: Document " Peter Wang
                   ` (5 more replies)
  0 siblings, 6 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-20 22:31 UTC (permalink / raw)
  To: notmuch

Add a command to list all keys in a given configuration section.

One use is as follows: a MUA may prefer to store data in a central
notmuch configuration file so that the data is accessible across
different machines, e.g. an addressbook.  The list command helps
to implement features such as tab completion on the keys.
---
 notmuch-config.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index e9b2750..595cf54 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -799,6 +799,31 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
     return ret;
 }
 
+static int
+notmuch_config_command_list (void *ctx, char *group)
+{
+    notmuch_config_t *config;
+    char **keys;
+    size_t i, length;
+
+    config = notmuch_config_open (ctx, NULL, NULL);
+    if (config == NULL)
+	return 1;
+
+    keys = g_key_file_get_keys (config->key_file,
+				group, &length, NULL);
+    if (keys != NULL) {
+	for (i = 0; i < length; i++)
+	    printf ("%s\n", keys[i]);
+
+	free (keys);
+    }
+
+    notmuch_config_close (config);
+
+    return 0;
+}
+
 int
 notmuch_config_command (void *ctx, int argc, char *argv[])
 {
@@ -813,6 +838,8 @@ notmuch_config_command (void *ctx, int argc, char *argv[])
 	return notmuch_config_command_get (ctx, argv[1]);
     else if (strcmp (argv[0], "set") == 0)
 	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);
+    else if (strcmp (argv[0], "list") == 0)
+	return notmuch_config_command_list (ctx, argv[1]);
 
     fprintf (stderr, "Unrecognized argument for notmuch config: %s\n",
 	     argv[0]);
-- 
1.7.4.4

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

* [PATCH 2/2] man: Document 'config list' command
  2012-03-20 22:31 [PATCH 1/2] config: Add 'config list' command Peter Wang
@ 2012-03-20 22:31 ` Peter Wang
  2012-03-21 18:30 ` [PATCH 1/2] config: Add " Tomi Ollila
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-20 22:31 UTC (permalink / raw)
  To: notmuch

Document the 'config list' command and its output.
---
 man/man1/notmuch-config.1 |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-config.1 b/man/man1/notmuch-config.1
index 395cb9c..b9f81a5 100644
--- a/man/man1/notmuch-config.1
+++ b/man/man1/notmuch-config.1
@@ -9,6 +9,9 @@ notmuch-config \- Access notmuch configuration file.
 .B notmuch config set
 .RI  "<" section ">.<" item "> [" value " ...]"
 
+.B notmuch config list
+.RI  "<" section ">
+
 .SH DESCRIPTION
 
 The
@@ -35,6 +38,15 @@ If no values are provided, the specified configuration item will be
 removed from the configuration file.
 .RE
 
+.RS 4
+.TP 4
+.B list
+The name of each configuration item in the specified section is
+printed to stdout, without the section name prefix. Each is terminated
+by a newline character. The output is empty if the section does not
+exist.
+.RE
+
 The available configuration items are described below.
 
 .RS 4
-- 
1.7.4.4

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

* Re: [PATCH 1/2] config: Add 'config list' command
  2012-03-20 22:31 [PATCH 1/2] config: Add 'config list' command Peter Wang
  2012-03-20 22:31 ` [PATCH 2/2] man: Document " Peter Wang
@ 2012-03-21 18:30 ` Tomi Ollila
  2012-03-21 22:52   ` Peter Wang
  2012-03-26 15:03   ` Jameson Graef Rollins
  2012-03-26 14:56 ` Jameson Graef Rollins
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 36+ messages in thread
From: Tomi Ollila @ 2012-03-21 18:30 UTC (permalink / raw)
  To: Peter Wang, notmuch

On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang <novalazy@gmail.com> wrote:
> Add a command to list all keys in a given configuration section.
> 
> One use is as follows: a MUA may prefer to store data in a central
> notmuch configuration file so that the data is accessible across
> different machines, e.g. an addressbook.  The list command helps
> to implement features such as tab completion on the keys.

Before getting deeper into implementation it is good to remember
that 'notmuch config' is loosely modeled after 'git config'.

git config --list outputs somewhat different output.

Maybe this command should be like 'notmuch config list-section-keys'

> ---
>  notmuch-config.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)

Tomi

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

* Re: [PATCH 1/2] config: Add 'config list' command
  2012-03-21 18:30 ` [PATCH 1/2] config: Add " Tomi Ollila
@ 2012-03-21 22:52   ` Peter Wang
  2012-03-26 15:03   ` Jameson Graef Rollins
  1 sibling, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-21 22:52 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

On Wed, 21 Mar 2012 20:30:43 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang <novalazy@gmail.com> wrote:
> > Add a command to list all keys in a given configuration section.
> > 
> > One use is as follows: a MUA may prefer to store data in a central
> > notmuch configuration file so that the data is accessible across
> > different machines, e.g. an addressbook.  The list command helps
> > to implement features such as tab completion on the keys.
> 
> Before getting deeper into implementation it is good to remember
> that 'notmuch config' is loosely modeled after 'git config'.
> 
> git config --list outputs somewhat different output.
> 
> Maybe this command should be like 'notmuch config list-section-keys'

That would be fine.  The documentation never mentions keys, only
sections and items.  The implementation seems to think item == group.key.

Peter

P.S. In the first patch free() should be g_strfreev().

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

* Re: [PATCH 1/2] config: Add 'config list' command
  2012-03-20 22:31 [PATCH 1/2] config: Add 'config list' command Peter Wang
  2012-03-20 22:31 ` [PATCH 2/2] man: Document " Peter Wang
  2012-03-21 18:30 ` [PATCH 1/2] config: Add " Tomi Ollila
@ 2012-03-26 14:56 ` Jameson Graef Rollins
  2012-03-30 23:15 ` [PATCH v2] " Peter Wang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 36+ messages in thread
From: Jameson Graef Rollins @ 2012-03-26 14:56 UTC (permalink / raw)
  To: Peter Wang, notmuch

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

On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang <novalazy@gmail.com> wrote:
> Add a command to list all keys in a given configuration section.

Hi, Peter.  Don't forget to include appropriate tests when introducing
new features.  Thanks.

jamie.

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

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

* Re: [PATCH 1/2] config: Add 'config list' command
  2012-03-21 18:30 ` [PATCH 1/2] config: Add " Tomi Ollila
  2012-03-21 22:52   ` Peter Wang
@ 2012-03-26 15:03   ` Jameson Graef Rollins
  1 sibling, 0 replies; 36+ messages in thread
From: Jameson Graef Rollins @ 2012-03-26 15:03 UTC (permalink / raw)
  To: Tomi Ollila, Peter Wang, notmuch

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

On Wed, 21 Mar 2012 20:30:43 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang <novalazy@gmail.com> wrote:
> > Add a command to list all keys in a given configuration section.
> > 
> > One use is as follows: a MUA may prefer to store data in a central
> > notmuch configuration file so that the data is accessible across
> > different machines, e.g. an addressbook.  The list command helps
> > to implement features such as tab completion on the keys.
> 
> Before getting deeper into implementation it is good to remember
> that 'notmuch config' is loosely modeled after 'git config'.
> 
> git config --list outputs somewhat different output.
> 
> Maybe this command should be like 'notmuch config list-section-keys'

Git's interface is completely inconsistent, so I see no reason to try to
emulate it in every regard.  Let's just try to keep the notmuch
interface as nice and consistent as possible.

I do agree, though, that I think I would prefer to see the output in the
same format as "git config --list".  So I would suggest the command and
output be:

$ notmuch config list
database.path=/home/jrollins/.mail
user.name=Jameson Graef Rollins
user.primary_email=jrollins@finestructure.net
user.other_email=
new.tags=new;unread
maildir.synchronize_flags=false
search.exclude_tags=deleted;spam;

That seem like a much more generally useful output.

jamie.

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

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

* [PATCH v2] Add 'config list' command
  2012-03-20 22:31 [PATCH 1/2] config: Add 'config list' command Peter Wang
                   ` (2 preceding siblings ...)
  2012-03-26 14:56 ` Jameson Graef Rollins
@ 2012-03-30 23:15 ` Peter Wang
  2012-03-30 23:15   ` [PATCH v2 1/5] config: Fix free in 'config get' implementation Peter Wang
                     ` (4 more replies)
  2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
  5 siblings, 5 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-30 23:15 UTC (permalink / raw)
  To: notmuch

This version prints all config items and their values
instead of just the keys of a single section.

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

* [PATCH v2 1/5] config: Fix free in 'config get' implementation.
  2012-03-30 23:15 ` [PATCH v2] " Peter Wang
@ 2012-03-30 23:15   ` Peter Wang
  2012-03-30 23:15   ` [PATCH v2 2/5] test: Add tests for 'config' command Peter Wang
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-30 23:15 UTC (permalink / raw)
  To: notmuch

The array returned by g_key_file_get_string_list() should be freed with
g_strfreev(), not free().
---
 notmuch-config.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index e9b2750..85fc774 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -751,7 +751,7 @@ notmuch_config_command_get (void *ctx, char *item)
 	for (i = 0; i < length; i++)
 	    printf ("%s\n", value[i]);
 
-	free (value);
+	g_strfreev (value);
     }
 
     notmuch_config_close (config);
-- 
1.7.4.4

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

* [PATCH v2 2/5] test: Add tests for 'config' command
  2012-03-30 23:15 ` [PATCH v2] " Peter Wang
  2012-03-30 23:15   ` [PATCH v2 1/5] config: Fix free in 'config get' implementation Peter Wang
@ 2012-03-30 23:15   ` Peter Wang
  2012-03-31  7:45     ` Mark Walters
  2012-03-30 23:15   ` [PATCH v2 3/5] test: Add broken test for 'config list' Peter Wang
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 36+ messages in thread
From: Peter Wang @ 2012-03-30 23:15 UTC (permalink / raw)
  To: notmuch

Start a new test script.
---
 test/config       |   20 ++++++++++++++++++++
 test/notmuch-test |    1 +
 2 files changed, 21 insertions(+), 0 deletions(-)
 create mode 100755 test/config

diff --git a/test/config b/test/config
new file mode 100755
index 0000000..d3e574c
--- /dev/null
+++ b/test/config
@@ -0,0 +1,20 @@
+#!/usr/bin/env bash
+
+test_description='"notmuch config"'
+. test-lib.sh
+
+test_begin_subtest "Get string value"
+test_expect_equal "$(notmuch config get user.name)" "Notmuch Test Suite"
+
+test_begin_subtest "Get list value"
+test_expect_equal "$(notmuch config get new.tags)" "\
+unread
+inbox"
+
+test_expect_success "Set string value" \
+  'notmuch config set foo.bar baz'
+
+test_expect_success "Set list value" \
+  'notmuch config set foo.list xxx "yyy yyy" "zzz zzz"'
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index f03b594..e08ec72 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -19,6 +19,7 @@ cd $(dirname "$0")
 TESTS="
   basic
   help-test
+  config
   new
   count
   search
-- 
1.7.4.4

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

* [PATCH v2 3/5] test: Add broken test for 'config list'
  2012-03-30 23:15 ` [PATCH v2] " Peter Wang
  2012-03-30 23:15   ` [PATCH v2 1/5] config: Fix free in 'config get' implementation Peter Wang
  2012-03-30 23:15   ` [PATCH v2 2/5] test: Add tests for 'config' command Peter Wang
@ 2012-03-30 23:15   ` Peter Wang
  2012-03-30 23:15   ` [PATCH v2 4/5] config: Add 'config list' command Peter Wang
  2012-03-30 23:15   ` [PATCH v2 5/5] man: Document " Peter Wang
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-30 23:15 UTC (permalink / raw)
  To: notmuch

Proposed functionality.
---
 test/config |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/test/config b/test/config
index d3e574c..73c58ff 100755
--- a/test/config
+++ b/test/config
@@ -17,4 +17,20 @@ test_expect_success "Set string value" \
 test_expect_success "Set list value" \
   'notmuch config set foo.list xxx "yyy yyy" "zzz zzz"'
 
+test_begin_subtest "List all items"
+test_subtest_known_broken
+notmuch config set database.path "/canonical/path"
+output=$(notmuch config list)
+test_expect_equal "$output" "\
+database.path=/canonical/path
+user.name=Notmuch Test Suite
+user.primary_email=test_suite@notmuchmail.org
+user.other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
+new.tags=unread;inbox;
+new.ignore=
+search.exclude_tags=
+maildir.synchronize_flags=true
+foo.bar=baz
+foo.list=xxx;yyy yyy;zzz zzz;"
+
 test_done
-- 
1.7.4.4

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

* [PATCH v2 4/5] config: Add 'config list' command
  2012-03-30 23:15 ` [PATCH v2] " Peter Wang
                     ` (2 preceding siblings ...)
  2012-03-30 23:15   ` [PATCH v2 3/5] test: Add broken test for 'config list' Peter Wang
@ 2012-03-30 23:15   ` Peter Wang
  2012-03-30 23:15   ` [PATCH v2 5/5] man: Document " Peter Wang
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-30 23:15 UTC (permalink / raw)
  To: notmuch

Add a command to list all configuration items with their associated
values.

One use is as follows: a MUA may prefer to store data in a central
notmuch configuration file so that the data is accessible across
different machines, e.g. an addressbook.  The list command helps
to implement features such as tab completion on the keys.
---
 notmuch-config.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 test/config      |    1 -
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 85fc774..d5540ac 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -799,20 +799,78 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
     return ret;
 }
 
+static int
+notmuch_config_command_list (void *ctx)
+{
+    notmuch_config_t *config;
+    char **groups;
+    size_t g, groups_length;
+
+    config = notmuch_config_open (ctx, NULL, NULL);
+    if (config == NULL)
+	return 1;
+
+    groups = g_key_file_get_groups (config->key_file, &groups_length);
+    if (groups == NULL)
+	return 1;
+
+    for (g = 0; g < groups_length; g++) {
+	char **keys;
+	size_t k, keys_length;
+
+	keys = g_key_file_get_keys (config->key_file,
+				    groups[g], &keys_length, NULL);
+	if (keys == NULL)
+	    continue;
+
+	for (k = 0; k < keys_length; k++) {
+	    char *value;
+
+	    value = g_key_file_get_string (config->key_file,
+					   groups[g], keys[k], NULL);
+	    if (value != NULL) {
+		printf ("%s.%s=%s\n", groups[g], keys[k], value);
+		free (value);
+	    }
+	}
+
+	g_strfreev (keys);
+    }
+
+    g_strfreev (groups);
+
+    notmuch_config_close (config);
+
+    return 0;
+}
+
 int
 notmuch_config_command (void *ctx, int argc, char *argv[])
 {
     argc--; argv++; /* skip subcommand argument */
 
-    if (argc < 2) {
-	fprintf (stderr, "Error: notmuch config requires at least two arguments.\n");
+    if (argc < 1) {
+	fprintf (stderr, "Error: notmuch config requires at least one argument.\n");
 	return 1;
     }
 
-    if (strcmp (argv[0], "get") == 0)
+    if (strcmp (argv[0], "get") == 0) {
+	if (argc < 2) {
+	    fprintf (stderr, "Error: notmuch config get requires at least "
+		     "two arguments.\n");
+	    return 1;
+	}
 	return notmuch_config_command_get (ctx, argv[1]);
-    else if (strcmp (argv[0], "set") == 0)
+    } else if (strcmp (argv[0], "set") == 0) {
+	if (argc < 2) {
+	    fprintf (stderr, "Error: notmuch config set requires at least "
+		     "two arguments.\n");
+	    return 1;
+	}
 	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);
+    } else if (strcmp (argv[0], "list") == 0) {
+	return notmuch_config_command_list (ctx);
+    }
 
     fprintf (stderr, "Unrecognized argument for notmuch config: %s\n",
 	     argv[0]);
diff --git a/test/config b/test/config
index 73c58ff..93e3a04 100755
--- a/test/config
+++ b/test/config
@@ -18,7 +18,6 @@ test_expect_success "Set list value" \
   'notmuch config set foo.list xxx "yyy yyy" "zzz zzz"'
 
 test_begin_subtest "List all items"
-test_subtest_known_broken
 notmuch config set database.path "/canonical/path"
 output=$(notmuch config list)
 test_expect_equal "$output" "\
-- 
1.7.4.4

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

* [PATCH v2 5/5] man: Document 'config list' command
  2012-03-30 23:15 ` [PATCH v2] " Peter Wang
                     ` (3 preceding siblings ...)
  2012-03-30 23:15   ` [PATCH v2 4/5] config: Add 'config list' command Peter Wang
@ 2012-03-30 23:15   ` Peter Wang
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-03-30 23:15 UTC (permalink / raw)
  To: notmuch

Document the 'config list' command and its output.
---
 man/man1/notmuch-config.1 |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-config.1 b/man/man1/notmuch-config.1
index 395cb9c..a55dbef 100644
--- a/man/man1/notmuch-config.1
+++ b/man/man1/notmuch-config.1
@@ -9,6 +9,8 @@ notmuch-config \- Access notmuch configuration file.
 .B notmuch config set
 .RI  "<" section ">.<" item "> [" value " ...]"
 
+.B notmuch config list
+
 .SH DESCRIPTION
 
 The
@@ -35,6 +37,18 @@ If no values are provided, the specified configuration item will be
 removed from the configuration file.
 .RE
 
+.RS 4
+.TP 4
+.B list
+Every configuration item is printed to stdout, each on a separate line
+of the form:
+
+.RI  "" section "." item "=" value
+
+No additional whitespace surrounds the dot or equals sign characters. In a
+multiple-value item (a list), the values are separated by semicolon characters.
+.RE
+
 The available configuration items are described below.
 
 .RS 4
-- 
1.7.4.4

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

* Re: [PATCH v2 2/5] test: Add tests for 'config' command
  2012-03-30 23:15   ` [PATCH v2 2/5] test: Add tests for 'config' command Peter Wang
@ 2012-03-31  7:45     ` Mark Walters
  2012-03-31 21:47       ` Jameson Graef Rollins
  0 siblings, 1 reply; 36+ messages in thread
From: Mark Walters @ 2012-03-31  7:45 UTC (permalink / raw)
  To: Peter Wang, notmuch


This whole series looks good to me with one minor query.

> Start a new test script.
> ---
>  test/config       |   20 ++++++++++++++++++++
>  test/notmuch-test |    1 +
>  2 files changed, 21 insertions(+), 0 deletions(-)
>  create mode 100755 test/config
>
> diff --git a/test/config b/test/config
> new file mode 100755
> index 0000000..d3e574c
> --- /dev/null
> +++ b/test/config
> @@ -0,0 +1,20 @@
> +#!/usr/bin/env bash
> +
> +test_description='"notmuch config"'
> +. test-lib.sh
> +
> +test_begin_subtest "Get string value"
> +test_expect_equal "$(notmuch config get user.name)" "Notmuch Test Suite"
> +
> +test_begin_subtest "Get list value"
> +test_expect_equal "$(notmuch config get new.tags)" "\
> +unread
> +inbox"
> +
> +test_expect_success "Set string value" \
> +  'notmuch config set foo.bar baz'
> +
> +test_expect_success "Set list value" \
> +  'notmuch config set foo.list xxx "yyy yyy" "zzz zzz"'

It seems off to call is success without checking that the value has
actually been set. Of course it is checked in the notmuch config list
test introduced in the next commit but I think if it would be better to
check with notmuch config get here too (i.e. check that reading back the
value gives what you want). Otherwise a failure in `setting' will show up
as a test failure in `listing'.

Best wishes

Mark

> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index f03b594..e08ec72 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -19,6 +19,7 @@ cd $(dirname "$0")
>  TESTS="
>    basic
>    help-test
> +  config
>    new
>    count
>    search
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH v2 2/5] test: Add tests for 'config' command
  2012-03-31  7:45     ` Mark Walters
@ 2012-03-31 21:47       ` Jameson Graef Rollins
  0 siblings, 0 replies; 36+ messages in thread
From: Jameson Graef Rollins @ 2012-03-31 21:47 UTC (permalink / raw)
  To: Mark Walters, Peter Wang, notmuch

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

On Sat, Mar 31 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> It seems off to call is success without checking that the value has
> actually been set. Of course it is checked in the notmuch config list
> test introduced in the next commit but I think if it would be better to
> check with notmuch config get here too (i.e. check that reading back the
> value gives what you want). Otherwise a failure in `setting' will show up
> as a test failure in `listing'.

Hey, Peter.  I think Mark makes a good point here.  I think it would
make more sense for the test to set the value, and then check that the
value is properly set as expected.  It would make the tests multi-step,
but that's fine.  There's plenty of precedent for that.

jamie.

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

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

* [PATCH v3 0/5] Add 'config list' command
  2012-03-20 22:31 [PATCH 1/2] config: Add 'config list' command Peter Wang
                   ` (3 preceding siblings ...)
  2012-03-30 23:15 ` [PATCH v2] " Peter Wang
@ 2012-04-06  1:48 ` Peter Wang
  2012-04-06  1:48   ` [PATCH v3 1/5] config: Fix free in 'config get' implementation Peter Wang
                     ` (4 more replies)
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
  5 siblings, 5 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-06  1:48 UTC (permalink / raw)
  To: notmuch

This version tests 'config set' by reading back the set value, as suggested.


Peter Wang (5):
  config: Fix free in 'config get' implementation.
  test: Add tests for 'config' command
  test: Add broken test for 'config list'
  config: Add 'config list' command
  man: Document 'config list' command

 man/man1/notmuch-config.1 |   14 +++++++++
 notmuch-config.c          |   68 +++++++++++++++++++++++++++++++++++++++++---
 test/config               |   40 ++++++++++++++++++++++++++
 test/notmuch-test         |    1 +
 4 files changed, 118 insertions(+), 5 deletions(-)
 create mode 100755 test/config

-- 
1.7.4.4

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

* [PATCH v3 1/5] config: Fix free in 'config get' implementation.
  2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
@ 2012-04-06  1:48   ` Peter Wang
  2012-04-06  1:48   ` [PATCH v3 2/5] test: Add tests for 'config' command Peter Wang
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-06  1:48 UTC (permalink / raw)
  To: notmuch

The array returned by g_key_file_get_string_list() should be freed with
g_strfreev(), not free().
---
 notmuch-config.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index e9b2750..85fc774 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -751,7 +751,7 @@ notmuch_config_command_get (void *ctx, char *item)
 	for (i = 0; i < length; i++)
 	    printf ("%s\n", value[i]);
 
-	free (value);
+	g_strfreev (value);
     }
 
     notmuch_config_close (config);
-- 
1.7.4.4

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

* [PATCH v3 2/5] test: Add tests for 'config' command
  2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
  2012-04-06  1:48   ` [PATCH v3 1/5] config: Fix free in 'config get' implementation Peter Wang
@ 2012-04-06  1:48   ` Peter Wang
  2012-04-10  7:30     ` Jameson Graef Rollins
  2012-04-06  1:48   ` [PATCH v3 3/5] test: Add broken test for 'config list' Peter Wang
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 36+ messages in thread
From: Peter Wang @ 2012-04-06  1:48 UTC (permalink / raw)
  To: notmuch

Start a new test script.
---
 test/config       |   25 +++++++++++++++++++++++++
 test/notmuch-test |    1 +
 2 files changed, 26 insertions(+), 0 deletions(-)
 create mode 100755 test/config

diff --git a/test/config b/test/config
new file mode 100755
index 0000000..75f662d
--- /dev/null
+++ b/test/config
@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+
+test_description='"notmuch config"'
+. test-lib.sh
+
+test_begin_subtest "Get string value"
+test_expect_equal "$(notmuch config get user.name)" "Notmuch Test Suite"
+
+test_begin_subtest "Get list value"
+test_expect_equal "$(notmuch config get new.tags)" "\
+unread
+inbox"
+
+test_begin_subtest "Set string value"
+notmuch config set foo.bar baz
+test_expect_equal "$(notmuch config get foo.bar)" "baz"
+
+test_begin_subtest "Set list value"
+notmuch config set foo.list xxx "yyy yyy" "zzz zzz"
+test_expect_equal "$(notmuch config get foo.list)" "\
+xxx
+yyy yyy
+zzz zzz"
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index f03b594..e08ec72 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -19,6 +19,7 @@ cd $(dirname "$0")
 TESTS="
   basic
   help-test
+  config
   new
   count
   search
-- 
1.7.4.4

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

* [PATCH v3 3/5] test: Add broken test for 'config list'
  2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
  2012-04-06  1:48   ` [PATCH v3 1/5] config: Fix free in 'config get' implementation Peter Wang
  2012-04-06  1:48   ` [PATCH v3 2/5] test: Add tests for 'config' command Peter Wang
@ 2012-04-06  1:48   ` Peter Wang
  2012-04-06  1:48   ` [PATCH v3 4/5] config: Add 'config list' command Peter Wang
  2012-04-06  1:48   ` [PATCH v3 5/5] man: Document " Peter Wang
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-06  1:48 UTC (permalink / raw)
  To: notmuch

Proposed functionality.
---
 test/config |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/test/config b/test/config
index 75f662d..cce2abc 100755
--- a/test/config
+++ b/test/config
@@ -22,4 +22,20 @@ xxx
 yyy yyy
 zzz zzz"
 
+test_begin_subtest "List all items"
+test_subtest_known_broken
+notmuch config set database.path "/canonical/path"
+output=$(notmuch config list)
+test_expect_equal "$output" "\
+database.path=/canonical/path
+user.name=Notmuch Test Suite
+user.primary_email=test_suite@notmuchmail.org
+user.other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
+new.tags=unread;inbox;
+new.ignore=
+search.exclude_tags=
+maildir.synchronize_flags=true
+foo.bar=baz
+foo.list=xxx;yyy yyy;zzz zzz;"
+
 test_done
-- 
1.7.4.4

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

* [PATCH v3 4/5] config: Add 'config list' command
  2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
                     ` (2 preceding siblings ...)
  2012-04-06  1:48   ` [PATCH v3 3/5] test: Add broken test for 'config list' Peter Wang
@ 2012-04-06  1:48   ` Peter Wang
  2012-04-10  7:22     ` Jameson Graef Rollins
  2012-04-06  1:48   ` [PATCH v3 5/5] man: Document " Peter Wang
  4 siblings, 1 reply; 36+ messages in thread
From: Peter Wang @ 2012-04-06  1:48 UTC (permalink / raw)
  To: notmuch

Add a command to list all configuration items with their associated
values.

One use is as follows: a MUA may prefer to store data in a central
notmuch configuration file so that the data is accessible across
different machines, e.g. an addressbook.  The list command helps
to implement features such as tab completion on the keys.
---
 notmuch-config.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 test/config      |    1 -
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 85fc774..d5540ac 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -799,20 +799,78 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
     return ret;
 }
 
+static int
+notmuch_config_command_list (void *ctx)
+{
+    notmuch_config_t *config;
+    char **groups;
+    size_t g, groups_length;
+
+    config = notmuch_config_open (ctx, NULL, NULL);
+    if (config == NULL)
+	return 1;
+
+    groups = g_key_file_get_groups (config->key_file, &groups_length);
+    if (groups == NULL)
+	return 1;
+
+    for (g = 0; g < groups_length; g++) {
+	char **keys;
+	size_t k, keys_length;
+
+	keys = g_key_file_get_keys (config->key_file,
+				    groups[g], &keys_length, NULL);
+	if (keys == NULL)
+	    continue;
+
+	for (k = 0; k < keys_length; k++) {
+	    char *value;
+
+	    value = g_key_file_get_string (config->key_file,
+					   groups[g], keys[k], NULL);
+	    if (value != NULL) {
+		printf ("%s.%s=%s\n", groups[g], keys[k], value);
+		free (value);
+	    }
+	}
+
+	g_strfreev (keys);
+    }
+
+    g_strfreev (groups);
+
+    notmuch_config_close (config);
+
+    return 0;
+}
+
 int
 notmuch_config_command (void *ctx, int argc, char *argv[])
 {
     argc--; argv++; /* skip subcommand argument */
 
-    if (argc < 2) {
-	fprintf (stderr, "Error: notmuch config requires at least two arguments.\n");
+    if (argc < 1) {
+	fprintf (stderr, "Error: notmuch config requires at least one argument.\n");
 	return 1;
     }
 
-    if (strcmp (argv[0], "get") == 0)
+    if (strcmp (argv[0], "get") == 0) {
+	if (argc < 2) {
+	    fprintf (stderr, "Error: notmuch config get requires at least "
+		     "two arguments.\n");
+	    return 1;
+	}
 	return notmuch_config_command_get (ctx, argv[1]);
-    else if (strcmp (argv[0], "set") == 0)
+    } else if (strcmp (argv[0], "set") == 0) {
+	if (argc < 2) {
+	    fprintf (stderr, "Error: notmuch config set requires at least "
+		     "two arguments.\n");
+	    return 1;
+	}
 	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);
+    } else if (strcmp (argv[0], "list") == 0) {
+	return notmuch_config_command_list (ctx);
+    }
 
     fprintf (stderr, "Unrecognized argument for notmuch config: %s\n",
 	     argv[0]);
diff --git a/test/config b/test/config
index cce2abc..0fbc79b 100755
--- a/test/config
+++ b/test/config
@@ -23,7 +23,6 @@ yyy yyy
 zzz zzz"
 
 test_begin_subtest "List all items"
-test_subtest_known_broken
 notmuch config set database.path "/canonical/path"
 output=$(notmuch config list)
 test_expect_equal "$output" "\
-- 
1.7.4.4

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

* [PATCH v3 5/5] man: Document 'config list' command
  2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
                     ` (3 preceding siblings ...)
  2012-04-06  1:48   ` [PATCH v3 4/5] config: Add 'config list' command Peter Wang
@ 2012-04-06  1:48   ` Peter Wang
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-06  1:48 UTC (permalink / raw)
  To: notmuch

Document the 'config list' command and its output.
---
 man/man1/notmuch-config.1 |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-config.1 b/man/man1/notmuch-config.1
index 395cb9c..a55dbef 100644
--- a/man/man1/notmuch-config.1
+++ b/man/man1/notmuch-config.1
@@ -9,6 +9,8 @@ notmuch-config \- Access notmuch configuration file.
 .B notmuch config set
 .RI  "<" section ">.<" item "> [" value " ...]"
 
+.B notmuch config list
+
 .SH DESCRIPTION
 
 The
@@ -35,6 +37,18 @@ If no values are provided, the specified configuration item will be
 removed from the configuration file.
 .RE
 
+.RS 4
+.TP 4
+.B list
+Every configuration item is printed to stdout, each on a separate line
+of the form:
+
+.RI  "" section "." item "=" value
+
+No additional whitespace surrounds the dot or equals sign characters. In a
+multiple-value item (a list), the values are separated by semicolon characters.
+.RE
+
 The available configuration items are described below.
 
 .RS 4
-- 
1.7.4.4

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

* Re: [PATCH v3 4/5] config: Add 'config list' command
  2012-04-06  1:48   ` [PATCH v3 4/5] config: Add 'config list' command Peter Wang
@ 2012-04-10  7:22     ` Jameson Graef Rollins
  2012-04-10  8:31       ` Peter Wang
  0 siblings, 1 reply; 36+ messages in thread
From: Jameson Graef Rollins @ 2012-04-10  7:22 UTC (permalink / raw)
  To: Peter Wang, notmuch

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

On Thu, Apr 05 2012, Peter Wang <novalazy@gmail.com> wrote:
> Add a command to list all configuration items with their associated
> values.
>
> One use is as follows: a MUA may prefer to store data in a central
> notmuch configuration file so that the data is accessible across
> different machines, e.g. an addressbook.  The list command helps
> to implement features such as tab completion on the keys.
> ---
>  notmuch-config.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++---
>  test/config      |    1 -
>  2 files changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index 85fc774..d5540ac 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -799,20 +799,78 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
>      return ret;
>  }
>  
> +static int
> +notmuch_config_command_list (void *ctx)
> +{
> +    notmuch_config_t *config;
> +    char **groups;
> +    size_t g, groups_length;
> +
> +    config = notmuch_config_open (ctx, NULL, NULL);
> +    if (config == NULL)
> +	return 1;
> +
> +    groups = g_key_file_get_groups (config->key_file, &groups_length);
> +    if (groups == NULL)
> +	return 1;
> +
> +    for (g = 0; g < groups_length; g++) {
> +	char **keys;
> +	size_t k, keys_length;
> +
> +	keys = g_key_file_get_keys (config->key_file,
> +				    groups[g], &keys_length, NULL);
> +	if (keys == NULL)
> +	    continue;
> +
> +	for (k = 0; k < keys_length; k++) {
> +	    char *value;
> +
> +	    value = g_key_file_get_string (config->key_file,
> +					   groups[g], keys[k], NULL);
> +	    if (value != NULL) {
> +		printf ("%s.%s=%s\n", groups[g], keys[k], value);
> +		free (value);
> +	    }
> +	}
> +
> +	g_strfreev (keys);
> +    }
> +
> +    g_strfreev (groups);
> +
> +    notmuch_config_close (config);
> +
> +    return 0;
> +}
> +
>  int
>  notmuch_config_command (void *ctx, int argc, char *argv[])
>  {
>      argc--; argv++; /* skip subcommand argument */
>  
> -    if (argc < 2) {
> -	fprintf (stderr, "Error: notmuch config requires at least two arguments.\n");
> +    if (argc < 1) {
> +	fprintf (stderr, "Error: notmuch config requires at least one argument.\n");
>  	return 1;
>      }

Hey, Peter.  I would say everything up to here looks great.

> -    if (strcmp (argv[0], "get") == 0)
> +    if (strcmp (argv[0], "get") == 0) {
> +	if (argc < 2) {
> +	    fprintf (stderr, "Error: notmuch config get requires at least "
> +		     "two arguments.\n");
> +	    return 1;
> +	}
>  	return notmuch_config_command_get (ctx, argv[1]);
> -    else if (strcmp (argv[0], "set") == 0)
> +    } else if (strcmp (argv[0], "set") == 0) {
> +	if (argc < 2) {
> +	    fprintf (stderr, "Error: notmuch config set requires at least "
> +		     "two arguments.\n");
> +	    return 1;
> +	}
>  	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);

But then these changes look unrelated to me.  They do look good
intentioned, though.  It's probably best to submit these changes in a
separate unrelated patch.

jamie.

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

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

* Re: [PATCH v3 2/5] test: Add tests for 'config' command
  2012-04-06  1:48   ` [PATCH v3 2/5] test: Add tests for 'config' command Peter Wang
@ 2012-04-10  7:30     ` Jameson Graef Rollins
  0 siblings, 0 replies; 36+ messages in thread
From: Jameson Graef Rollins @ 2012-04-10  7:30 UTC (permalink / raw)
  To: Peter Wang, notmuch

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

On Thu, Apr 05 2012, Peter Wang <novalazy@gmail.com> wrote:
> +test_begin_subtest "Set string value"
> +notmuch config set foo.bar baz
> +test_expect_equal "$(notmuch config get foo.bar)" "baz"
> +
> +test_begin_subtest "Set list value"
> +notmuch config set foo.list xxx "yyy yyy" "zzz zzz"
> +test_expect_equal "$(notmuch config get foo.list)" "\
> +xxx
> +yyy yyy
> +zzz zzz"

I find it slightly strange to use non-existent fields here, but I also
don't see that it hurts anything either.  At least at the moment.
Another option would be to use an existing field, and change it back
when you're done.

jamie.

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

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

* Re: [PATCH v3 4/5] config: Add 'config list' command
  2012-04-10  7:22     ` Jameson Graef Rollins
@ 2012-04-10  8:31       ` Peter Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-10  8:31 UTC (permalink / raw)
  To: Jameson Graef Rollins; +Cc: notmuch

On Tue, 10 Apr 2012 00:22:01 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Thu, Apr 05 2012, Peter Wang <novalazy@gmail.com> wrote:
> 
> > -    if (strcmp (argv[0], "get") == 0)
> > +    if (strcmp (argv[0], "get") == 0) {
> > +	if (argc < 2) {
> > +	    fprintf (stderr, "Error: notmuch config get requires at least "
> > +		     "two arguments.\n");
> > +	    return 1;
> > +	}
> >  	return notmuch_config_command_get (ctx, argv[1]);
> > -    else if (strcmp (argv[0], "set") == 0)
> > +    } else if (strcmp (argv[0], "set") == 0) {
> > +	if (argc < 2) {
> > +	    fprintf (stderr, "Error: notmuch config set requires at least "
> > +		     "two arguments.\n");
> > +	    return 1;
> > +	}
> >  	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);
> 
> But then these changes look unrelated to me.  They do look good
> intentioned, though.  It's probably best to submit these changes in a
> separate unrelated patch.

Well, the only reason to duplicate the arity check is due to the
introduction of the 'list' subcommand.  But I see that the error
messages are wrong anyway, so I will separate out the changes in another
patch series.

Peter

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

* [PATCH v4 0/6] Config-related patches
  2012-03-20 22:31 [PATCH 1/2] config: Add 'config list' command Peter Wang
                   ` (4 preceding siblings ...)
  2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
@ 2012-04-14  1:41 ` Peter Wang
  2012-04-14  1:41   ` [PATCH v4 1/6] config: Fix free in 'config get' implementation Peter Wang
                     ` (7 more replies)
  5 siblings, 8 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-14  1:41 UTC (permalink / raw)
  To: notmuch

Changes from v3:
- rephrase part of the 'list' implementation as a separate patch
- test 'set' on an extant key
- test removing keys

Peter Wang (6):
  config: Fix free in 'config get' implementation.
  config: Check 'config get' arity exactly
  test: Add tests for 'config' command
  test: Add broken test for 'config list'
  config: Add 'config list' command
  man: Document 'config list' command

 man/man1/notmuch-config.1 |   14 +++++++++
 notmuch-config.c          |   68 +++++++++++++++++++++++++++++++++++++++++---
 test/config               |   60 +++++++++++++++++++++++++++++++++++++++
 test/notmuch-test         |    1 +
 4 files changed, 138 insertions(+), 5 deletions(-)
 create mode 100755 test/config

-- 
1.7.4.4

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

* [PATCH v4 1/6] config: Fix free in 'config get' implementation.
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
@ 2012-04-14  1:41   ` Peter Wang
  2012-04-25  2:29     ` David Bremner
  2012-04-14  1:41   ` [PATCH v4 2/6] config: Check 'config get' arity exactly Peter Wang
                     ` (6 subsequent siblings)
  7 siblings, 1 reply; 36+ messages in thread
From: Peter Wang @ 2012-04-14  1:41 UTC (permalink / raw)
  To: notmuch

The array returned by g_key_file_get_string_list() should be freed with
g_strfreev(), not free().
---
 notmuch-config.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index e9b2750..85fc774 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -751,7 +751,7 @@ notmuch_config_command_get (void *ctx, char *item)
 	for (i = 0; i < length; i++)
 	    printf ("%s\n", value[i]);
 
-	free (value);
+	g_strfreev (value);
     }
 
     notmuch_config_close (config);
-- 
1.7.4.4

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

* [PATCH v4 2/6] config: Check 'config get' arity exactly
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
  2012-04-14  1:41   ` [PATCH v4 1/6] config: Fix free in 'config get' implementation Peter Wang
@ 2012-04-14  1:41   ` Peter Wang
  2012-04-28 13:51     ` David Bremner
  2012-04-14  1:41   ` [PATCH v4 3/6] test: Add tests for 'config' command Peter Wang
                     ` (5 subsequent siblings)
  7 siblings, 1 reply; 36+ messages in thread
From: Peter Wang @ 2012-04-14  1:41 UTC (permalink / raw)
  To: notmuch

Require that 'config get' is passed exactly one additional argument,
instead of silently ignoring extra arguments. As a side-effect, produce
more specific error messages for the 'config' command as a whole.
---
 notmuch-config.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 85fc774..f9eb977 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -804,15 +804,26 @@ notmuch_config_command (void *ctx, int argc, char *argv[])
 {
     argc--; argv++; /* skip subcommand argument */
 
-    if (argc < 2) {
-	fprintf (stderr, "Error: notmuch config requires at least two arguments.\n");
+    if (argc < 1) {
+	fprintf (stderr, "Error: notmuch config requires at least one argument.\n");
 	return 1;
     }
 
-    if (strcmp (argv[0], "get") == 0)
+    if (strcmp (argv[0], "get") == 0) {
+	if (argc != 2) {
+	    fprintf (stderr, "Error: notmuch config get requires exactly "
+		     "one argument.\n");
+	    return 1;
+	}
 	return notmuch_config_command_get (ctx, argv[1]);
-    else if (strcmp (argv[0], "set") == 0)
+    } else if (strcmp (argv[0], "set") == 0) {
+	if (argc < 2) {
+	    fprintf (stderr, "Error: notmuch config set requires at least "
+		     "one argument.\n");
+	    return 1;
+	}
 	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);
+    }
 
     fprintf (stderr, "Unrecognized argument for notmuch config: %s\n",
 	     argv[0]);
-- 
1.7.4.4

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

* [PATCH v4 3/6] test: Add tests for 'config' command
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
  2012-04-14  1:41   ` [PATCH v4 1/6] config: Fix free in 'config get' implementation Peter Wang
  2012-04-14  1:41   ` [PATCH v4 2/6] config: Check 'config get' arity exactly Peter Wang
@ 2012-04-14  1:41   ` Peter Wang
  2012-04-14  1:41   ` [PATCH v4 4/6] test: Add broken test for 'config list' Peter Wang
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-14  1:41 UTC (permalink / raw)
  To: notmuch

Start a new test script.
---
 test/config       |   45 +++++++++++++++++++++++++++++++++++++++++++++
 test/notmuch-test |    1 +
 2 files changed, 46 insertions(+), 0 deletions(-)
 create mode 100755 test/config

diff --git a/test/config b/test/config
new file mode 100755
index 0000000..9030154
--- /dev/null
+++ b/test/config
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+
+test_description='"notmuch config"'
+. test-lib.sh
+
+test_begin_subtest "Get string value"
+test_expect_equal "$(notmuch config get user.name)" "Notmuch Test Suite"
+
+test_begin_subtest "Get list value"
+test_expect_equal "$(notmuch config get new.tags)" "\
+unread
+inbox"
+
+test_begin_subtest "Set string value"
+notmuch config set foo.string "this is a string value"
+test_expect_equal "$(notmuch config get foo.string)" "this is a string value"
+
+test_begin_subtest "Set string value again"
+notmuch config set foo.string "this is another string value"
+test_expect_equal "$(notmuch config get foo.string)" "this is another string value"
+
+test_begin_subtest "Set list value"
+notmuch config set foo.list this "is a" "list value"
+test_expect_equal "$(notmuch config get foo.list)" "\
+this
+is a
+list value"
+
+test_begin_subtest "Set list value again"
+notmuch config set foo.list this "is another" "list value"
+test_expect_equal "$(notmuch config get foo.list)" "\
+this
+is another
+list value"
+
+test_begin_subtest "Remove key"
+notmuch config set foo.remove baz
+notmuch config set foo.remove
+test_expect_equal "$(notmuch config get foo.remove)" ""
+
+test_begin_subtest "Remove non-existent key"
+notmuch config set foo.nonexistent
+test_expect_equal "$(notmuch config get foo.nonexistent)" ""
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index f03b594..e08ec72 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -19,6 +19,7 @@ cd $(dirname "$0")
 TESTS="
   basic
   help-test
+  config
   new
   count
   search
-- 
1.7.4.4

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

* [PATCH v4 4/6] test: Add broken test for 'config list'
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
                     ` (2 preceding siblings ...)
  2012-04-14  1:41   ` [PATCH v4 3/6] test: Add tests for 'config' command Peter Wang
@ 2012-04-14  1:41   ` Peter Wang
  2012-04-14  1:41   ` [PATCH v4 5/6] config: Add 'config list' command Peter Wang
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-14  1:41 UTC (permalink / raw)
  To: notmuch

Proposed functionality.
---
 test/config |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/test/config b/test/config
index 9030154..3bf8098 100755
--- a/test/config
+++ b/test/config
@@ -42,4 +42,20 @@ test_begin_subtest "Remove non-existent key"
 notmuch config set foo.nonexistent
 test_expect_equal "$(notmuch config get foo.nonexistent)" ""
 
+test_begin_subtest "List all items"
+test_subtest_known_broken
+notmuch config set database.path "/canonical/path"
+output=$(notmuch config list)
+test_expect_equal "$output" "\
+database.path=/canonical/path
+user.name=Notmuch Test Suite
+user.primary_email=test_suite@notmuchmail.org
+user.other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
+new.tags=unread;inbox;
+new.ignore=
+search.exclude_tags=
+maildir.synchronize_flags=true
+foo.string=this is another string value
+foo.list=this;is another;list value;"
+
 test_done
-- 
1.7.4.4

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

* [PATCH v4 5/6] config: Add 'config list' command
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
                     ` (3 preceding siblings ...)
  2012-04-14  1:41   ` [PATCH v4 4/6] test: Add broken test for 'config list' Peter Wang
@ 2012-04-14  1:41   ` Peter Wang
  2012-04-14  1:41   ` [PATCH v4 6/6] man: Document " Peter Wang
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-14  1:41 UTC (permalink / raw)
  To: notmuch

Add a command to list all configuration items with their associated
values.

One use is as follows: a MUA may prefer to store data in a central
notmuch configuration file so that the data is accessible across
different machines, e.g. an addressbook.  The list command helps
to implement features such as tab completion on the keys.
---
 notmuch-config.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 test/config      |    1 -
 2 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index f9eb977..3e37a2d 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -799,6 +799,51 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
     return ret;
 }
 
+static int
+notmuch_config_command_list (void *ctx)
+{
+    notmuch_config_t *config;
+    char **groups;
+    size_t g, groups_length;
+
+    config = notmuch_config_open (ctx, NULL, NULL);
+    if (config == NULL)
+	return 1;
+
+    groups = g_key_file_get_groups (config->key_file, &groups_length);
+    if (groups == NULL)
+	return 1;
+
+    for (g = 0; g < groups_length; g++) {
+	char **keys;
+	size_t k, keys_length;
+
+	keys = g_key_file_get_keys (config->key_file,
+				    groups[g], &keys_length, NULL);
+	if (keys == NULL)
+	    continue;
+
+	for (k = 0; k < keys_length; k++) {
+	    char *value;
+
+	    value = g_key_file_get_string (config->key_file,
+					   groups[g], keys[k], NULL);
+	    if (value != NULL) {
+		printf ("%s.%s=%s\n", groups[g], keys[k], value);
+		free (value);
+	    }
+	}
+
+	g_strfreev (keys);
+    }
+
+    g_strfreev (groups);
+
+    notmuch_config_close (config);
+
+    return 0;
+}
+
 int
 notmuch_config_command (void *ctx, int argc, char *argv[])
 {
@@ -823,6 +868,8 @@ notmuch_config_command (void *ctx, int argc, char *argv[])
 	    return 1;
 	}
 	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);
+    } else if (strcmp (argv[0], "list") == 0) {
+	return notmuch_config_command_list (ctx);
     }
 
     fprintf (stderr, "Unrecognized argument for notmuch config: %s\n",
diff --git a/test/config b/test/config
index 3bf8098..93ecb13 100755
--- a/test/config
+++ b/test/config
@@ -43,7 +43,6 @@ notmuch config set foo.nonexistent
 test_expect_equal "$(notmuch config get foo.nonexistent)" ""
 
 test_begin_subtest "List all items"
-test_subtest_known_broken
 notmuch config set database.path "/canonical/path"
 output=$(notmuch config list)
 test_expect_equal "$output" "\
-- 
1.7.4.4

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

* [PATCH v4 6/6] man: Document 'config list' command
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
                     ` (4 preceding siblings ...)
  2012-04-14  1:41   ` [PATCH v4 5/6] config: Add 'config list' command Peter Wang
@ 2012-04-14  1:41   ` Peter Wang
  2012-04-14  8:25   ` [PATCH v4 0/6] Config-related patches Mark Walters
  2012-04-14 19:32   ` Jameson Graef Rollins
  7 siblings, 0 replies; 36+ messages in thread
From: Peter Wang @ 2012-04-14  1:41 UTC (permalink / raw)
  To: notmuch

Document the 'config list' command and its output.
---
 man/man1/notmuch-config.1 |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-config.1 b/man/man1/notmuch-config.1
index 395cb9c..a55dbef 100644
--- a/man/man1/notmuch-config.1
+++ b/man/man1/notmuch-config.1
@@ -9,6 +9,8 @@ notmuch-config \- Access notmuch configuration file.
 .B notmuch config set
 .RI  "<" section ">.<" item "> [" value " ...]"
 
+.B notmuch config list
+
 .SH DESCRIPTION
 
 The
@@ -35,6 +37,18 @@ If no values are provided, the specified configuration item will be
 removed from the configuration file.
 .RE
 
+.RS 4
+.TP 4
+.B list
+Every configuration item is printed to stdout, each on a separate line
+of the form:
+
+.RI  "" section "." item "=" value
+
+No additional whitespace surrounds the dot or equals sign characters. In a
+multiple-value item (a list), the values are separated by semicolon characters.
+.RE
+
 The available configuration items are described below.
 
 .RS 4
-- 
1.7.4.4

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

* Re: [PATCH v4 0/6] Config-related patches
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
                     ` (5 preceding siblings ...)
  2012-04-14  1:41   ` [PATCH v4 6/6] man: Document " Peter Wang
@ 2012-04-14  8:25   ` Mark Walters
  2012-04-14 19:33     ` Jameson Graef Rollins
  2012-04-25  8:06     ` Mark Walters
  2012-04-14 19:32   ` Jameson Graef Rollins
  7 siblings, 2 replies; 36+ messages in thread
From: Mark Walters @ 2012-04-14  8:25 UTC (permalink / raw)
  To: Peter Wang, notmuch

On Sat, 14 Apr 2012, Peter Wang <novalazy@gmail.com> wrote:
> Changes from v3:
> - rephrase part of the 'list' implementation as a separate patch
> - test 'set' on an extant key
> - test removing keys

This looks good to me. +1

Two minor comments which you might like to consider (but definitely are
not required):
    1) You could check that there are no further arguments when the user
    calls `notmuch config list'
    2) In the man page you could explicitly say what the output is for a
    configuration item which has not been set.

Best wishes

Mark



    


> Peter Wang (6):
>   config: Fix free in 'config get' implementation.
>   config: Check 'config get' arity exactly
>   test: Add tests for 'config' command
>   test: Add broken test for 'config list'
>   config: Add 'config list' command
>   man: Document 'config list' command
>
>  man/man1/notmuch-config.1 |   14 +++++++++
>  notmuch-config.c          |   68 +++++++++++++++++++++++++++++++++++++++++---
>  test/config               |   60 +++++++++++++++++++++++++++++++++++++++
>  test/notmuch-test         |    1 +
>  4 files changed, 138 insertions(+), 5 deletions(-)
>  create mode 100755 test/config
>
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH v4 0/6] Config-related patches
  2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
                     ` (6 preceding siblings ...)
  2012-04-14  8:25   ` [PATCH v4 0/6] Config-related patches Mark Walters
@ 2012-04-14 19:32   ` Jameson Graef Rollins
  7 siblings, 0 replies; 36+ messages in thread
From: Jameson Graef Rollins @ 2012-04-14 19:32 UTC (permalink / raw)
  To: Peter Wang, notmuch

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

On Fri, Apr 13 2012, Peter Wang <novalazy@gmail.com> wrote:
> Changes from v3:
> - rephrase part of the 'list' implementation as a separate patch
> - test 'set' on an extant key
> - test removing keys

LGTM, tested, and +1.

jamie.

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

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

* Re: [PATCH v4 0/6] Config-related patches
  2012-04-14  8:25   ` [PATCH v4 0/6] Config-related patches Mark Walters
@ 2012-04-14 19:33     ` Jameson Graef Rollins
  2012-04-25  8:06     ` Mark Walters
  1 sibling, 0 replies; 36+ messages in thread
From: Jameson Graef Rollins @ 2012-04-14 19:33 UTC (permalink / raw)
  To: Mark Walters, Peter Wang, notmuch

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

On Sat, Apr 14 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> Two minor comments which you might like to consider (but definitely are
> not required):
>     1) You could check that there are no further arguments when the user
>     calls `notmuch config list'

This doesn't seem so necessary to me.  I'm fine if it just silently
ignores extra arguments.

>     2) In the man page you could explicitly say what the output is for a
>     configuration item which has not been set.

I'm also fine with it as it is.  I think it's implied that if the value
is null then the field is left blank.  That's ok.

jamie.

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

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

* Re: [PATCH v4 1/6] config: Fix free in 'config get' implementation.
  2012-04-14  1:41   ` [PATCH v4 1/6] config: Fix free in 'config get' implementation Peter Wang
@ 2012-04-25  2:29     ` David Bremner
  0 siblings, 0 replies; 36+ messages in thread
From: David Bremner @ 2012-04-25  2:29 UTC (permalink / raw)
  To: Peter Wang, notmuch

Peter Wang <novalazy@gmail.com> writes:

> The array returned by g_key_file_get_string_list() should be freed with
> g_strfreev(), not free().

Pushed this one patch from the series.

d

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

* Re: [PATCH v4 0/6] Config-related patches
  2012-04-14  8:25   ` [PATCH v4 0/6] Config-related patches Mark Walters
  2012-04-14 19:33     ` Jameson Graef Rollins
@ 2012-04-25  8:06     ` Mark Walters
  1 sibling, 0 replies; 36+ messages in thread
From: Mark Walters @ 2012-04-25  8:06 UTC (permalink / raw)
  To: Peter Wang, notmuch


On Sat, 14 Apr 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> On Sat, 14 Apr 2012, Peter Wang <novalazy@gmail.com> wrote:
>> Changes from v3:
>> - rephrase part of the 'list' implementation as a separate patch
>> - test 'set' on an extant key
>> - test removing keys
>
> This looks good to me. +1
>
> Two minor comments which you might like to consider (but definitely are
> not required):
>     1) You could check that there are no further arguments when the user
>     calls `notmuch config list'
>     2) In the man page you could explicitly say what the output is for a
>     configuration item which has not been set.
>
> Best wishes
>
> Mark

Just to make it completely clear the above was not meant to delay
acceptance of this series: I am quite happy with it as is.

Best wishes

Mark

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

* Re: [PATCH v4 2/6] config: Check 'config get' arity exactly
  2012-04-14  1:41   ` [PATCH v4 2/6] config: Check 'config get' arity exactly Peter Wang
@ 2012-04-28 13:51     ` David Bremner
  0 siblings, 0 replies; 36+ messages in thread
From: David Bremner @ 2012-04-28 13:51 UTC (permalink / raw)
  To: Peter Wang, notmuch

Peter Wang <novalazy@gmail.com> writes:

> Require that 'config get' is passed exactly one additional argument,
> instead of silently ignoring extra arguments. As a side-effect, produce
> more specific error messages for the 'config' command as a whole.

Hi Peter;

I pushed the rest of the series to master. Would you mind making patch
to NEWS describing the new feature?

thanks

d

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

end of thread, other threads:[~2012-04-28 13:51 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-20 22:31 [PATCH 1/2] config: Add 'config list' command Peter Wang
2012-03-20 22:31 ` [PATCH 2/2] man: Document " Peter Wang
2012-03-21 18:30 ` [PATCH 1/2] config: Add " Tomi Ollila
2012-03-21 22:52   ` Peter Wang
2012-03-26 15:03   ` Jameson Graef Rollins
2012-03-26 14:56 ` Jameson Graef Rollins
2012-03-30 23:15 ` [PATCH v2] " Peter Wang
2012-03-30 23:15   ` [PATCH v2 1/5] config: Fix free in 'config get' implementation Peter Wang
2012-03-30 23:15   ` [PATCH v2 2/5] test: Add tests for 'config' command Peter Wang
2012-03-31  7:45     ` Mark Walters
2012-03-31 21:47       ` Jameson Graef Rollins
2012-03-30 23:15   ` [PATCH v2 3/5] test: Add broken test for 'config list' Peter Wang
2012-03-30 23:15   ` [PATCH v2 4/5] config: Add 'config list' command Peter Wang
2012-03-30 23:15   ` [PATCH v2 5/5] man: Document " Peter Wang
2012-04-06  1:48 ` [PATCH v3 0/5] Add " Peter Wang
2012-04-06  1:48   ` [PATCH v3 1/5] config: Fix free in 'config get' implementation Peter Wang
2012-04-06  1:48   ` [PATCH v3 2/5] test: Add tests for 'config' command Peter Wang
2012-04-10  7:30     ` Jameson Graef Rollins
2012-04-06  1:48   ` [PATCH v3 3/5] test: Add broken test for 'config list' Peter Wang
2012-04-06  1:48   ` [PATCH v3 4/5] config: Add 'config list' command Peter Wang
2012-04-10  7:22     ` Jameson Graef Rollins
2012-04-10  8:31       ` Peter Wang
2012-04-06  1:48   ` [PATCH v3 5/5] man: Document " Peter Wang
2012-04-14  1:41 ` [PATCH v4 0/6] Config-related patches Peter Wang
2012-04-14  1:41   ` [PATCH v4 1/6] config: Fix free in 'config get' implementation Peter Wang
2012-04-25  2:29     ` David Bremner
2012-04-14  1:41   ` [PATCH v4 2/6] config: Check 'config get' arity exactly Peter Wang
2012-04-28 13:51     ` David Bremner
2012-04-14  1:41   ` [PATCH v4 3/6] test: Add tests for 'config' command Peter Wang
2012-04-14  1:41   ` [PATCH v4 4/6] test: Add broken test for 'config list' Peter Wang
2012-04-14  1:41   ` [PATCH v4 5/6] config: Add 'config list' command Peter Wang
2012-04-14  1:41   ` [PATCH v4 6/6] man: Document " Peter Wang
2012-04-14  8:25   ` [PATCH v4 0/6] Config-related patches Mark Walters
2012-04-14 19:33     ` Jameson Graef Rollins
2012-04-25  8:06     ` Mark Walters
2012-04-14 19:32   ` Jameson Graef Rollins

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).