[BUG] Minor: config list adds a semi-colon to lists with a single item until set

[BUG] Minor: config list adds a semi-colon to lists with a single item until set

[Ian]

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

It seems on first config some settings are listed by "config list" with a semicolon suffix even if they are a single item. If they are subsequently set to another/the same single value they will then appear in config list WITHOUT the semicolon suffix. See output below. This is a very minor inconsistency but has affected one downstream script: [https://github.com/mturquette/ghar-email/issues/6.](https://github.com/mturquette/ghar-email/issues/6)

---------------------------------------

$ notmuch config list
...
search.exclude_tags=spam

;

<-- Semi-colon
...
$ notmuch config get search.exclude_tags
spam
$ notmuch config

set

search.exclude_tags spam
$ notmuch config get search.exclude_tags
spam
$ notmuch config list
...
search.exclude_tags=spam                        <-- No semi-colon

---------------------------------------

[-- Attachment #2: Type: text/html, Size: 1023 bytes --]

Re: [BUG] Minor: config list adds a semi-colon to lists with a single item until set

[David Bremner]

Ian <saturns_rings@protonmail.com> writes:

> $ notmuch config list
> ...
> search.exclude_tags=spam
>
> ;
>
> <-- Semi-colon
> ...
> $ notmuch config get search.exclude_tags
> spam
> $ notmuch config
>
> set
>
> search.exclude_tags spam
> $ notmuch config get search.exclude_tags
> spam
> $ notmuch config list
> ...
> search.exclude_tags=spam                        <-- No semi-colon

Thanks for the report. It looks like two issues. The main one is that
"notmuch config list" is not sanitizing the input sufficiently, so if
extra ';' are added, they show in the list output. The second is that
notmuch setup is adding ; to single item lists.

d

PS. For better or worse, HTML is bound to get mangled this list.

[PATCH 1/2] test/setup: add known broken test for single items

[David Bremner]

In [1] Ian observed that notmuch setup was inconsistent with notmuch
config set when adding single items, namely adding an unneeded
semi-colon at the end. This test replicates that bug.

[1]: id:6O3LTUhoXlrnkPWCtPJCP4cagU7mFVEdyTpcC_37BoSzStlARXDBa7oczy6hB0jyjGjBQvgj_jFV58cw0aNx-jUg1h1O-FQ7820k68C0X4M=@protonmail.com
---
 test/T040-setup.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/T040-setup.sh b/test/T040-setup.sh
index 42c621c8..36a42562 100755
--- a/test/T040-setup.sh
+++ b/test/T040-setup.sh
@@ -23,6 +23,14 @@ EOF
 expected_dir=$NOTMUCH_SRCDIR/test/setup.expected-output
 test_expect_equal_file ${expected_dir}/config-with-comments new-notmuch-config
 
+test_begin_subtest "setup consistent with config-set for single items"
+test_subtest_known_broken
+# note this relies on the config state from the previous test.
+notmuch --config=new-notmuch-config config list > list.setup
+notmuch --config=new-notmuch-config config set search.exclude_tags baz
+notmuch --config=new-notmuch-config config list > list.config
+test_expect_equal_file list.setup list.config
+
 test_begin_subtest "notmuch with a config but without a database suggests notmuch new"
 notmuch 2>&1 | notmuch_dir_sanitize > OUTPUT
 cat <<EOF > EXPECTED
-- 
2.34.1

[PATCH 2/2] CLI/setup: special case single item lists

[David Bremner]

This matches the heuristic used by "notmuch config set" to decide if
something is a list.

This change fixes the bug reported at [1].

[1]: id:6O3LTUhoXlrnkPWCtPJCP4cagU7mFVEdyTpcC_37BoSzStlARXDBa7oczy6hB0jyjGjBQvgj_jFV58cw0aNx-jUg1h1O-FQ7820k68C0X4M=@protonmail.com
---
 notmuch-config.c                                | 5 ++++-
 test/T040-setup.sh                              | 1 -
 test/setup.expected-output/config-with-comments | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 11d8d68b..e9456d79 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -383,7 +383,10 @@ _config_set_list (notmuch_conffile_t *config,
 		  const char *list[],
 		  size_t length)
 {
-    g_key_file_set_string_list (config->key_file, group, key, list, length);
+    if (length > 1)
+	g_key_file_set_string_list (config->key_file, group, key, list, length);
+    else
+	g_key_file_set_string (config->key_file, group, key, list[0]);
 }
 
 void
diff --git a/test/T040-setup.sh b/test/T040-setup.sh
index 36a42562..10b29ec3 100755
--- a/test/T040-setup.sh
+++ b/test/T040-setup.sh
@@ -24,7 +24,6 @@ expected_dir=$NOTMUCH_SRCDIR/test/setup.expected-output
 test_expect_equal_file ${expected_dir}/config-with-comments new-notmuch-config
 
 test_begin_subtest "setup consistent with config-set for single items"
-test_subtest_known_broken
 # note this relies on the config state from the previous test.
 notmuch --config=new-notmuch-config config list > list.setup
 notmuch --config=new-notmuch-config config set search.exclude_tags baz
diff --git a/test/setup.expected-output/config-with-comments b/test/setup.expected-output/config-with-comments
index 56c628e5..d8397714 100644
--- a/test/setup.expected-output/config-with-comments
+++ b/test/setup.expected-output/config-with-comments
@@ -31,7 +31,7 @@ path=/path/to/maildir
 [user]
 name=Test Suite
 primary_email=test.suite@example.com
-other_email=another.suite@example.com;
+other_email=another.suite@example.com
 
 # Configuration for "notmuch new"
 #
@@ -60,7 +60,7 @@ tags=foo;bar;
 #		query will override that exclusion.
 #
 [search]
-exclude_tags=baz;
+exclude_tags=baz
 
 # Maildir compatibility configuration
 #
-- 
2.34.1

Re: [PATCH 2/2] CLI/setup: special case single item lists

[David Bremner]

David Bremner <david@tethera.net> writes:

> This matches the heuristic used by "notmuch config set" to decide if
> something is a list.
>
> This change fixes the bug reported at [1].
>
> [1]: id:6O3LTUhoXlrnkPWCtPJCP4cagU7mFVEdyTpcC_37BoSzStlARXDBa7oczy6hB0jyjGjBQvgj_jFV58cw0aNx-jUg1h1O-FQ7820k68C0X4M=@protonmail.com

Series applied to master.

Re: [BUG] Minor: config list adds a semi-colon to lists with a single item until set

[David Bremner]

Ian <saturns_rings@protonmail.com> writes:

> It seems on first config some settings are listed by "config list" with a semicolon suffix even if they are a single item. If they are subsequently set to another/the same single value they will then appear in config list WITHOUT the semicolon suffix. See output below. This is a very minor inconsistency but has affected one downstream script: [https://github.com/mturquette/ghar-email/issues/6.](https://github.com/mturquette/ghar-email/issues/6)

This should be fixed as of commit 88633bc7a79d47ba96879f698ec2267f2d3f6766

Re: [BUG] Minor: config list adds a semi-colon to lists with a single item until set

[Ian]

Thanks David

Sent with ProtonMail secure email.

------- Original Message -------

On Monday, January 24th, 2022 at 13:37, David Bremner <david@tethera.net> wrote:

> Ian saturns_rings@protonmail.com writes:
>
> > It seems on first config some settings are listed by "config list" with a semicolon suffix even if they are a single item. If they are subsequently set to another/the same single value they will then appear in config list WITHOUT the semicolon suffix. See output below. This is a very minor inconsistency but has affected one downstream script: https://github.com/mturquette/ghar-email/issues/6.
>
> This should be fixed as of commit 88633bc7a79d47ba96879f698ec2267f2d3f6766