[PATCH] doc: add support for adding configure options as Sphinx tags

2019-03-19 Thread Jani Nikula
Add the configure options specified in $(TAGS) that equal 1 as tags on
the Sphinx command line using the -t option. The tags may be used to
conditionally include documentation using the Sphinx "only" directive
[1].

As an example, indicate in the documentation whether the Xapian field
processor is likely to be available (assuming the notmuch binary was
built in the same environment as the documentation).

[1] 
http://www.sphinx-doc.org/en/stable/markup/misc.html#including-content-based-on-tags

---

This is something I wrote two years ago, ISTR David asking about
something like this on IRC, but I've already forgotten what it was
about...
---
 doc/Makefile.local| 7 ++-
 doc/man7/notmuch-search-terms.rst | 8 
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/doc/Makefile.local b/doc/Makefile.local
index bab3d0d286ce..95df9ae82b4f 100644
--- a/doc/Makefile.local
+++ b/doc/Makefile.local
@@ -7,8 +7,13 @@ SPHINXOPTS:= -q
 SPHINXBUILD   = sphinx-build
 DOCBUILDDIR  := $(dir)/_build
 
+# Configure options to be added as Sphinx tags.
+# Add "-t " for each make variable in TAGS that equals 1.
+TAGS := HAVE_XAPIAN_FIELD_PROCESSOR HAVE_XAPIAN_COMPACT
+TAGOPTS := $(patsubst %=1,-t %,$(filter %=1,$(foreach 
tag,$(TAGS),$(tag)=$(value $(tag)
+
 # Internal variables.
-ALLSPHINXOPTS   := -d $(DOCBUILDDIR)/doctrees $(SPHINXOPTS) $(srcdir)/$(dir)
+ALLSPHINXOPTS   := -d $(DOCBUILDDIR)/doctrees $(SPHINXOPTS) $(TAGOPTS) 
$(srcdir)/$(dir)
 APIMAN := $(DOCBUILDDIR)/man/man3/notmuch.3
 DOXYFILE   := $(srcdir)/$(dir)/doxygen.cfg
 
diff --git a/doc/man7/notmuch-search-terms.rst 
b/doc/man7/notmuch-search-terms.rst
index f7a39ceb9df4..4840f03743aa 100644
--- a/doc/man7/notmuch-search-terms.rst
+++ b/doc/man7/notmuch-search-terms.rst
@@ -451,6 +451,14 @@ notmuch was built against a sufficiently recent version of 
Xapian by running
 
   % notmuch config get built_with.field_processor
 
+.. only:: HAVE_XAPIAN_FIELD_PROCESSOR
+
+   The documentation was built on a host with field processor support.
+
+.. only:: not HAVE_XAPIAN_FIELD_PROCESSOR
+
+   The documentation was built on a host without field processor support.
+
 Currently the following features require field processor support:
 
 - non-range date queries, e.g. "date:today"
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] python: fix threads.__str__ automethod documentation

2019-02-11 Thread Jani Nikula
Indent the directive properly to attach it to Threads autoclass
documentation.

Fixes:

WARNING: don't know which module to import for autodocumenting
'__str__' (try placing a "module" or "currentmodule" directive in the
document, or giving an explicit module name)
---
 bindings/python/docs/source/threads.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bindings/python/docs/source/threads.rst 
b/bindings/python/docs/source/threads.rst
index 4324ac82a389..46ce5be5412a 100644
--- a/bindings/python/docs/source/threads.rst
+++ b/bindings/python/docs/source/threads.rst
@@ -11,4 +11,4 @@
   iterator and broke list(Threads()). Use `len(list(msgs))`
   instead.
 
-.. automethod:: __str__
+   .. automethod:: __str__
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/3] python: fix documentation title underline

2019-02-11 Thread Jani Nikula
Fix documentation build sphinx warning:

filesystem.rst:18: WARNING: Title underline too short.
---
 bindings/python/docs/source/filesystem.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bindings/python/docs/source/filesystem.rst 
b/bindings/python/docs/source/filesystem.rst
index 558c93de592a..13fe11946c46 100644
--- a/bindings/python/docs/source/filesystem.rst
+++ b/bindings/python/docs/source/filesystem.rst
@@ -15,7 +15,7 @@ Files and directories
   instead.
 
 :class:`Directory` -- A directory entry in the database
---
+---
 
 .. autoclass:: Directory
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/3] python: fix documentation build with python 3.7

2019-02-11 Thread Jani Nikula
From: Jani Nikula 

The simplistic mocking in conf.py falls short on python 3.7. Just use
unittest.mock instead.

Fixes:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/sphinx/config.py", line 368, in 
eval_config_file
execfile_(filename, namespace)
  File "/usr/lib/python3/dist-packages/sphinx/util/pycompat.py", line 150, in 
execfile_
exec_(code, _globals)
  File "/path/to/notmuch/bindings/python/docs/source/conf.py", line 39, in 

from notmuch import __VERSION__,__AUTHOR__
  File "/path/to/notmuch/bindings/python/notmuch/__init__.py", line 54, in 

from .database import Database
  File "/path/to/notmuch/bindings/python/notmuch/database.py", line 25, in 

from .globals import (
  File "/path/to/notmuch/bindings/python/notmuch/globals.py", line 48, in 

class NotmuchDatabaseS(Structure):
TypeError: __mro_entries__ must return a tuple
---
 bindings/python/docs/source/conf.py | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/bindings/python/docs/source/conf.py 
b/bindings/python/docs/source/conf.py
index 5b901c4ec4d8..8b43c5ca3f9f 100644
--- a/bindings/python/docs/source/conf.py
+++ b/bindings/python/docs/source/conf.py
@@ -13,22 +13,13 @@
 
 import sys, os
 
+from unittest.mock import Mock
+
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 sys.path.insert(0,os.path.abspath('../..'))
 
-class Mock(object):
-def __init__(self, *args, **kwargs):
-pass
-
-def __call__(self, *args, **kwargs):
-return Mock()
-
-@classmethod
-def __getattr__(self, name):
-return Mock() if name not in ('__file__', '__path__') else '/dev/null'
-
 MOCK_MODULES = [
 'ctypes',
 ]
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [Bug] Cannot build python3 bindings documentation of notmuch 0.28.1

2019-02-10 Thread Jani Nikula
On Wed, 06 Feb 2019, Dan Čermák  wrote:
> Hi list,
>
> the python3 bindings documentation seems to be broken:
>
> on OpenSUSE Tumbleweed and Fedora 29 (both have Python 3.7 and sphinx
> 1.7.6) I get this:

Looks like a change in Python 3.7 makes the ctypes mocking in
bindings/python/docs/source/conf.py fall short. If you just remove
'ctypes' from MOCK_MODULES, it'll work.

However, that would probably break the documentation build at
readthedocs.org, which won't have libnotmuch around.

Short term, I guess someone(tm) has to hack the mocking.

Long term, I'd love to see the python bindings Sphinx documentation
merged to the main Sphinx notmuch documentation, and have all of that
get automatically built and updated to notmuchmail.org.


BR,
Jani.


>
> $ cd bindings/python/docs/
> $ make dirhtml
> sphinx-build -b dirhtml -d build/doctrees   source build/dirhtml
> Running Sphinx v1.7.6
>
> Configuration error:
> There is a programable error in your configuration file:
>
> Traceback (most recent call last):
>   File "/usr/lib/python3.7/site-packages/sphinx/config.py", line 161, in 
> __init__
> execfile_(filename, config)
>   File "/usr/lib/python3.7/site-packages/sphinx/util/pycompat.py", line 150, 
> in execfile_
> exec_(code, _globals)
>   File "conf.py", line 39, in 
> from notmuch import __VERSION__,__AUTHOR__
>   File 
> "/home/dan/packages/git.notmuchmail.org/git/notmuch/bindings/python/notmuch/__init__.py",
>  line 54, in 
> from .database import Database
>   File 
> "/home/dan/packages/git.notmuchmail.org/git/notmuch/bindings/python/notmuch/database.py",
>  line 25, in 
> from .globals import (
>   File 
> "/home/dan/packages/git.notmuchmail.org/git/notmuch/bindings/python/notmuch/globals.py",
>  line 48, in 
> class NotmuchDatabaseS(Structure):
> TypeError: __mro_entries__ must return a tuple
>
> make: *** [Makefile:38: dirhtml] Error 2
>
>
> Cheers,
>
> Dan
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 1/4] test: dynamically generate parser tests

2019-01-27 Thread Jani Nikula


Apologies for the noise, I sent the entire series to the wrong list. :(

BR,
Jani.

On Mon, 28 Jan 2019, Jani Nikula  wrote:
> It's impossible to have expected failures or other unittest decorators
> at subtest granularity. They only work at the test method level. On the
> other hand we don't want to be manually adding test methods when all of
> the tests are defined in terms of input files and expected results.
>
> Generate test methods dynamically from the input files, and assign to
> the test class. Running code at import time to do this is less than
> stellar, but it needs to be done early to have unittest test discovery
> find the methods.
>
> The alternative would be to add a load_tests protocol function [1], but
> that seems like more boilerplate. Can be added later as needed.
>
> Finally, one massive upside to this is the ability to run individual
> named testcases. For example, to test enum.c and typedef-enum.c, use:
>
> $ test/test_hawkmoth.py ParserTest.test_enum ParserTest.test_typedef_enum
>
> [1] https://docs.python.org/3/library/unittest.html#load-tests-protocol
> ---
>  test/test_hawkmoth.py | 26 +++---
>  test/testenv.py   | 29 +
>  2 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/test/test_hawkmoth.py b/test/test_hawkmoth.py
> index 1fe02efc004d..75eebbe35eef 100755
> --- a/test/test_hawkmoth.py
> +++ b/test/test_hawkmoth.py
> @@ -8,28 +8,16 @@ import unittest
>  import testenv
>  from hawkmoth import hawkmoth
>  
> -class ParserTest(unittest.TestCase):
> -def _run_test(self, input_filename):
> -# sanity check
> -self.assertTrue(os.path.isfile(input_filename))
> -
> -options = testenv.get_testcase_options(input_filename)
> -output = hawkmoth.parse_to_string(input_filename, False, **options)
> -expected = testenv.read_file(input_filename, ext='stdout')
> +def _get_output(input_filename, **options):
> +return hawkmoth.parse_to_string(input_filename, False, **options)
>  
> -self.assertEqual(expected, output)
> +def _get_expected(input_filename, **options):
> +return testenv.read_file(input_filename, ext='stdout')
>  
> -def _run_dir(self, path):
> -# sanity check
> -self.assertTrue(os.path.isdir(path))
> -
> -with self.subTest(path=path):
> -for f in testenv.get_testcases(path):
> -with self.subTest(source=os.path.basename(f)):
> -self._run_test(f)
> +class ParserTest(unittest.TestCase):
> +pass
>  
> -def test_parser(self):
> -self._run_dir(testenv.testdir)
> +testenv.assign_test_methods(ParserTest, _get_output, _get_expected)
>  
>  if __name__ == '__main__':
>  unittest.main()
> diff --git a/test/testenv.py b/test/testenv.py
> index f026aead8c07..cc80ef2218ed 100644
> --- a/test/testenv.py
> +++ b/test/testenv.py
> @@ -3,6 +3,7 @@
>  
>  import sys
>  import os
> +import unittest
>  
>  testext = '.c'
>  testdir = os.path.dirname(os.path.abspath(__file__))
> @@ -10,6 +11,16 @@ rootdir = os.path.dirname(testdir)
>  
>  sys.path.insert(0, rootdir)
>  
> +def _testcase_name(testcase):
> +"""Convert a testcase filename into a test case identifier."""
> +name = os.path.splitext(os.path.basename(testcase))[0]
> +name = name.replace('-', '_')
> +name = 'test_{name}'.format(name=name)
> +
> +assert name.isidentifier()
> +
> +return name
> +
>  def get_testcases(path):
>  for f in sorted(os.listdir(path)):
>  if f.endswith(testext):
> @@ -52,3 +63,21 @@ def read_file(filename, **kwargs):
>  expected = file.read()
>  
>  return expected
> +
> +def _test_generator(get_output, get_expected, input_filename, **options):
> +"""Return a function that compares output/expected results on 
> input_filename."""
> +def test(self):
> +output = get_output(input_filename, **options)
> +expected = get_expected(input_filename, **options)
> +
> +self.assertEqual(expected, output)
> +
> +return test
> +
> +def assign_test_methods(cls, get_output, get_expected):
> +"""Assign test case functions to the given class."""
> +for f in get_testcases(testdir):
> +options = get_testcase_options(f)
> +method = _test_generator(get_output, get_expected, f, **options)
> +
> +setattr(cls, _testcase_name(f), method)
> -- 
> 2.20.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 4/4] test: use html builder for directive tests

2019-01-27 Thread Jani Nikula
Slower but does not lose information as much, providing more accurate
results. Switch to the basic template for speed.
---
 test/conf.py  | 2 +-
 test/test_cautodoc.py | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/conf.py b/test/conf.py
index 6d36327df085..d4c3d34c8b3b 100644
--- a/test/conf.py
+++ b/test/conf.py
@@ -75,7 +75,7 @@ pygments_style = None
 # The theme to use for HTML and HTML Help pages.  See the documentation for
 # a list of builtin themes.
 #
-html_theme = 'alabaster'
+html_theme = 'basic'
 
 # Theme options are theme-specific and customize the look and feel of a theme
 # further.  For a list of options available for each theme, see the
diff --git a/test/test_cautodoc.py b/test/test_cautodoc.py
index 0a8e5bb2815f..ad54cae2e98e 100755
--- a/test/test_cautodoc.py
+++ b/test/test_cautodoc.py
@@ -9,7 +9,7 @@ import unittest
 import testenv
 from sphinx_testing import with_app
 
-@with_app(confdir=testenv.testdir, create_new_srcdir=True, buildername='text')
+@with_app(confdir=testenv.testdir, create_new_srcdir=True, buildername='html')
 def _get_output(input_filename, app, status, warning, **options):
 shutil.copyfile(input_filename,
 testenv.modify_filename(input_filename, dir=app.srcdir))
@@ -23,16 +23,16 @@ def _get_output(input_filename, app, status, warning, 
**options):
 
 app.build()
 
-return testenv.read_file(os.path.join(app.outdir, 'index.txt'))
+return testenv.read_file(os.path.join(app.outdir, 'index.html'))
 
-@with_app(confdir=testenv.testdir, create_new_srcdir=True, buildername='text')
+@with_app(confdir=testenv.testdir, create_new_srcdir=True, buildername='html')
 def _get_expected(input_filename, app, status, warning, **options):
 shutil.copyfile(testenv.modify_filename(input_filename, ext='stdout'),
 os.path.join(app.srcdir, 'index.rst'))
 
 app.build()
 
-return testenv.read_file(os.path.join(app.outdir, 'index.txt'))
+return testenv.read_file(os.path.join(app.outdir, 'index.html'))
 
 class DirectiveTest(unittest.TestCase):
 pass
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 3/4] test: add support for flagging expected failures in testcase options

2019-01-27 Thread Jani Nikula
Since our tests are dynamically created, we also need to decorate
expected failures dynamically. Use the testcase options file as the
source. Only pass known directive options to the directive in the
cautodoc test.

Add a meta test to verify this works, with result "OK (expected
failures=2)".
---
 test/meta-expected-failure.c   | 3 +++
 test/meta-expected-failure.options | 1 +
 test/meta-expected-failure.stdout  | 3 +++
 test/test_cautodoc.py  | 2 +-
 test/testenv.py| 8 
 5 files changed, 16 insertions(+), 1 deletion(-)
 create mode 100644 test/meta-expected-failure.c
 create mode 100644 test/meta-expected-failure.options
 create mode 100644 test/meta-expected-failure.stdout

diff --git a/test/meta-expected-failure.c b/test/meta-expected-failure.c
new file mode 100644
index ..18065136572f
--- /dev/null
+++ b/test/meta-expected-failure.c
@@ -0,0 +1,3 @@
+/**
+ * Meta test: This fails. Always.
+ */
diff --git a/test/meta-expected-failure.options 
b/test/meta-expected-failure.options
new file mode 100644
index ..68c905946fce
--- /dev/null
+++ b/test/meta-expected-failure.options
@@ -0,0 +1 @@
+test-expected-failure
diff --git a/test/meta-expected-failure.stdout 
b/test/meta-expected-failure.stdout
new file mode 100644
index ..dbd71dcb308f
--- /dev/null
+++ b/test/meta-expected-failure.stdout
@@ -0,0 +1,3 @@
+
+Meta test: This fails.
+
diff --git a/test/test_cautodoc.py b/test/test_cautodoc.py
index 848f2105a1da..0a8e5bb2815f 100755
--- a/test/test_cautodoc.py
+++ b/test/test_cautodoc.py
@@ -17,7 +17,7 @@ def _get_output(input_filename, app, status, warning, 
**options):
 with open(os.path.join(app.srcdir, 'index.rst'), 'w') as file:
 fmt = '.. c:autodoc:: {source}\n'
 file.write(fmt.format(source=os.path.basename(input_filename)))
-for key in options.keys():
+for key in [k for k in options.keys() if k in 
testenv.directive_options]:
 fmt = '   :{key}: {value}\n'
 file.write(fmt.format(key=key, value=options[key]))
 
diff --git a/test/testenv.py b/test/testenv.py
index cc80ef2218ed..b6842a81b375 100644
--- a/test/testenv.py
+++ b/test/testenv.py
@@ -26,6 +26,11 @@ def get_testcases(path):
 if f.endswith(testext):
 yield os.path.join(path, f)
 
+directive_options = [
+'compat',
+'clang',
+]
+
 def get_testcase_options(testcase):
 options_filename = modify_filename(testcase, ext='options')
 
@@ -80,4 +85,7 @@ def assign_test_methods(cls, get_output, get_expected):
 options = get_testcase_options(f)
 method = _test_generator(get_output, get_expected, f, **options)
 
+if options.get('test-expected-failure') is not None:
+method = unittest.expectedFailure(method)
+
 setattr(cls, _testcase_name(f), method)
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 2/4] test: dynamically generate directive tests

2019-01-27 Thread Jani Nikula
Similar to the parser test transition to dynamically generate test
methods, but with more test separation added. After this, the Sphinx
build is done independently for each test case, and separately for the
actual directive output and the expected output. This removes any
potential C domain interactions between test cases and expected/actual
inputs.

The change does mean the Sphinx build is run roughly 50x times per full
test run, at the current number of test cases. This is somewhat offset
by the ability to run individual directive test cases:

$ test/test_cautodoc.py DirectiveTest.test_example_10_macro
---
 test/{sphinx => }/conf.py |  0
 test/sphinx/index.rst | 20 -
 test/test_cautodoc.py | 61 ++-
 3 files changed, 21 insertions(+), 60 deletions(-)
 rename test/{sphinx => }/conf.py (100%)
 delete mode 100644 test/sphinx/index.rst

diff --git a/test/sphinx/conf.py b/test/conf.py
similarity index 100%
rename from test/sphinx/conf.py
rename to test/conf.py
diff --git a/test/sphinx/index.rst b/test/sphinx/index.rst
deleted file mode 100644
index 20404ce8ee5e..
--- a/test/sphinx/index.rst
+++ /dev/null
@@ -1,20 +0,0 @@
-.. Hawkmoth Test documentation master file, created by
-   sphinx-quickstart on Wed Dec 12 13:11:26 2018.
-   You can adapt this file completely to your liking, but it should at least
-   contain the root `toctree` directive.
-
-Welcome to Hawkmoth Test's documentation!
-=
-
-.. toctree::
-   :maxdepth: 2
-   :caption: Contents:
-
-
-
-Indices and tables
-==
-
-* :ref:`genindex`
-* :ref:`modindex`
-* :ref:`search`
diff --git a/test/test_cautodoc.py b/test/test_cautodoc.py
index 3fb98af8629d..848f2105a1da 100755
--- a/test/test_cautodoc.py
+++ b/test/test_cautodoc.py
@@ -9,54 +9,35 @@ import unittest
 import testenv
 from sphinx_testing import with_app
 
-class DirectiveTest(unittest.TestCase):
-
-def _setup_src(self, srcdir, testcase_in):
-testcase_out = testenv.modify_filename(testcase_in, dir=srcdir)
-
-# use the pre-generated rst as comparison data
-shutil.copyfile(testenv.modify_filename(testcase_in, ext='stdout'),
-testenv.modify_filename(testcase_out, 
ext='expected.rst'))
-
-# set up an rst file to run the extension
-shutil.copyfile(testcase_in, testcase_out)
-options = testenv.get_testcase_options(testcase_in)
+@with_app(confdir=testenv.testdir, create_new_srcdir=True, buildername='text')
+def _get_output(input_filename, app, status, warning, **options):
+shutil.copyfile(input_filename,
+testenv.modify_filename(input_filename, dir=app.srcdir))
 
-with open(testenv.modify_filename(testcase_out, ext='output.rst'), 
'w') as file:
-fmt = '.. c:autodoc:: {source}\n'
-file.write(fmt.format(source=os.path.basename(testcase_out)))
-for key in options.keys():
-fmt = '   :{key}: {value}\n'
-file.write(fmt.format(key=key, value=options[key]))
+with open(os.path.join(app.srcdir, 'index.rst'), 'w') as file:
+fmt = '.. c:autodoc:: {source}\n'
+file.write(fmt.format(source=os.path.basename(input_filename)))
+for key in options.keys():
+fmt = '   :{key}: {value}\n'
+file.write(fmt.format(key=key, value=options[key]))
 
-def _check_out(self, outdir, testcase_in):
-testcase_out = testenv.modify_filename(testcase_in, dir=outdir)
+app.build()
 
-# compare output from the pre-generated rst against the output 
generated
-# by the extension
+return testenv.read_file(os.path.join(app.outdir, 'index.txt'))
 
-output = testenv.read_file(testenv.modify_filename(testcase_out,
-   ext='output.txt'))
+@with_app(confdir=testenv.testdir, create_new_srcdir=True, buildername='text')
+def _get_expected(input_filename, app, status, warning, **options):
+shutil.copyfile(testenv.modify_filename(input_filename, ext='stdout'),
+os.path.join(app.srcdir, 'index.rst'))
 
-expected = testenv.read_file(testenv.modify_filename(testcase_out,
- 
ext='expected.txt'))
+app.build()
 
-self.assertEqual(expected, output)
+return testenv.read_file(os.path.join(app.outdir, 'index.txt'))
 
-# Use copy_srcdir_to_tmpdir=False and outdir='some-dir' for debugging
-@with_app(srcdir=os.path.join(testenv.testdir, 'sphinx'),
-  buildername='text', copy_srcdir_to_tmpdir=True)
-def test_directive(self, app, status, warning):
-testcases = list(testenv.get_testcases(testenv.testdir))
-
-for f in testcases:
-self._setup_src(app.srcdir, f)
-
-app.build()
+class DirectiveTest(unittest.TestCase):
+pass
 
-for f in testcases:
- 

[PATCH v2 1/4] test: dynamically generate parser tests

2019-01-27 Thread Jani Nikula
It's impossible to have expected failures or other unittest decorators
at subtest granularity. They only work at the test method level. On the
other hand we don't want to be manually adding test methods when all of
the tests are defined in terms of input files and expected results.

Generate test methods dynamically from the input files, and assign to
the test class. Running code at import time to do this is less than
stellar, but it needs to be done early to have unittest test discovery
find the methods.

The alternative would be to add a load_tests protocol function [1], but
that seems like more boilerplate. Can be added later as needed.

Finally, one massive upside to this is the ability to run individual
named testcases. For example, to test enum.c and typedef-enum.c, use:

$ test/test_hawkmoth.py ParserTest.test_enum ParserTest.test_typedef_enum

[1] https://docs.python.org/3/library/unittest.html#load-tests-protocol
---
 test/test_hawkmoth.py | 26 +++---
 test/testenv.py   | 29 +
 2 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/test/test_hawkmoth.py b/test/test_hawkmoth.py
index 1fe02efc004d..75eebbe35eef 100755
--- a/test/test_hawkmoth.py
+++ b/test/test_hawkmoth.py
@@ -8,28 +8,16 @@ import unittest
 import testenv
 from hawkmoth import hawkmoth
 
-class ParserTest(unittest.TestCase):
-def _run_test(self, input_filename):
-# sanity check
-self.assertTrue(os.path.isfile(input_filename))
-
-options = testenv.get_testcase_options(input_filename)
-output = hawkmoth.parse_to_string(input_filename, False, **options)
-expected = testenv.read_file(input_filename, ext='stdout')
+def _get_output(input_filename, **options):
+return hawkmoth.parse_to_string(input_filename, False, **options)
 
-self.assertEqual(expected, output)
+def _get_expected(input_filename, **options):
+return testenv.read_file(input_filename, ext='stdout')
 
-def _run_dir(self, path):
-# sanity check
-self.assertTrue(os.path.isdir(path))
-
-with self.subTest(path=path):
-for f in testenv.get_testcases(path):
-with self.subTest(source=os.path.basename(f)):
-self._run_test(f)
+class ParserTest(unittest.TestCase):
+pass
 
-def test_parser(self):
-self._run_dir(testenv.testdir)
+testenv.assign_test_methods(ParserTest, _get_output, _get_expected)
 
 if __name__ == '__main__':
 unittest.main()
diff --git a/test/testenv.py b/test/testenv.py
index f026aead8c07..cc80ef2218ed 100644
--- a/test/testenv.py
+++ b/test/testenv.py
@@ -3,6 +3,7 @@
 
 import sys
 import os
+import unittest
 
 testext = '.c'
 testdir = os.path.dirname(os.path.abspath(__file__))
@@ -10,6 +11,16 @@ rootdir = os.path.dirname(testdir)
 
 sys.path.insert(0, rootdir)
 
+def _testcase_name(testcase):
+"""Convert a testcase filename into a test case identifier."""
+name = os.path.splitext(os.path.basename(testcase))[0]
+name = name.replace('-', '_')
+name = 'test_{name}'.format(name=name)
+
+assert name.isidentifier()
+
+return name
+
 def get_testcases(path):
 for f in sorted(os.listdir(path)):
 if f.endswith(testext):
@@ -52,3 +63,21 @@ def read_file(filename, **kwargs):
 expected = file.read()
 
 return expected
+
+def _test_generator(get_output, get_expected, input_filename, **options):
+"""Return a function that compares output/expected results on 
input_filename."""
+def test(self):
+output = get_output(input_filename, **options)
+expected = get_expected(input_filename, **options)
+
+self.assertEqual(expected, output)
+
+return test
+
+def assign_test_methods(cls, get_output, get_expected):
+"""Assign test case functions to the given class."""
+for f in get_testcases(testdir):
+options = get_testcase_options(f)
+method = _test_generator(get_output, get_expected, f, **options)
+
+setattr(cls, _testcase_name(f), method)
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] cli: Support --include-html and --body=false for --format=text

2018-10-25 Thread Jani Nikula
On Thu, 25 Oct 2018, ma...@kakoune.org wrote:
> text format is convenient for shell based parsing of notmuch output,
> adding support for --include-html and --body=false improves its
> usefulness to write complex shell based scripts.

It's debatable whether parsing the --format=text output correctly is
convenient or not. Particularly for notmuch show I think the text format
output is basically legacy, and I don't think we're all that fond of
adding new features to it.

In any case, this patch has two independent changes in one, and should
be split.

BR,
Jani.


> ---
>   NEWS  |  9 +
>   doc/man1/notmuch-show.rst | 15 ---
>   notmuch-show.c| 20 +++-
>   test/T190-multipart.sh| 24 
>   4 files changed, 56 insertions(+), 12 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index ca3ba99e..6d7e7162 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,3 +1,12 @@
> +Notmuch 0.29 (UNRELEASED)
> +=
> +
> +Command Line Interface
> +--
> +
> +`notmuch show` now supports --include-html and --body=false with
> +--format=text
> +
>   Notmuch 0.28 (2018-10-12)
>   =
>
> diff --git a/doc/man1/notmuch-show.rst b/doc/man1/notmuch-show.rst
> index 8bfa87c6..a2708a04 100644
> --- a/doc/man1/notmuch-show.rst
> +++ b/doc/man1/notmuch-show.rst
> @@ -176,18 +176,19 @@ Supported options for **show** include
>   ``--body=(true|false)``
>   If true (the default) **notmuch show** includes the bodies of the
>   messages in the output; if false, bodies are omitted.
> -``--body=false`` is only implemented for the json and sexp formats
> -and it is incompatible with ``--part > 0.``
> +``--body=false`` is only implemented for the text, json and sexp
> +formats and it is incompatible with ``--part > 0.``
>
>   This is useful if the caller only needs the headers as body-less
>   output is much faster and substantially smaller.
>
>   ``--include-html``
> -Include "text/html" parts as part of the output (currently only
> -supported with ``--format=json`` and ``--format=sexp``). By 
> default,
> -unless ``--part=N`` is used to select a specific part or
> -``--include-html`` is used to include all "text/html" parts, no
> -part with content type "text/html" is included in the output.
> +Include "text/html" parts as part of the output (currently
> +only supported with ``--format=text``, ``--format=json`` and
> +``--format=sexp``). By default, unless ``--part=N`` is used to
> +select a specific part or ``--include-html`` is used to include all
> +"text/html" parts, no part with content type "text/html" is 
> included
> +in the output.
>
>   A common use of **notmuch show** is to display a single thread of email
>   messages. For this, use a search term of "thread:" as can be
> diff --git a/notmuch-show.c b/notmuch-show.c
> index c3a3783a..07e9a5db 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -574,12 +574,18 @@ format_part_text (const void *ctx, sprinter_t *sp, 
> mime_node_t *node,
>   g_mime_stream_printf (stream, "Date: %s\n", date_string);
>   g_mime_stream_printf (stream, "\fheader}\n");
>
> + if (!params->output_body)
> + {
> + g_mime_stream_printf (stream, "\f%s}\n", part_type);
> + return NOTMUCH_STATUS_SUCCESS;
> + }
>   g_mime_stream_printf (stream, "\fbody{\n");
>   }
>
>   if (leaf) {
>   if (g_mime_content_type_is_type (content_type, "text", "*") &&
> - !g_mime_content_type_is_type (content_type, "text", "html"))
> + (params->include_html ||
> +  ! g_mime_content_type_is_type (content_type, "text", "html")))
>   {
>   show_text_part_content (node->part, stream, 0);
>   } else {
> @@ -1204,15 +1210,19 @@ notmuch_show_command (notmuch_config_t *config, 
> int argc, char *argv[])
>   fprintf (stderr, "Warning: --body=false is incompatible with 
> --part > 0. Disabling.\n");
>   params.output_body = true;
>   } else {
> - if (format != NOTMUCH_FORMAT_JSON && format != 
> NOTMUCH_FORMAT_SEXP)
> + if (format != NOTMUCH_FORMAT_TEXT &&
> + format != NOTMUCH_FORMAT_JSON &&
> + format != NOTMUCH_FORMAT_SEXP)
>   fprintf (stderr,
> -  "Warning: --body=false only implemented for 
> format=json and 
> format=sexp\n");
> +  "Warning: --body=false only implemented for 
> format=text, 
> format=json and format=sexp\n");
>   }
>   }
>
>   if (params.include_html &&
> -(format != NOTMUCH_FORMAT_JSON && format != 
> NOTMUCH_FORMAT_SEXP)) {
> - fprintf (stderr, "Warning: --include-html only implemented for 
> format=json and format=sexp\n");
> +(format != NOTMUCH_FORMAT_TEXT &&
> +  format != NOTMUCH_FORMAT_JSON &&
> +  format != NOTMUCH_FORMAT_SEXP)) {
> + fprintf (stderr, 

Re: Notmuch DB Problems

2018-09-05 Thread Jani Nikula
On Tue, 04 Sep 2018, mu...@nawaz.org wrote:
> Hi,
>
> A few days ago I noticed notmuch new was no longer working (I have it
> as a cron job so it took a while to figure it out).
>
> It just freezes. I do have a Python hook, and it was freezing on the
> line that opens the database.
>
> I tried a notmuch dump. Same problem - freezes
>
> Based on some earlier threads, I tried a notmuch compact. Same problem
> - freezes.
>
> All these freezes seem to use no memory/CPU. 
>
> Interestingly, queries work fine - from both the command line and the
> Emacs interface. So I can read old stuff just fine. But all the
> commands above cause a freeze. 
>
> Currently using notmuch-0.24.2. I tried notmuch-0.27 - same problem.
>
> Results of a xapian check:
>
> docdata:
> blocksize=8K items=6 firstunused=3 revision=6442 levels=0 root=0
> B-tree checked okay
> docdata table structure checked OK
>
> termlist:
> blocksize=8K items=178562 firstunused=53441 revision=6442 levels=2
> root=46086
> /usr/bin/xapian-check: DatabaseError: 1 unused block(s) missing from
> the free list, first is 0
>
> What are my options? Unfortunately the last dump I have is many months
> old, so I'm a bit wary of deleting the database and rebuilding. Given
> that the show and search commands work, I was wondering if I can write
> a script to get all the message/thread ID's for all the tags and store
> them, and then rebuild the database and use that stored information to
> retag all my messages (all without using the dump command)?

It might be interesting to see an strace log to possibly get an idea
where it gets stuck.

Is the filesystem writable and working okay?

If search and show work, I'm guessing it gets stuck in trying to open
the database writable. One hackish idea is to patch notmuch dump to open
the database in read-only mode, and dump the tags. See below. The dump
command opens the database writable to prevent changes while
dumping. (Arguably this could be a command line option for cases like
yours.)

BR,
Jani.

diff --git a/notmuch-dump.c b/notmuch-dump.c
index ef2f02dfeb5c..d06dbcf50224 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -364,7 +364,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, 
char *argv[])
 int ret;
 
 if (notmuch_database_open (notmuch_config_get_database_path (config),
-  NOTMUCH_DATABASE_MODE_READ_WRITE, ))
+  NOTMUCH_DATABASE_MODE_READ_ONLY, ))
return EXIT_FAILURE;
 
 notmuch_exit_if_unmatched_db_uuid (notmuch);
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: notmuch shows tag which is not on any email

2018-06-04 Thread Jani Nikula
On Mon, 04 Jun 2018, Gregor Zattler  wrote:
> Dear notmuch developers,
>
> does notmuch remember tags even if at time of query there is no
> message tagged with the respective tag?:
>
> $ notmuch search --output=tags '*' | grep telegraph
> EA%3Dtelegraph%40gmx%2Enet
> EA=telegr...@gmx.net
>
>
> $ notmuch count  -- is:EA%3Dtelegraph%40gmx%2Enet
> 0
>
>
> $ notmuch count  -- is:/EA.*telegraph/
> 4498
>
>
> There is no invisible character or some such:
> $ notmuch search --output=tags '*' | grep telegraph | head -n 1 | wc -c
> 27

Try all of the above with --exclude=false parameter and see if it makes
a difference. If it does, each of the messages tagged with
EA%3Dtelegraph%40gmx%2Enet is probably also tagged with one of the tags
in 'notmuch config get search.exclude_tags'.

> I might have done something wrong while experimenting with this
> tags and  'EA%3Dtelegraph%40gmx%2Enet' may be a leftover.  I
> would like to remove it but since no message matches it's not
> possible to remove it from messages and therefore the tag remains:
>
> $ notmuch tag +EA=telegr...@gmx.net  -- is:/EA.*telegraph/

Try that with

$ notmuch tag +EA=telegr...@gmx.net -EA%3Dtelegraph%40gmx%2Enet -- 
is:/EA.*telegraph/

to remedy the situation.

BR,
Jani.

> $ notmuch search --output=tags '*' | grep telegraph
> EA%3Dtelegraph%40gmx%2Enet
> EA=telegr...@gmx.net
>
>
> I tried to utilise emacs to remove the tag, but notmuch-emacs
> does not show 'EA%3Dtelegraph%40gmx%2Enet' in notmuch-hello's
> 'All tags' section.
>
>
>
> Why is said tag in the tags listing, how to get rid of it?
>
>
> Thanks, Gregor
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 3/4] lib: define specialized get_thread_id for use in thread subquery

2018-05-06 Thread Jani Nikula
On Sat, 05 May 2018, David Bremner  wrote:
> The observation is that we are only using the messages to get there
> thread_id, which is kindof a pessimal access pattern for the current
> notmuch_message_get_thread_id

LGTM.

> ---
>  lib/message.cc| 17 +
>  lib/notmuch-private.h |  4 
>  lib/thread-fp.cc  |  2 +-
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/lib/message.cc b/lib/message.cc
> index d5db89b6..b2067076 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -318,6 +318,23 @@ _notmuch_message_get_term (notmuch_message_t *message,
>  return value;
>  }
>  
> +/*
> + * For special applications where we only want the thread id, reading
> + * in all metadata is a heavy I/O penalty.
> + */
> +const char *
> +_notmuch_message_get_thread_id_only (notmuch_message_t *message)
> +{
> +
> +Xapian::TermIterator i = message->doc.termlist_begin ();
> +Xapian::TermIterator end = message->doc.termlist_end ();
> +
> +message->thread_id = _notmuch_message_get_term (message, i, end,
> + _find_prefix ("thread"));
> +return message->thread_id;
> +}
> +
> +
>  static void
>  _notmuch_message_ensure_metadata (notmuch_message_t *message, void *field)
>  {
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index 1093429c..4598577f 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -537,6 +537,10 @@ _notmuch_message_database (notmuch_message_t *message);
>  
>  void
>  _notmuch_message_remove_unprefixed_terms (notmuch_message_t *message);
> +
> +const char *
> +_notmuch_message_get_thread_id_only(notmuch_message_t *message);
> +
>  /* sha1.c */
>  
>  char *
> diff --git a/lib/thread-fp.cc b/lib/thread-fp.cc
> index dd292bf6..661d00dd 100644
> --- a/lib/thread-fp.cc
> +++ b/lib/thread-fp.cc
> @@ -50,7 +50,7 @@ ThreadFieldProcessor::operator() (const std::string & str)
>   std::string term = thread_prefix;
>   notmuch_message_t *message;
>   message = notmuch_messages_get (messages);
> - term += notmuch_message_get_thread_id (message);
> + term += _notmuch_message_get_thread_id_only (message);
>   terms.insert (term);
>   }
>   return Xapian::Query (Xapian::Query::OP_OR, terms.begin (), 
> terms.end ());
> -- 
> 2.17.0
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/4] lib: add thread subqueries.

2018-05-06 Thread Jani Nikula
On Sat, 05 May 2018, David Bremner  wrote:
> This change allows queries of the form
>
>  thread:{from:me} and thread:{from:jian} and not thread:{from:dave}
>
> This is still somewhat brute-force, but it's a big improvement over
> both the shell script solution and the previous proposal [1], because it
> does not build the whole thread structure just generate a
> query. A further potential optimization is to replace the calls to
> notmuch with more specialized Xapian code; in particular it's not
> likely that reading all of the message metadata is a win here.
>
> [1]: id:20170820213240.20526-1-da...@tethera.net
> ---
>  lib/Makefile.local   |  3 +-
>  lib/database.cc  |  6 +++-
>  lib/thread-fp.cc | 67 
>  lib/thread-fp.h  | 42 ++
>  test/T585-thread-subquery.sh | 46 +
>  5 files changed, 162 insertions(+), 2 deletions(-)
>  create mode 100644 lib/thread-fp.cc
>  create mode 100644 lib/thread-fp.h
>  create mode 100755 test/T585-thread-subquery.sh
>
> diff --git a/lib/Makefile.local b/lib/Makefile.local
> index 8aa03891..5dc057c0 100644
> --- a/lib/Makefile.local
> +++ b/lib/Makefile.local
> @@ -58,7 +58,8 @@ libnotmuch_cxx_srcs =   \
>   $(dir)/query-fp.cc  \
>   $(dir)/config.cc\
>   $(dir)/regexp-fields.cc \
> - $(dir)/thread.cc
> + $(dir)/thread.cc \
> + $(dir)/thread-fp.cc
>  
>  libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) 
> $(libnotmuch_cxx_srcs:.cc=.o)
>  
> diff --git a/lib/database.cc b/lib/database.cc
> index 02444e09..9cf8062c 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -21,6 +21,7 @@
>  #include "database-private.h"
>  #include "parse-time-vrp.h"
>  #include "query-fp.h"
> +#include "thread-fp.h"
>  #include "regexp-fields.h"
>  #include "string-util.h"
>  
> @@ -258,7 +259,8 @@ prefix_t prefix_table[] = {
>  { "directory",   "XDIRECTORY",   NOTMUCH_FIELD_NO_FLAGS },
>  { "file-direntry",   "XFDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS 
> },
>  { "directory-direntry",  "XDDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS },
> -{ "thread",  "G",NOTMUCH_FIELD_EXTERNAL 
> },
> +{ "thread",  "G",NOTMUCH_FIELD_EXTERNAL |
> + NOTMUCH_FIELD_PROCESSOR },
>  { "tag", "K",NOTMUCH_FIELD_EXTERNAL |
>   NOTMUCH_FIELD_PROCESSOR },
>  { "is",  "K",NOTMUCH_FIELD_EXTERNAL |
> @@ -317,6 +319,8 @@ _setup_query_field (const prefix_t *prefix, 
> notmuch_database_t *notmuch)
>   fp = (new DateFieldProcessor())->release ();
>   else if (STRNCMP_LITERAL(prefix->name, "query") == 0)
>   fp = (new QueryFieldProcessor (*notmuch->query_parser, 
> notmuch))->release ();
> + else if (STRNCMP_LITERAL(prefix->name, "thread") == 0)
> + fp = (new ThreadFieldProcessor (*notmuch->query_parser, 
> notmuch))->release ();
>   else
>   fp = (new RegexpFieldProcessor (prefix->name, prefix->flags,
>   *notmuch->query_parser, 
> notmuch))->release ();
> diff --git a/lib/thread-fp.cc b/lib/thread-fp.cc
> new file mode 100644
> index ..dd292bf6
> --- /dev/null
> +++ b/lib/thread-fp.cc
> @@ -0,0 +1,67 @@
> +/* thread-fp.cc - "thread:" field processor glue
> + *
> + * This file is part of notmuch.
> + *
> + * Copyright © 2018 David Bremner
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see https://www.gnu.org/licenses/ .
> + *
> + * Author: David Bremner 
> + */
> +
> +#include "database-private.h"
> +#include "thread-fp.h"
> +#include 
> +
> +#if HAVE_XAPIAN_FIELD_PROCESSOR
> +
> +Xapian::Query
> +ThreadFieldProcessor::operator() (const std::string & str)
> +{
> +notmuch_status_t status;
> +const char *thread_prefix = _find_prefix ("thread");
> +
> +if (str.at (0) == '{') {
> + if (str.length () > 1 && str.at (str.size () - 1) == '}') {

IIUC .length() and .size() are the same thing, but it's confusing to see
them both used on the same line.

Nitpick, I always favor dealing with error cases first, so you can do
the happy day scenario with less indent. So I'd check the opposite,

Re: [PATCH 2/2] test: use --no-mtime-opt in T050-new.sh

2018-04-29 Thread Jani Nikula
On Sun, 29 Apr 2018, David Bremner  wrote:
> Wherever the test relies on directories being scanned, this option
> should be used to avoid skipping them due to mtimes on directories
> matching the database.

I think you could additionally remove a few touch calls in the
test. Some of them do actually create empty files, but some of them just
touch directories to force rescans.

Otherwise, LGTM.

BR,
Jani.

> ---
>  test/T050-new.sh | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/test/T050-new.sh b/test/T050-new.sh
> index 9025fa7a..12dba471 100755
> --- a/test/T050-new.sh
> +++ b/test/T050-new.sh
> @@ -87,7 +87,7 @@ notmuch new > /dev/null
>  
>  mv "${MAIL_DIR}"/dir "${MAIL_DIR}"/dir-renamed
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory 
> ${MAIL_DIR}/dir for deletion from database
>  No new mail. Detected 3 file renames."
>  
> @@ -95,7 +95,7 @@ No new mail. Detected 3 file renames."
>  test_begin_subtest "Deleted directory"
>  rm -rf "${MAIL_DIR}"/dir-renamed
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 2: queuing passed directory 
> ${MAIL_DIR}/dir-renamed for deletion from database
>  No new mail. Removed 3 messages."
>  
> @@ -114,7 +114,7 @@ test_begin_subtest "Deleted directory (end of list)"
>  
>  rm -rf "${MAIL_DIR}"/zzz
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover 
> directory ${MAIL_DIR}/zzz for deletion from database
>  No new mail. Removed 3 messages."
>  
> @@ -165,7 +165,7 @@ test_begin_subtest "Deleted two-level directory"
>  
>  rm -rf "${MAIL_DIR}"/two
>  
> -output=$(NOTMUCH_NEW --debug)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt)
>  test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover 
> directory ${MAIL_DIR}/two for deletion from database
>  No new mail. Removed 3 messages."
>  
> @@ -211,7 +211,7 @@ Subject: Test mbox message 2
>  
>  Body 2.
>  EOF
> -output=$(NOTMUCH_NEW --debug 2>&1)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt 2>&1)
>  test_expect_equal "$output" \
>  "Note: Ignoring non-mail file: ${MAIL_DIR}/.git/config
>  Note: Ignoring non-mail file: ${MAIL_DIR}/.ignored_hidden_file
> @@ -234,7 +234,7 @@ touch "${MAIL_DIR}"/.git # change .git's mtime for 
> notmuch new to rescan.
>  touch "${MAIL_DIR}"  # likewise for MAIL_DIR
>  mkdir -p "${MAIL_DIR}"/one/two/three/.git
>  touch "${MAIL_DIR}"/{one,one/two,one/two/three}/ignored_file
> -output=$(NOTMUCH_NEW --debug 2>&1 | sort)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt 2>&1 | sort)
>  test_expect_equal "$output" \
>  "(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.git
>  (D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
> @@ -261,7 +261,7 @@ test_expect_equal "$output" "No new mail."
>  
>  test_begin_subtest "Ignore files and directories specified in new.ignore 
> (regexp)"
>  notmuch config set new.ignore ".git" "/^bro.*ink\$/" "/ignored.*file/"
> -output=$(NOTMUCH_NEW --debug 2>&1 | sort)
> +output=$(NOTMUCH_NEW --debug --no-mtime-opt 2>&1 | sort)
>  test_expect_equal "$output" \
>  "(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.git
>  (D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
> -- 
> 2.17.0
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] CLI/new: add mtime-opt option

2018-04-29 Thread Jani Nikula
On Sun, 29 Apr 2018, David Bremner  wrote:
> This option, on by default, controls whether mtimes are used to
> optimize the scanning of directories. The intent is to turn this
> optimization off for certain tests.
> ---
>  notmuch-new.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index c4345705..099bbbae 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -47,6 +47,7 @@ typedef struct {
>  int output_is_a_tty;
>  enum verbosity verbosity;
>  bool debug;
> +bool mtime_opt;
>  const char **new_tags;
>  size_t new_tags_length;
>  const char **ignore_verbatim;
> @@ -527,7 +528,7 @@ add_files (notmuch_database_t *notmuch,
>   * mistakenly return the total number of directory entries, since
>   * that only inflates the count beyond 2.
>   */
> -if (directory && fs_mtime == db_mtime && st.st_nlink == 2) {
> +if (directory && state->mtime_opt && fs_mtime == db_mtime && st.st_nlink 
> == 2) {
>   /* There's one catch: pass 1 below considers symlinks to
>* directories to be directories, but these don't increase the
>* file system link count.  So, only bail early if the
> @@ -618,7 +619,7 @@ add_files (notmuch_database_t *notmuch,
>   * being discovered until the clock catches up and the directory
>   * is modified again).
>   */
> -if (directory && fs_mtime == db_mtime)
> +if (directory && state->mtime_opt && fs_mtime == db_mtime)
>   goto DONE;
>  
>  /* If the database has never seen this directory before, we can
> @@ -771,7 +772,7 @@ add_files (notmuch_database_t *notmuch,
>   * the database because a message could be delivered later in this
>   * same second.  This may lead to unnecessary re-scans, but it
>   * avoids overlooking messages. */
> -if (fs_mtime != stat_time)
> +if (state->mtime_opt && fs_mtime != stat_time)
>   _filename_list_add (state->directory_mtimes, path)->mtime = fs_mtime;

I don't think we should skip this part. We've done a full scan now, so
we should record that in the database so a subsequent scan doesn't have
to.

>  
>DONE:
> @@ -1053,6 +1054,7 @@ notmuch_new_command (notmuch_config_t *config, int 
> argc, char *argv[])
>  add_files_state_t add_files_state = {
>   .verbosity = VERBOSITY_NORMAL,
>   .debug = false,
> + .mtime_opt = true,
>   .output_is_a_tty = isatty (fileno (stdout)),
>  };
>  struct timeval tv_start;
> @@ -1073,6 +1075,7 @@ notmuch_new_command (notmuch_config_t *config, int 
> argc, char *argv[])
>   { .opt_bool = , .name = "quiet" },
>   { .opt_bool = , .name = "verbose" },
>   { .opt_bool = _files_state.debug, .name = "debug" },
> + { .opt_bool = _files_state.mtime_opt, .name = "mtime-opt" },

--full-scan? --force? --force-scan?

I think we've had people ask how they can have notmuch scan some
directories again, for some reason or another. Maybe to test their
ignores, and they can't have notmuch rescan the directory without
touching it. IMHO --no-mtime-opt doesn't sound very intuitive for that
purpose.

Otherwise, seems like a thing we should add.

BR,
Jani.

>   { .opt_bool = , .name = "hooks" },
>   { .opt_inherit = notmuch_shared_indexing_options },
>   { .opt_inherit = notmuch_shared_options },
> -- 
> 2.17.0
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: non-deterministic behaviour of new.ignore (regexp) test

2018-04-29 Thread Jani Nikula
On Sat, 28 Apr 2018, David Bremner  wrote:
> For me the following seems to consistently fail after between 30 and 500
> attempts
>
> export NOTMUCH_TEST_QUIET=yes; count=0; while ./T050-new.sh; do (( 
> count++ )); echo $count; done

I believe this happens because the directory mtime is unchanged from the
previous scan in the test, and we skip the directory before we could
ignore the files. Quoting add_files():

/* If the directory's modification time in the filesystem is the
 * same as what we recorded in the database the last time we
 * scanned it, then we can skip the second pass entirely.
 *
 * We test for strict equality here to avoid a bug that can happen
 * if the system clock jumps backward, (preventing new mail from
 * being discovered until the clock catches up and the directory
 * is modified again).
 */

I can't reproduce if I add this to the test:

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 9025fa7aa63e..0db76f47130b 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -260,6 +260,7 @@ output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" "No new mail."
 
 test_begin_subtest "Ignore files and directories specified in new.ignore 
(regexp)"
+touch "${MAIL_DIR}" # force rescan of the top level directory
 notmuch config set new.ignore ".git" "/^bro.*ink\$/" "/ignored.*file/"
 output=$(NOTMUCH_NEW --debug 2>&1 | sort)
 test_expect_equal "$output" \

---

However, I'm not sure even that is enough if all this happens in the
same second. I think the way notmuch new is written, it may skip as long
as it ensures a subsequent scan will catch the modified files:

/* If the directory's mtime is the same as the wall-clock time
 * when we stat'ed the directory, we skip updating the mtime in
 * the database because a message could be delivered later in this
 * same second.  This may lead to unnecessary re-scans, but it
 * avoids overlooking messages. */

I think we can make the problem less likely with the touch, but as
everything gets faster, we might hit this more and more. One approach
might be a notmuch new --force option that would rescan all directories
regardless of mtimes. We could use this for testing (except when we're
testing the optimization).

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] cli: looking for config file in $XDG_CONFIG_HOME

2018-04-06 Thread Jani Nikula
On Thu, 15 Mar 2018, Daniel Kahn Gillmor  wrote:
> On Wed 2018-03-14 22:54:06 -0300, David Bremner wrote:
>> There doesn't seem to be a good reason to drop ~/.notmuch-config
>> completely here. As Tomi notes, that would break notmuch for all current
>> users. I suppose I could live with $XDG_CONFIG_HOME/notmuch/config (or
>> whatever) taking precedence if it exists.
>
> If we ant to keep the config file, then i agree that
> $XDG_CONFIG_HOME/notmuch/config is probably the a better place for it
> than ~/.notmuch-config.  But:
>
> It seemed to me that there was a growing consensus that the configfile
> could be phased out in favor of storing configuration details in the
> database itself.  (this is dependent on someone™ actually doing the
> implementation work, of course)

What about configuration specific to the CLI that is irrelevant for the
library/database? Do you propose to store those in the database too? For
example user.name or crypto.gpg_path. And then there's the database
location, which obviously can't be in the database.

For database specific configuration that currently lies in the CLI
config, there's also a bunch of rework required. For example, if we
store new.tags in the database, what would be the point of the CLI
reading those out of the database, and then applying them to the
message? Shouldn't the new.tags be applied to the messages at the
library level directly? Ditto for search.exclude_tags and
maildir.synchronize_flags.

Where's the point in moving all this stuff to the database, unless we
also use them directly in the database?

> if we do this, it seems likely that we'll need to keep around handling
> the config file for backward compatibility, via something like:
>
>  * when we observe a config file, we could walk each option present in
>it.  For each option:
>
> a) if that option is not present in the database, copy it into the
>database.
>
> b) if that option is present in the database, and it matches the
>option in the config file, ignore
>
> c) if that option is present in the database but does not match the
>config file, use the version in the config file but warn the user
>that they have a conflict they should probably resolve soon.

Wouldn't this require having an in-memory copy of the database config,
where the items originating from the config file would override but not
get saved into the on-disk version of the database config?

> after some number of versions has elapsed and we are ready to explicitly
> deprecate the config file, perhaps we convert situations (a) and (b)
> into warning messages and situation (c) into a hard error.
>
> This is all a bit confusing, but it is the price we pay for having a
> smooth transition that allows users to upgrade knowing they can roll
> back if a new version isn't working for them.

I think it's more confusing than you think. ;)

I also kind of like the CLI config file that I can just save in git
along with my other dotfiles. The database config is much more annoying
in this regard.

> So i think it would be a shame to have an additional layer of confusion
> added by having two different deprecated configuration files.  So i lean
> against adopting this change. I'd much rather see work on phasing out
> the configfile.

Agreed on not increasing the confusion, at least not before we figure
out what we want to do.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: notmuch-search-get-tags unique candidates

2018-03-23 Thread Jani Nikula
On Wed, Mar 21, 2018 at 11:21 PM, Nicolò Balzarotti
 wrote:
> Hi, I'm using notmuch with emacs and I'm loving it. One thing I don't
> like is that in `notmuch-search`, when applying a tag to a selection the
> tab completition return multiple copies of the same tag. Example:
>
>> -inbox
>> -lists/aaa
>> -lists
>> -inbox
>> -lists/aaa
>> -lists
>
> I changed `notmuch-search-get-tags-region` to:
> ```(defun notmuch-search-get-tags-region (beg end)
>   (let (output)
> (notmuch-search-foreach-result beg end
>   (lambda (pos)
> (setq output (append output (notmuch-search-get-tags pos)
> (delete-dups output)));; add delete-dups before returning
> ```
>
> Why is the current one the default behavior?

FWIW, I can't reproduce the behaviour you describe.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: multilingual notmuch (and Content-Language)

2018-03-18 Thread Jani Nikula
On Sun, 18 Mar 2018, Daniel Kahn Gillmor  wrote:
>  * if we know our index expects english, and we have a message part that
>*is not* english (e.g. Content-Language: es), we could avoid indexing
>that part.

Why would we do that? Search mostly works just fine for non-English
languages, it's just that the *stemming* is not right.

> what do you think?  what ideas are missing from the branstorm above?  I'd
> love to hear from people with multilingual mailboxes about how we might
> be able to make notmuch work better for them.

With my limited understanding of this, stemming happens both at indexing
and searching. Basically at indexing, the term generator indexes both
the full and the stemmed version of words. I'm wondering if we could
look at Content-Language (and missing that, heuristics), and (if the
user so desires) use multiple term generators with different stemmers on
a per document basis. Or, use non-stemming indexing for unidentified or
unsupported languages. How far would that take us? Then, perhaps, we
could also perform language specific queries?

I don't know how feasible that is, or if it would require Xapian
changes.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] notmuch-mutt: use --format=text0 and xargs -0

2018-02-27 Thread Jani Nikula
On Tue, 27 Feb 2018, Jani Nikula <j...@nikula.org> wrote:
> notmuch-mutt fails for message files with special characters such as
> single quote in their filename. Use notmuch search --format=text0 and
> xargs -0 combo to handle them.
>
> Reported and tested by "dob1" on IRC.
> ---
>  contrib/notmuch-mutt/notmuch-mutt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/notmuch-mutt/notmuch-mutt 
> b/contrib/notmuch-mutt/notmuch-mutt
> index 0e46a8c1b95e..57f13075aa22 100755
> --- a/contrib/notmuch-mutt/notmuch-mutt
> +++ b/contrib/notmuch-mutt/notmuch-mutt
> @@ -48,9 +48,9 @@ sub search($$$) {
>  }
>  
>  empty_maildir($maildir);
> -system("notmuch search --output=files $dup_option $query"
> +system("notmuch search --format=text0 --output=files $dup_option $query"
>  . " | sed -e 's: : :g'"

Come to think of it, does this need sed -z too?

> -. " | xargs -r -I searchoutput ln -s searchoutput $maildir/cur/");
> +. " | xargs -0 -r -I searchoutput ln -s searchoutput $maildir/cur/");
>  }
>  
>  sub prompt($$) {
> -- 
> 2.11.0
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] notmuch-mutt: use --format=text0 and xargs -0

2018-02-27 Thread Jani Nikula
notmuch-mutt fails for message files with special characters such as
single quote in their filename. Use notmuch search --format=text0 and
xargs -0 combo to handle them.

Reported and tested by "dob1" on IRC.
---
 contrib/notmuch-mutt/notmuch-mutt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/notmuch-mutt/notmuch-mutt 
b/contrib/notmuch-mutt/notmuch-mutt
index 0e46a8c1b95e..57f13075aa22 100755
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -48,9 +48,9 @@ sub search($$$) {
 }
 
 empty_maildir($maildir);
-system("notmuch search --output=files $dup_option $query"
+system("notmuch search --format=text0 --output=files $dup_option $query"
   . " | sed -e 's: : :g'"
-  . " | xargs -r -I searchoutput ln -s searchoutput $maildir/cur/");
+  . " | xargs -0 -r -I searchoutput ln -s searchoutput $maildir/cur/");
 }
 
 sub prompt($$) {
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re:

2018-02-03 Thread Jani Nikula
On Thu, 01 Feb 2018, Matthew Lear  wrote:
> From: Matthew Lear 
> To: notmuch@notmuchmail.org
> Cc: Matthew Lear 
> Subject: [PATCH] Update date search syntax.
> Date: Thu,  1 Feb 2018 20:52:18 +
> Message-Id: <20180201205218.4368-1-m...@bubblegen.co.uk>
> X-Mailer: git-send-email 2.14.1
>
> If searching using the date prefix and timestamps, each timestamp
> is required to be prefixed with an @
> Legacy syntax of .. without the
> date prefix is still honoured, only without the @ specifiers.
> ---
>  doc/man7/notmuch-search-terms.rst | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/doc/man7/notmuch-search-terms.rst 
> b/doc/man7/notmuch-search-terms.rst
> index 6d2bf62a..b6e7079a 100644
> --- a/doc/man7/notmuch-search-terms.rst
> +++ b/doc/man7/notmuch-search-terms.rst
> @@ -124,10 +124,13 @@ date:.. or date:
>  The time range can also be specified using timestamps with a
>  syntax of:
>  
> -..
> +@..@

So I think I'd add the @ syntax in the DATE AND TIME SEARCH section,
maybe under a separate new heading, and just emphasize this here is
about the non-date prefixed thing.

BR,
Jani.
  
>  Each timestamp is a number representing the number of seconds
> -since 1970-01-01 00:00:00 UTC.
> +since 1970-01-01 00:00:00 UTC. A date range search using
> +timestamps is also permitted without using the date prefix and
> +@ specifiers, although this is considered legacy and pre-dates
> +the date prefix.
>  
>  lastmod:..
>  The **lastmod:** prefix can be used to restrict the result by the
> -- 
> 2.14.1
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: trouble searching with unix timestamps

2018-01-29 Thread Jani Nikula
On Mon, 29 Jan 2018, Matthew Lear  wrote:
> Hi. I've noticed that I'm unable to notmuch search using date with unix
> timestamps (ie number of seconds since Jan 1st 1970 UTC). I get a xapian
> error. The notmuch man pages state that searching using
> 'date:..'
> where "each timestamp is a number representing the number of seconds since
> 1970-01-01 00:00:00 UTC" is supported, so I'm wondering what's wrong with
> my installation.

The man page could use some clarification.

The .. syntax predates the date:
prefix, and needs to be given as-is:

$ notmuch search 1517152333..1517238733

Alternatively, you can use @ in date: queries, although this
seems to be completely undocumented:

$ notmuch search date:@1517152333..@1517238733

HTH,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v3] doc: unify definition list usage across man pages

2017-12-30 Thread Jani Nikula
On Sat, 30 Dec 2017, David Bremner <da...@tethera.net> wrote:
> Jani Nikula <j...@nikula.org> writes:
>
>> Make all parameter descriptions etc. use reStructuredText definition
>> lists with uniform style and indentation. Remove redundant indentation
>> from around the lists. Remove blank lines between term lines and
>> definition blocks. Use four spaces for indentation.
>>
>> This is almost completely whitespace and paragraph reflow changes.
>
> My inclination is to apply this to the master branch, but not to include
> it in the 0.26 release. Any comments?

Make it so.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3] doc: unify definition list usage across man pages

2017-12-30 Thread Jani Nikula
Make all parameter descriptions etc. use reStructuredText definition
lists with uniform style and indentation. Remove redundant indentation
from around the lists. Remove blank lines between term lines and
definition blocks. Use four spaces for indentation.

This is almost completely whitespace and paragraph reflow changes.
---
 doc/man1/notmuch-address.rst| 170 ++-
 doc/man1/notmuch-compact.rst|  16 +-
 doc/man1/notmuch-config.rst | 354 +++-
 doc/man1/notmuch-count.rst  |  65 
 doc/man1/notmuch-dump.rst   | 116 ++---
 doc/man1/notmuch-emacs-mua.rst  |  64 
 doc/man1/notmuch-insert.rst |  82 +-
 doc/man1/notmuch-new.rst|  44 +++--
 doc/man1/notmuch-reindex.rst|  45 +++--
 doc/man1/notmuch-reply.rst  | 102 ++--
 doc/man1/notmuch-restore.rst| 118 +++---
 doc/man1/notmuch-search.rst | 220 -
 doc/man1/notmuch-show.rst   | 264 +++---
 doc/man1/notmuch-tag.rst|  33 ++--
 doc/man1/notmuch.rst|  33 ++--
 doc/man5/notmuch-hooks.rst  |  55 +++
 doc/man7/notmuch-properties.rst |   1 -
 17 files changed, 865 insertions(+), 917 deletions(-)

diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index 68415d13c5b6..c00d7d743e3e 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -18,93 +18,89 @@ See **notmuch-search-terms(7)** for details of the 
supported syntax for
 
 Supported options for **address** include
 
-``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
-Presents the results in either JSON, S-Expressions, newline
-character separated plain-text (default), or null character
-separated plain-text (compatible with **xargs(1)** -0 option
-where available).
-
-``--format-version=N``
-Use the specified structured output format version. This is
-intended for programs that invoke **notmuch(1)** internally. If
-omitted, the latest supported version will be used.
-
-``--output=(sender|recipients|count|address)``
-
-Controls which information appears in the output. This option
-can be given multiple times to combine different outputs.
-When neither --output=sender nor --output=recipients is
-given, --output=sender is implied.
-
-**sender**
-Output all addresses from the *From* header.
-
-Note: Searching for **sender** should be much faster than
-searching for **recipients**, because sender addresses are
-cached directly in the database whereas other addresses
-need to be fetched from message files.
-
-**recipients**
-Output all addresses from the *To*, *Cc* and *Bcc*
-headers.
-
-**count**
-Print the count of how many times was the address
-encountered during search.
-
-Note: With this option, addresses are printed only after
-the whole search is finished. This may take long time.
-
-**address**
-Output only the email addresses instead of the full
-mailboxes with names and email addresses. This option has
-no effect on the JSON or S-Expression output formats.
-
-``--deduplicate=(no|mailbox|address)``
-
-Control the deduplication of results.
-
-**no**
-Output all occurrences of addresses in the matching
-messages. This is not applicable with --output=count.
-
-**mailbox**
-Deduplicate addresses based on the full, case sensitive
-name and email address, or mailbox. This is effectively
-the same as piping the --deduplicate=no output to **sort |
-uniq**, except for the order of results. This is the
-default.
-
-**address**
-Deduplicate addresses based on the case insensitive
-address part of the mailbox. Of all the variants (with
-different name or case), print the one occurring most
-frequently among the matching messages. If --output=count
-is specified, include all variants in the count.
-
-``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
-This option can be used to present results in either
-chronological order (**oldest-first**) or reverse chronological
-order (**newest-first**).
-
-By default, results will be displayed in reverse chronological
-order, (that is, the newest results will be displayed first).
-
-However, if either --output=count or --deduplicate=address is
-specified, this option is ignored and the order of the results
-is unspecified.
-
-``--exclude=(true|false)``
-A message is called "excluded" if it matches at least one tag in
-search.tag\_exclude that 

Re: [PATCH] cli: add support for only printing the addresses in notmuch address

2017-12-20 Thread Jani Nikula
On Tue, 19 Dec 2017, Jameson Graef Rollins <jroll...@finestructure.net> wrote:
> On Wed, Dec 20 2017, Jani Nikula <j...@nikula.org> wrote:
>> ~$ notmuch address --output=sender --output=recipients --output=address 
>> --output=count id:878tdy8a2q@ligo.caltech.edu
>> 1notmuch@notmuchmail.org
>> 1jroll...@finestructure.net
>> 1d...@fifthhorseman.net
>> 1j...@nikula.org
>>
>> I prefer this to separate options.
>
> Really?  If each is just a switch then why not:
>
> ~$ notmuch address --sender --recipients --address --count 
> id:878tdy8a2q@ligo.caltech.edu
>
> ?  It's just shorter, right?
>
> Also, this behavior is quite different than for search, in which only
> the last --output applies.
>
>> notmuch search uses separate --entire-thread, --body, and --include-html
>> options, and I think those are getting messy.

That was supposed to be notmuch show, not search.

> The seem less messy than a "--output=" prefixed version of the same.

Matter of taste. I like the self-documenting aspect of *what* these
options control. --output=body is obvious, --body is less
so. --include-html includes html somewhere, I think --output=html would
be better.

In the spirit of worse is better, I'll note that using --output= stores
all the possible values in a single bit mask, and it's easy to define
the defaults for *all* possible outputs in one variable, and it's easy
to check *all* possible combinations with bit masks. Not so with
independent parameters. Again, matter of taste how much you appreciate
implementation simplicity. With notmuch show, I think that guideline
would have made the interface better too. YMMV.

As to notmuch address --output=address, it fulfills all the feature
needs that popped up in this thread. If there's a strong desired to
change the interface, we'll need patches as this one's already merged.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] cli: add support for only printing the addresses in notmuch address

2017-12-19 Thread Jani Nikula
On Tue, 19 Dec 2017, Jameson Graef Rollins <jroll...@finestructure.net> wrote:
> On Tue, Dec 19 2017, Daniel Kahn Gillmor <d...@fifthhorseman.net> wrote:
>> On Tue 2017-12-19 13:23:55 -0800, Jameson Graef Rollins wrote:
>>> On Thu, Nov 02 2017, Jani Nikula <j...@nikula.org> wrote:
>>>> The notmuch address output is much more useful for scripts with just
>>>> the addresses printed. Support this using the --output=address option.
>>>
>>> Isn't "address" kind of orthogonal to "sender" and "recipient"?  Isn't
>>> this more like the --output/--format distinction in search?
>>
>> i think i agree with Jamie here -- it'd be nice to be able to ask for
>> the addresses of the senders, or the addresses of the recipients.  how
>> would that be done here?
>
> Sorry, I see now that address already has the --format option with the
> expected values.  So I think either address-only or sender/recipient
> should be a separate option.

Note that you can give the notmuch address --output option multiple
times to control the output. For example,

~$ notmuch address --output=sender --output=recipients 
id:878tdy8a2q@ligo.caltech.edu
Jameson Graef Rollins <jroll...@finestructure.net>
Daniel Kahn Gillmor <d...@fifthhorseman.net>
Jani Nikula <j...@nikula.org>
notmuch@notmuchmail.org

~$ notmuch address --output=recipients --output=address 
id:878tdy8a2q@ligo.caltech.edu
d...@fifthhorseman.net
j...@nikula.org
notmuch@notmuchmail.org

~$ notmuch address --output=sender --output=recipients --output=address 
--output=count id:878tdy8a2q@ligo.caltech.edu
1   notmuch@notmuchmail.org
1   jroll...@finestructure.net
1   d...@fifthhorseman.net
1   j...@nikula.org

I prefer this to separate options.

notmuch search uses separate --entire-thread, --body, and --include-html
options, and I think those are getting messy.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2] doc: unify definition list usage across man pages

2017-11-11 Thread Jani Nikula
Make all parameter descriptions etc. use reStructuredText definition
lists with uniform style and indentation. Remove redundant indentation
from around the lists. Remove blank lines between term lines and
definition blocks. Use four spaces for indentation.

This is almost completely whitespace and paragraph reflow changes.
---
 doc/man1/notmuch-address.rst| 160 -
 doc/man1/notmuch-compact.rst|  16 +--
 doc/man1/notmuch-config.rst | 256 +++-
 doc/man1/notmuch-count.rst  |  65 +-
 doc/man1/notmuch-dump.rst   | 116 +-
 doc/man1/notmuch-emacs-mua.rst  |  64 +-
 doc/man1/notmuch-insert.rst |  64 +-
 doc/man1/notmuch-new.rst|  33 +++---
 doc/man1/notmuch-reindex.rst|  21 ++--
 doc/man1/notmuch-reply.rst  |  94 ---
 doc/man1/notmuch-restore.rst| 118 +-
 doc/man1/notmuch-search.rst | 220 +-
 doc/man1/notmuch-show.rst   | 256 
 doc/man1/notmuch-tag.rst|  33 +++---
 doc/man1/notmuch.rst|  33 +++---
 doc/man5/notmuch-hooks.rst  |  55 +
 doc/man7/notmuch-properties.rst |   1 -
 17 files changed, 779 insertions(+), 826 deletions(-)

diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index dbac20f7b012..00cb554523e7 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -18,88 +18,84 @@ See **notmuch-search-terms(7)** for details of the 
supported syntax for
 
 Supported options for **address** include
 
-``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
-Presents the results in either JSON, S-Expressions, newline
-character separated plain-text (default), or null character
-separated plain-text (compatible with **xargs(1)** -0 option
-where available).
-
-``--format-version=N``
-Use the specified structured output format version. This is
-intended for programs that invoke **notmuch(1)** internally. If
-omitted, the latest supported version will be used.
-
-``--output=(sender|recipients|count)``
-
-Controls which information appears in the output. This option
-can be given multiple times to combine different outputs.
-When neither --output=sender nor --output=recipients is
-given, --output=sender is implied.
-
-**sender**
-Output all addresses from the *From* header.
-
-Note: Searching for **sender** should be much faster than
-searching for **recipients**, because sender addresses are
-cached directly in the database whereas other addresses
-need to be fetched from message files.
-
-**recipients**
-Output all addresses from the *To*, *Cc* and *Bcc*
-headers.
-
-**count**
-Print the count of how many times was the address
-encountered during search.
-
-Note: With this option, addresses are printed only after
-the whole search is finished. This may take long time.
-
-``--deduplicate=(no|mailbox|address)``
-
-Control the deduplication of results.
-
-**no**
-Output all occurrences of addresses in the matching
-messages. This is not applicable with --output=count.
-
-**mailbox**
-Deduplicate addresses based on the full, case sensitive
-name and email address, or mailbox. This is effectively
-the same as piping the --deduplicate=no output to **sort |
-uniq**, except for the order of results. This is the
-default.
-
-**address**
-Deduplicate addresses based on the case insensitive
-address part of the mailbox. Of all the variants (with
-different name or case), print the one occurring most
-frequently among the matching messages. If --output=count
-is specified, include all variants in the count.
-
-``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
-This option can be used to present results in either
-chronological order (**oldest-first**) or reverse chronological
-order (**newest-first**).
-
-By default, results will be displayed in reverse chronological
-order, (that is, the newest results will be displayed first).
-
-However, if either --output=count or --deduplicate=address is
-specified, this option is ignored and the order of the results
-is unspecified.
-
-``--exclude=(true|false)``
-A message is called "excluded" if it matches at least one tag in
-search.tag\_exclude that does not appear explicitly in the
-search terms. This option specifies whether to omit excluded
-messages in the search process.
-
-The default value, **true**, 

[PATCH] test: expand argument parsing sanity checks

2017-11-11 Thread Jani Nikula
Test the boolean --arg=true and --arg=false formats.

---

Extracted from id:20171014131608.17587-1-j...@nikula.org as this makes
sense independent of that series.
---
 test/T410-argument-parsing.sh | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 71ed7e38553b..910843d684c6 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -37,4 +37,18 @@ positional arg 1 false
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "--boolean=true"
+$TEST_DIRECTORY/arg-test --boolean=true > OUTPUT
+cat < EXPECTED
+boolean 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--boolean=false"
+$TEST_DIRECTORY/arg-test --boolean=false > OUTPUT
+cat < EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: what is the concept of archiving?

2017-11-09 Thread Jani Nikula
On Thu, Nov 9, 2017 at 11:56 AM, Linus Arver  wrote:
> Unfortunately, I cannot find enough information online about what any of
> these functions do, or what the concept of "archiving" means. From
> reading some of the sources in notmuch-tree.el I found out that
> archiving a message results in "applying the tag changes in
> `notmuch-archive-tags' to it" but I am not sure what this means.

Archiving, by default, simply means removing the inbox tag, either
from a single message or from all messages in a thread. You can use
notmuch-archive-tags to configure the tag change actions if you like.

> I found out that the key bindings are in that same file, which assigns
> "a" to notmuch-tree-archive-message-then-next; when I press "a" in my
> inbox view (all messages tagged "inbox"), it appears to just remove the
> "inbox" tag. I have some questions:
>
> (1) What is the point of archiving (what problem does it solve)?

Remove the messages from your inbox search. (At least I prefer to keep
this distinct from removing the unread tag; I archive tons of messages
without reading, and I have some messages with inbox tag that I've
already read.)

> (2) Do any other things happen to such archived messages, apart from the
> tag being removed?

No. Optionally, you can configure notmuch-archive-tags to remove more
or other tags, or add tags.

> (3) How do I un-archive these messages, if I press the "a" key by
> accident?

Find the message, hit "k k a". ("k" enters a submenu for doing
customizable tagging operations quickly, "k k" does the reverse
operation.)

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: buggy tags

2017-11-09 Thread Jani Nikula
On Thu, Nov 9, 2017 at 12:11 PM, Tom Hirschowitz
 wrote:
> I sometimes mistype some tags, e.g., collaegues instead of colleagues,
> etc. Of course, I then replace them with the right version.
>
> But notmuch keeps track of these wrong tags (as of all others) and
> proposes them when autocompleting, which after a few years of using
> notmuch, starts being significantly annoying.

Notmuch only completes to tags that are actually present in the
database. So you've missed some.

> Is there a way of getting rid of them?

This will list all the tags you have:

$ notmuch search --output=tags "*"

And this will replace a wrong tag with the right tag:

$ notmuch tag +right-tag -wrong-tag tag:wrong-tag

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Segfault with pkcs7 message

2017-11-03 Thread Jani Nikula
On Fri, Nov 3, 2017 at 11:05 AM, Holger Schurig  wrote:
> The attached e-mail makes notmuch from git HEAD segfault:
>
> $ notmuch --version
> notmuch 0.25.1+161~g1b91884
> $ notmuch show --decrypt id:86595dab-d486-536b-fe7a-ae3e1dca1...@posteo.de
> Warning: Crypto engine initialization failure (application/pkcs7-signature).
> Segmentation fault

See http://mid.mail-archive.com/20171016154044.14564-1-jani%40nikula.org

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] doc: unify definition list usage across man pages

2017-11-02 Thread Jani Nikula
Make all parameter descriptions etc. use reStructuredText definition
lists with uniform style and indentation. Remove redundant indentation
from around the lists. Remove blank lines between term lines and
definition blocks. Use four spaces for indentation.

This is almost completely whitespace and paragraph reflow changes.
---
 doc/man1/notmuch-address.rst| 160 -
 doc/man1/notmuch-compact.rst|  16 +--
 doc/man1/notmuch-config.rst | 256 +++-
 doc/man1/notmuch-count.rst  |  65 +-
 doc/man1/notmuch-dump.rst   | 116 +-
 doc/man1/notmuch-emacs-mua.rst  |  64 +-
 doc/man1/notmuch-insert.rst |  62 +-
 doc/man1/notmuch-new.rst|  33 +++---
 doc/man1/notmuch-reindex.rst|  21 ++--
 doc/man1/notmuch-reply.rst  |  94 ---
 doc/man1/notmuch-restore.rst| 118 +-
 doc/man1/notmuch-search.rst | 220 +-
 doc/man1/notmuch-show.rst   | 256 
 doc/man1/notmuch-tag.rst|  33 +++---
 doc/man1/notmuch.rst|  33 +++---
 doc/man5/notmuch-hooks.rst  |  55 +
 doc/man7/notmuch-properties.rst |   1 -
 17 files changed, 778 insertions(+), 825 deletions(-)

diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index dbac20f7b012..00cb554523e7 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -18,88 +18,84 @@ See **notmuch-search-terms(7)** for details of the 
supported syntax for
 
 Supported options for **address** include
 
-``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
-Presents the results in either JSON, S-Expressions, newline
-character separated plain-text (default), or null character
-separated plain-text (compatible with **xargs(1)** -0 option
-where available).
-
-``--format-version=N``
-Use the specified structured output format version. This is
-intended for programs that invoke **notmuch(1)** internally. If
-omitted, the latest supported version will be used.
-
-``--output=(sender|recipients|count)``
-
-Controls which information appears in the output. This option
-can be given multiple times to combine different outputs.
-When neither --output=sender nor --output=recipients is
-given, --output=sender is implied.
-
-**sender**
-Output all addresses from the *From* header.
-
-Note: Searching for **sender** should be much faster than
-searching for **recipients**, because sender addresses are
-cached directly in the database whereas other addresses
-need to be fetched from message files.
-
-**recipients**
-Output all addresses from the *To*, *Cc* and *Bcc*
-headers.
-
-**count**
-Print the count of how many times was the address
-encountered during search.
-
-Note: With this option, addresses are printed only after
-the whole search is finished. This may take long time.
-
-``--deduplicate=(no|mailbox|address)``
-
-Control the deduplication of results.
-
-**no**
-Output all occurrences of addresses in the matching
-messages. This is not applicable with --output=count.
-
-**mailbox**
-Deduplicate addresses based on the full, case sensitive
-name and email address, or mailbox. This is effectively
-the same as piping the --deduplicate=no output to **sort |
-uniq**, except for the order of results. This is the
-default.
-
-**address**
-Deduplicate addresses based on the case insensitive
-address part of the mailbox. Of all the variants (with
-different name or case), print the one occurring most
-frequently among the matching messages. If --output=count
-is specified, include all variants in the count.
-
-``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
-This option can be used to present results in either
-chronological order (**oldest-first**) or reverse chronological
-order (**newest-first**).
-
-By default, results will be displayed in reverse chronological
-order, (that is, the newest results will be displayed first).
-
-However, if either --output=count or --deduplicate=address is
-specified, this option is ignored and the order of the results
-is unspecified.
-
-``--exclude=(true|false)``
-A message is called "excluded" if it matches at least one tag in
-search.tag\_exclude that does not appear explicitly in the
-search terms. This option specifies whether to omit excluded
-messages in the search process.
-
-The default value, **true**, 

[PATCH] test: test notmuch insert --folder=""

2017-11-02 Thread Jani Nikula
Test insert into top level folder.
---
 test/T070-insert.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index f1650e623e35..40519bb2f217 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -132,6 +132,13 @@ output=$(notmuch search --output=files path:Drafts/new)
 dirname=$(dirname "$output")
 test_expect_equal "$dirname" "$MAIL_DIR/Drafts/new"
 
+test_begin_subtest "Insert message into top level folder"
+gen_insert_msg
+notmuch insert --folder="" < "$gen_msg_filename"
+output=$(notmuch search --output=files id:${gen_msg_id})
+dirname=$(dirname "$output")
+test_expect_equal "$dirname" "$MAIL_DIR/new"
+
 test_begin_subtest "Insert message into folder with trailing /"
 gen_insert_msg
 notmuch insert --folder=Drafts/ < "$gen_msg_filename"
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] cli: add support for only printing the addresses in notmuch address

2017-11-02 Thread Jani Nikula
The notmuch address output is much more useful for scripts with just
the addresses printed. Support this using the --output=address option.
---
 completion/notmuch-completion.bash |  2 +-
 doc/man1/notmuch-address.rst   |  7 ++-
 notmuch-search.c   |  7 ++-
 test/T095-address.sh   | 36 
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/completion/notmuch-completion.bash 
b/completion/notmuch-completion.bash
index 7aae4297ae0e..4e94275e3193 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -468,7 +468,7 @@ _notmuch_address()
return
;;
--output)
-   COMPREPLY=( $( compgen -W "sender recipients count" -- "${cur}" ) )
+   COMPREPLY=( $( compgen -W "sender recipients count address" -- 
"${cur}" ) )
return
;;
--sort)
diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index dbac20f7b012..68415d13c5b6 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -29,7 +29,7 @@ Supported options for **address** include
 intended for programs that invoke **notmuch(1)** internally. If
 omitted, the latest supported version will be used.
 
-``--output=(sender|recipients|count)``
+``--output=(sender|recipients|count|address)``
 
 Controls which information appears in the output. This option
 can be given multiple times to combine different outputs.
@@ -55,6 +55,11 @@ Supported options for **address** include
 Note: With this option, addresses are printed only after
 the whole search is finished. This may take long time.
 
+**address**
+Output only the email addresses instead of the full
+mailboxes with names and email addresses. This option has
+no effect on the JSON or S-Expression output formats.
+
 ``--deduplicate=(no|mailbox|address)``
 
 Control the deduplication of results.
diff --git a/notmuch-search.c b/notmuch-search.c
index 0abac08eb7ab..8f467db43cf0 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -34,6 +34,7 @@ typedef enum {
 OUTPUT_SENDER  = 1 << 5,
 OUTPUT_RECIPIENTS  = 1 << 6,
 OUTPUT_COUNT   = 1 << 7,
+OUTPUT_ADDRESS = 1 << 8,
 } output_t;
 
 typedef enum {
@@ -370,7 +371,10 @@ print_mailbox (const search_context_t *ctx, const 
mailbox_t *mailbox)
format->integer (format, count);
format->string (format, "\t");
}
-   format->string (format, name_addr);
+   if (ctx->output & OUTPUT_ADDRESS)
+   format->string (format, addr);
+   else
+   format->string (format, name_addr);
format->separator (format);
 } else {
format->begin_map (format);
@@ -877,6 +881,7 @@ notmuch_address_command (notmuch_config_t *config, int 
argc, char *argv[])
  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
  { "recipients", OUTPUT_RECIPIENTS },
  { "count", OUTPUT_COUNT },
+ { "address", OUTPUT_ADDRESS },
  { 0, 0 } } },
{ .opt_keyword = >exclude, .name = "exclude", .keywords =
  (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
diff --git a/test/T095-address.sh b/test/T095-address.sh
index f0291d29ec43..817be5380a45 100755
--- a/test/T095-address.sh
+++ b/test/T095-address.sh
@@ -119,6 +119,42 @@ cat OUTPUT
+cat OUTPUT
+cat 

[PATCH] test: fix test database backup/restore location

2017-10-24 Thread Jani Nikula
backup_database() and restore_database() used to store the backups in
the test specific temporary directory, through the current working
directory being there. Commit 8e7fb88237ae ("test: use source and
build paths in test-lib-common.sh") started using a test specific
backup directories under the build tree test directory. This was in
error. Switch back to the old location, but using paths to the
location instead of relying on current working directory.

Reported by Daniel Kahn Gillmor .
---
 test/test-lib-common.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
index 4300eb65418f..6c3571d4c560 100644
--- a/test/test-lib-common.sh
+++ b/test/test-lib-common.sh
@@ -31,14 +31,14 @@ fi
 
 backup_database () {
 test_name=$(basename $0 .sh)
-rm -rf $NOTMUCH_BUILDDIR/test/notmuch-dir-backup."$test_name"
-cp -pR ${MAIL_DIR}/.notmuch 
$NOTMUCH_BUILDDIR/test/notmuch-dir-backup."${test_name}"
+rm -rf $TMP_DIRECTORY/notmuch-dir-backup."$test_name"
+cp -pR ${MAIL_DIR}/.notmuch 
$TMP_DIRECTORY/notmuch-dir-backup."${test_name}"
 }
 
 restore_database () {
 test_name=$(basename $0 .sh)
 rm -rf ${MAIL_DIR}/.notmuch
-cp -pR $NOTMUCH_BUILDDIR/test/notmuch-dir-backup."${test_name}" 
${MAIL_DIR}/.notmuch
+cp -pR $TMP_DIRECTORY/notmuch-dir-backup."${test_name}" 
${MAIL_DIR}/.notmuch
 }
 
 # Test the binaries we have just built.  The tests are kept in
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: nmbug backtrace due to .mailmap

2017-10-22 Thread Jani Nikula
On Mon, 16 Oct 2017, "W. Trevor King"  wrote:
> b. Comment out the ‘_insist_committed()’ line in nmbug's ‘pull’
>definition and try again.  Make sure you restore the check after
>the successful pull.

FWIW this fixed it for me, thanks.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: web interface to notmuch

2017-10-21 Thread Jani Nikula
On Thu, 19 Oct 2017, Daniel Kahn Gillmor  wrote:
> On Thu 2017-10-19 11:01:53 -0400, Brian Sniffen wrote:
>> I put together something like this, visible at
>> https://github.com/briansniffen/notmuch/tree/nmweb/contrib/notmuch-web
>>
>> It's not much of a service.  I am pretty sure it is exploitable---that
>> content in text/html parts of messages can do Bad Things to your
>> session.
>
> I think this is the crux of the problem, right?  I was noticing the
> other day that notmuch's own mail archives are published in pipermail,
> which is *absolutely terrible* compared to dealing with a mailstore with
> notmuch as a frontend.  I'd love to be able to expose the archive to the
> public this way.

For the list archive, we could restrict to displaying text/plain only.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] NEWS: test suite out-of-tree builds

2017-10-21 Thread Jani Nikula
Out-of-tree builds now work and supersede --root option.
---
 NEWS | 13 +
 1 file changed, 13 insertions(+)

diff --git a/NEWS b/NEWS
index 3c8198d3ef12..c2ebb6447bd0 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,16 @@
+Notmuch 0.26 (UNRELEASED)
+=
+
+Test Suite
+--
+
+Out-of-tree builds
+
+  The test suite now works properly with out-of-tree builds, i.e. with
+  separate source and build directories. The --root option to tests
+  has been dropped. The same can now be achieved more reliably using
+  out-of-tree builds.
+
 Notmuch 0.25.1 (2017-09-11)
 ===
 
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: remove --root option and fix TMP_DIRECTORY cleanup

2017-10-21 Thread Jani Nikula
On Sat, 21 Oct 2017, David Bremner <da...@tethera.net> wrote:
> Jani Nikula <j...@nikula.org> writes:
>
>> The primary motivation here is to fix TMP_DIRECTORY cleanup prior to
>> running each test when the current working directory is not the test
>> subdirectory. Tests with failures would leave their TMP_DIRECTORY
>> directory behind for debugging, and repeated out-of-tree test runs
>> would have old temp directories. (This lead to e.g. T310-emacs.sh
>> hanging because emacs would prompt for overwriting files.)
>>
>> We remove the likely anyway defunct --root test option while at it,
>> just to be on the safe side when doing 'rm -rf' on the TMP_DIRECTORY.
>
> This seems to fix the problem I reported, however applying
>
>  
> id:cd03efa9c93ee54f7e7f7e166079062984ddd658.1506370901.git.j...@nikula.org
>
> on top breaks
>
>   ./devel/check-out-of-tree-build.sh test
>
> I get lots of errors along the lines of
>
>   Error opening database at
>   /tmp/tmp.GD3HPpejbL/test/tmp.T350-crypto/mail/.notmuch: No such
>   file or directory
>
> So this patch seems to unbreak in-tree builds (and out of tree builds
> where we copy the whole test hierarchy), and is probably worth applying
> as is, but I wanted to give you a chance to respond before I proceed.

So these two should be applied first, in either order:

id:20171021192141.-1-j...@nikula.org
id:20171021115802.31197-1-j...@nikula.org

And then this should work on top:

id:cd03efa9c93ee54f7e7f7e166079062984ddd658.1506370901.git.j...@nikula.org

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2] test: use source path in add_gnupg_home

2017-10-21 Thread Jani Nikula
Make a distinction between source and build directories.

---

v2 rebase, add_gnupg_home is now in test-lib.sh
---
 test/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 7926d2787710..5274e6e1cf2c 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -109,7 +109,7 @@ add_gnupg_home ()
 _gnupg_exit () { gpgconf --kill all 2>/dev/null || true; }
 at_exit_function _gnupg_exit
 mkdir -m 0700 "$GNUPGHOME"
-gpg --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc 
>"$GNUPGHOME"/import.log 2>&1
+gpg --no-tty --import <$NOTMUCH_SRCDIR/test/gnupg-secret-key.asc 
>"$GNUPGHOME"/import.log 2>&1
 test_debug "cat $GNUPGHOME/import.log"
 if (gpg --quick-random --version >/dev/null 2>&1) ; then
echo quick-random >> "$GNUPGHOME"/gpg.conf
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] lib: add support for thread: queries

2017-10-21 Thread Jani Nikula
On Sat, 21 Oct 2017, David Bremner <da...@tethera.net> wrote:
> Jani Nikula <j...@nikula.org> writes:
>
>> On Tue, 17 Oct 2017, David Bremner <da...@tethera.net> wrote:
>>> Jani Nikula <j...@nikula.org> writes:
>>>
>>> The other field processors throw Xapian::QueryParserErrors in case bad
>>> things happen (e.g. failure to read from the database). This seems to be
>>> better than nothing as it allows transmitting some detail to the
>>> user. See e.g. "regex error reporting" in test/T650-regexp-query.sh. And
>>> of course, speaking of tests, this should have one.
>>
>> I'm not sure what you're suggesting. Do you mean I should open-code
>> notmuch_database_find_message() or notmuch_message_get_thread_id() to
>> get at the exceptions thrown within them?
>>
>> BR,
>> Jani.
>
> nothing so fancy. Consider the following bit of query-fp.cc
>
> status = notmuch_database_get_config (notmuch, key.c_str (), );
> if (status) {
>   throw Xapian::QueryParserError ("error looking up key" + name);
> }
>
>
> That could no doubt be improved to report the specifics of `status`, but
> it's already better than silently ignoring the error, as this will
> eventually be reported back to library clients. I agree the exception ->
> error-code -> exception -> error-code  path is pretty gross, but thats
> the price we pay for mostly pretending we're not writing C++.

Okay. Seems like we are in agreement we want this feature, so I'll fix
that up and write some tests when I have a moment.

Thanks,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: remove --root option and fix TMP_DIRECTORY cleanup

2017-10-21 Thread Jani Nikula
On Sat, 21 Oct 2017, Jani Nikula <j...@nikula.org> wrote:
> The primary motivation here is to fix TMP_DIRECTORY cleanup prior to
> running each test when the current working directory is not the test
> subdirectory. Tests with failures would leave their TMP_DIRECTORY
> directory behind for debugging, and repeated out-of-tree test runs
> would have old temp directories. (This lead to e.g. T310-emacs.sh
> hanging because emacs would prompt for overwriting files.)

To clarify, plain 'make test' no longer has its CWD in the test
subdirectory either, leading to the same problem as out-of-tree runs
here.

The problem noticed by David could also be reproduced by running
'test/T310-emacs.sh; test/T310-emacs.sh' but not 'cd test;
./T310-emacs.sh; ./T310-emacs.sh'.

BR,
Jani.

>
> We remove the likely anyway defunct --root test option while at it,
> just to be on the safe side when doing 'rm -rf' on the TMP_DIRECTORY.
> ---
>  test/README | 9 -
>  test/test-lib-common.sh | 8 ++--
>  test/test-lib.sh| 3 ---
>  3 files changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/test/README b/test/README
> index 8e06f44241a6..b378c3ff3c5f 100644
> --- a/test/README
> +++ b/test/README
> @@ -80,15 +80,6 @@ The following command-line options are available when 
> running tests:
>   As the names depend on the tests' file names, it is safe to
>   run the tests with this option in parallel.
>  
> ---root=::
> - This runs the testsuites specified under a separate directory.
> - However, caution is advised, as not all tests are maintained
> - with this relocation in mind, so some tests may behave
> - differently.
> -
> - Pointing this argument at a tmpfs filesystem can improve the
> - speed of the test suite for some users.
> -
>  Certain tests require precomputed databases to complete. You can fetch these
>  databases with
>  
> diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
> index 5e53348a9438..4300eb65418f 100644
> --- a/test/test-lib-common.sh
> +++ b/test/test-lib-common.sh
> @@ -307,13 +307,9 @@ export PATH MANPATH
>  
>  # Test repository
>  test="tmp.$(basename "$0" .sh)"
> -test -n "$root" && test="$root/$test"
> -case "$test" in
> -/*) TMP_DIRECTORY="$test" ;;
> - *) TMP_DIRECTORY="$TEST_DIRECTORY/$test" ;;
> -esac
> +TMP_DIRECTORY="$TEST_DIRECTORY/$test"
>  test ! -z "$debug" || remove_tmp=$TMP_DIRECTORY
> -rm -fr "$test" || {
> +rm -rf "$TMP_DIRECTORY" || {
>   GIT_EXIT_OK=t
>   echo >&6 "FATAL: Cannot prepare test area"
>   exit 1
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 4619c327dd02..7926d2787710 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -160,9 +160,6 @@ do
>   valgrind=t; verbose=t; shift ;;
>   --tee)
>   shift ;; # was handled already
> - --root=*)
> - root=$(expr "z$1" : 'z[^=]*=\(.*\)')
> - shift ;;
>   *)
>   echo "error: unknown test option '$1'" >&2; exit 1 ;;
>   esac
> -- 
> 2.11.0
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: remove --root option and fix TMP_DIRECTORY cleanup

2017-10-21 Thread Jani Nikula
The primary motivation here is to fix TMP_DIRECTORY cleanup prior to
running each test when the current working directory is not the test
subdirectory. Tests with failures would leave their TMP_DIRECTORY
directory behind for debugging, and repeated out-of-tree test runs
would have old temp directories. (This lead to e.g. T310-emacs.sh
hanging because emacs would prompt for overwriting files.)

We remove the likely anyway defunct --root test option while at it,
just to be on the safe side when doing 'rm -rf' on the TMP_DIRECTORY.
---
 test/README | 9 -
 test/test-lib-common.sh | 8 ++--
 test/test-lib.sh| 3 ---
 3 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/test/README b/test/README
index 8e06f44241a6..b378c3ff3c5f 100644
--- a/test/README
+++ b/test/README
@@ -80,15 +80,6 @@ The following command-line options are available when 
running tests:
As the names depend on the tests' file names, it is safe to
run the tests with this option in parallel.
 
---root=::
-   This runs the testsuites specified under a separate directory.
-   However, caution is advised, as not all tests are maintained
-   with this relocation in mind, so some tests may behave
-   differently.
-
-   Pointing this argument at a tmpfs filesystem can improve the
-   speed of the test suite for some users.
-
 Certain tests require precomputed databases to complete. You can fetch these
 databases with
 
diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
index 5e53348a9438..4300eb65418f 100644
--- a/test/test-lib-common.sh
+++ b/test/test-lib-common.sh
@@ -307,13 +307,9 @@ export PATH MANPATH
 
 # Test repository
 test="tmp.$(basename "$0" .sh)"
-test -n "$root" && test="$root/$test"
-case "$test" in
-/*) TMP_DIRECTORY="$test" ;;
- *) TMP_DIRECTORY="$TEST_DIRECTORY/$test" ;;
-esac
+TMP_DIRECTORY="$TEST_DIRECTORY/$test"
 test ! -z "$debug" || remove_tmp=$TMP_DIRECTORY
-rm -fr "$test" || {
+rm -rf "$TMP_DIRECTORY" || {
GIT_EXIT_OK=t
echo >&6 "FATAL: Cannot prepare test area"
exit 1
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 4619c327dd02..7926d2787710 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -160,9 +160,6 @@ do
valgrind=t; verbose=t; shift ;;
--tee)
shift ;; # was handled already
-   --root=*)
-   root=$(expr "z$1" : 'z[^=]*=\(.*\)')
-   shift ;;
*)
echo "error: unknown test option '$1'" >&2; exit 1 ;;
esac
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] lib: add support for thread: queries

2017-10-21 Thread Jani Nikula
On Tue, 17 Oct 2017, David Bremner <da...@tethera.net> wrote:
> Jani Nikula <j...@nikula.org> writes:
>
>
>> +if (strchr (thread.c_str (), '@'))
>> +notmuch_database_find_message (notmuch, thread.c_str (), );
>> +
>> +if (message) {
>> +thread_id = notmuch_message_get_thread_id (message);
>> +notmuch_message_destroy (message);
>> +} else {
>> +thread_id = thread;
>> +}
>> +
>> +return Xapian::Query (term_prefix + thread_id);
>> +}
>
> The other field processors throw Xapian::QueryParserErrors in case bad
> things happen (e.g. failure to read from the database). This seems to be
> better than nothing as it allows transmitting some detail to the
> user. See e.g. "regex error reporting" in test/T650-regexp-query.sh. And
> of course, speaking of tests, this should have one.

I'm not sure what you're suggesting. Do you mean I should open-code
notmuch_database_find_message() or notmuch_message_get_thread_id() to
get at the exceptions thrown within them?

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


nmbug backtrace due to .mailmap

2017-10-16 Thread Jani Nikula

Presumably because of the .mailmap added in nmbug repository:

commit 5ea99ebcb7d188003bf4854b566f6d21721f5ea1
Author: W. Trevor King 
Date:   Mon Oct 16 10:46:27 2017 -0700

.mailmap: Consolidate David and Tomi's author names

The format is documented in git-shortlog(1).  In both cases, I've
prefered the email used for their most-recent commit.

A   .mailmap

I now get backtrace trying to nmbug pull:

Traceback (most recent call last):
  File "/home/jani/bin/nmbug", line 834, in 
args.func(**kwargs)
  File "/home/jani/bin/nmbug", line 433, in pull
_insist_committed()
  File "/home/jani/bin/nmbug", line 412, in _insist_committed
status = get_status()
  File "/home/jani/bin/nmbug", line 580, in get_status
maybe_deleted = _diff_index(index=index, filter='D')
  File "/home/jani/bin/nmbug", line 658, in _diff_index
for id, tag in _unpack_diff_lines(stream=p.stdout):
  File "/home/jani/bin/nmbug", line 678, in _unpack_diff_lines
'Invalid line in diff: {!r}'.format(line.strip()))
ValueError: Invalid line in diff: u'.mailmap'

I also didn't see any discussion about adding a .mailmap prior to it
being pushed.


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] lib: add support for thread: queries

2017-10-16 Thread Jani Nikula
Add support for querying threads using message-ids in addition to
thread-ids. The main benefit is that thread queries via message-ids
are portable across databases, re-indexing, and thread joining, while
thread ids can be somewhat transient. A thread: query can
be shared between notmuch users, while a thread: query may
cease to work after the regular delivery of a message that joins
threads.

What previously required:

$ notmuch search $(notmuch search --output=threads id:)

can now be reduced to:

$ notmuch search thread:

We limit the query to message-ids that have @ to optimize regular
thread-id queries and to avoid collisions with thread-ids which are
guaranteed (or which we can guarantee) to not contain @. This seems
reasonable, as valid message-ids will have @.

The performance penalty for regular thread-id queries seems to be
neglible, and thread queries via message-ids seem to be about 10%
slower than the direct thread-id query counterpart.
---
 doc/man7/notmuch-search-terms.rst |  9 ---
 lib/Makefile.local|  1 +
 lib/database.cc   |  6 -
 lib/thread-fp.cc  | 53 +++
 lib/thread-fp.h   | 39 
 5 files changed, 104 insertions(+), 4 deletions(-)
 create mode 100644 lib/thread-fp.cc
 create mode 100644 lib/thread-fp.h

diff --git a/doc/man7/notmuch-search-terms.rst 
b/doc/man7/notmuch-search-terms.rst
index b27f31f7545c..f3723ceb959b 100644
--- a/doc/man7/notmuch-search-terms.rst
+++ b/doc/man7/notmuch-search-terms.rst
@@ -52,7 +52,7 @@ indicate user-supplied values):
 
 -  id:
 
--  thread:
+-  thread: or thread:
 
 -  folder:
 
@@ -101,10 +101,13 @@ For **id:**, message ID values are the literal contents 
of the
 Message-ID: header of email messages, but without the '<', '>'
 delimiters.
 
-The **thread:** prefix can be used with the thread ID values that are
+The **thread:** prefix can be used with either the message ID of any
+of the messages in the thread or the thread ID values that are
 generated internally by notmuch (and do not appear in email messages).
 These thread ID values can be seen in the first column of output from
-**notmuch search**
+**notmuch search**. The thread ID values are transient. Thread queries
+by message ID are only available if notmuch is built with **Xapian
+Field Processors** (see below).
 
 The **path:** prefix searches for email messages that are in
 particular directories within the mail store. The directory must be
diff --git a/lib/Makefile.local b/lib/Makefile.local
index 8aa03891d775..123ef04c64a9 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -58,6 +58,7 @@ libnotmuch_cxx_srcs = \
$(dir)/query-fp.cc  \
$(dir)/config.cc\
$(dir)/regexp-fields.cc \
+   $(dir)/thread-fp.cc \
$(dir)/thread.cc
 
 libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
diff --git a/lib/database.cc b/lib/database.cc
index 35c66939bdcf..82d2dcf22aa0 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -21,6 +21,7 @@
 #include "database-private.h"
 #include "parse-time-vrp.h"
 #include "query-fp.h"
+#include "thread-fp.h"
 #include "regexp-fields.h"
 #include "string-util.h"
 
@@ -258,7 +259,8 @@ prefix_t prefix_table[] = {
 { "directory", "XDIRECTORY",   NOTMUCH_FIELD_NO_FLAGS },
 { "file-direntry", "XFDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS },
 { "directory-direntry","XDDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS },
-{ "thread","G",NOTMUCH_FIELD_EXTERNAL 
},
+{ "thread","G",NOTMUCH_FIELD_EXTERNAL |
+   NOTMUCH_FIELD_PROCESSOR },
 { "tag",   "K",NOTMUCH_FIELD_EXTERNAL |
NOTMUCH_FIELD_PROCESSOR },
 { "is","K",NOTMUCH_FIELD_EXTERNAL |
@@ -317,6 +319,8 @@ _setup_query_field (const prefix_t *prefix, 
notmuch_database_t *notmuch)
fp = (new DateFieldProcessor())->release ();
else if (STRNCMP_LITERAL(prefix->name, "query") == 0)
fp = (new QueryFieldProcessor (*notmuch->query_parser, 
notmuch))->release ();
+   else if (STRNCMP_LITERAL(prefix->name, "thread") == 0)
+   fp = (new ThreadFieldProcessor (notmuch))->release ();
else
fp = (new RegexpFieldProcessor (prefix->name, prefix->flags,
*notmuch->query_parser, 
notmuch))->release ();
diff --git a/lib/thread-fp.cc b/lib/thread-fp.cc
new file mode 100644
index ..f15aabba18c0
--- /dev/nul

[PATCH] cli/crypto: fix segfault on failed gmime2 crypto context creation

2017-10-16 Thread Jani Nikula
Commit 1fdc08d0ffab ("cli/crypto: treat failure to create a crypto
context as fatal.") started treating crypto context creation failures
"as fatal", returning NULL from _mime_node_create().

Unfortunately, we do not have NULL checks for _mime_node_create()
failures. The only caller, mime_node_child(), could check and return
NULL (as it's documented to do on errors) but none of the several call
sites have NULL checks either. And none of them really have a trivial
but feasible and graceful way of recovery.

So while the right thing to do would be to handle NULL returns
properly all over the place, and we have other scenarios that do
return NULL from above mentioned functions, the crypto context
creation failure is something that does seem to show up regularly in
some scenarios, revert back to the functionality before commit
1fdc08d0ffab as an interim fix.
---
 mime-node.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mime-node.c b/mime-node.c
index d48be4c46695..27de72fa2f84 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -271,7 +271,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
const char *protocol = g_mime_content_type_get_parameter (content_type, 
"protocol");
cryptoctx = _notmuch_crypto_get_gmime_context (node->ctx->crypto, 
protocol);
if (!cryptoctx)
-   return NULL;
+   return node;
 }
 #endif
 
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/3] cli: add support for --no- prefixed boolean and keyword flag arguments

2017-10-14 Thread Jani Nikula
On Sat, 14 Oct 2017, William Casarin <j...@jb55.com> wrote:
> Hey Jani,
>
> Patches look good so far, concept ack for sure.
>
>
> Jani Nikula <j...@nikula.org> writes:
>
>> For example, you can use --no-exclude instead of --exclude=false in
>> notmuch show. If we had keyword flag arguments with some flags
>> defaulting to on, say --include=tags in notmuch dump/restore, this
>> would allow --no-include=tags to switch that off while not affecting
>> other flags.
>
> I've been testing it a bit, I can't seem to make this work in this example:
>
> ./notmuch count --no-exclude
>
> After some brief investigation it might be because count is using
> EXCLUDE_true(1) and EXCLUDE_false(0) which are not equal to
> NOTMUCH_EXCLUDE_TRUE(1) and NOTMUCH_EXCLUDE_FALSE(2), but I'm not sure.

*blush* I screwed those enums up. Here's a patch that takes care of both
issues id:20171014201836.4486-1-j...@nikula.org. It's independent of
this series.

BR,
Jani.


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] cli: make notmuch count --exclude a boolean argument

2017-10-14 Thread Jani Nikula
Commit 0f314c0c99be ("cli: convert notmuch_bool_t to stdbool")
over-eagerly converted EXCLUDE_TRUE and EXCLUDE_FALSE to EXCLUDE_true
and EXCLUDE_false in notmuch-count.c. We could just fix the case back,
but convert the option to an actual boolean argument instead.

We've used a keyword argument rather than a boolean argument for
notmuch count --exclude for five years, since commit 785c1e497f05
("cli: move count to the new --exclude=(true|false|flag) naming
scheme."), "to allow future options to be added more easily". I think
we can conclude future options aren't coming any time soon.
---
 notmuch-count.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index 1ae7d5146d92..ca05c9793b70 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -27,12 +27,6 @@ enum {
 OUTPUT_FILES,
 };
 
-/* The following is to allow future options to be added more easily */
-enum {
-EXCLUDE_true,
-EXCLUDE_false,
-};
-
 /* Return the number of files matching the query, or -1 for an error */
 static int
 count_files (notmuch_query_t *query)
@@ -160,7 +154,7 @@ notmuch_count_command (notmuch_config_t *config, int argc, 
char *argv[])
 char *query_str;
 int opt_index;
 int output = OUTPUT_MESSAGES;
-int exclude = EXCLUDE_true;
+bool exclude = true;
 const char **search_exclude_tags = NULL;
 size_t search_exclude_tags_length = 0;
 bool batch = false;
@@ -175,10 +169,7 @@ notmuch_count_command (notmuch_config_t *config, int argc, 
char *argv[])
  { "messages", OUTPUT_MESSAGES },
  { "files", OUTPUT_FILES },
  { 0, 0 } } },
-   { .opt_keyword = , .name = "exclude", .keywords =
- (notmuch_keyword_t []){ { "true", EXCLUDE_true },
- { "false", EXCLUDE_false },
- { 0, 0 } } },
+   { .opt_bool = , .name = "exclude" },
{ .opt_bool = _lastmod, .name = "lastmod" },
{ .opt_bool = , .name = "batch" },
{ .opt_string = _file_name, .name = "input" },
@@ -221,7 +212,7 @@ notmuch_count_command (notmuch_config_t *config, int argc, 
char *argv[])
return EXIT_FAILURE;
 }
 
-if (exclude == EXCLUDE_true) {
+if (exclude) {
search_exclude_tags = notmuch_config_get_search_exclude_tags
(config, _exclude_tags_length);
 }
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/3] cli: use the negating boolean support for new and insert --no-hooks

2017-10-14 Thread Jani Nikula
This lets us use the positive hooks variable in code, increasing
clarity.
---
 notmuch-insert.c | 6 +++---
 notmuch-new.c| 8 
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 32be74193472..6878313e188f 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -455,7 +455,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
 const char *folder = "";
 bool create_folder = false;
 bool keep = false;
-bool no_hooks = false;
+bool hooks = true;
 bool synchronize_flags;
 char *maildir;
 char *newpath;
@@ -466,7 +466,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
{ .opt_string = , .name = "folder" },
{ .opt_bool = _folder, .name = "create-folder" },
{ .opt_bool = , .name = "keep" },
-   { .opt_bool =  _hooks, .name = "no-hooks" },
+   { .opt_bool = , .name = "hooks" },
{ .opt_inherit = notmuch_shared_options },
{ }
 };
@@ -573,7 +573,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
}
 }
 
-if (! no_hooks && status == NOTMUCH_STATUS_SUCCESS) {
+if (hooks && status == NOTMUCH_STATUS_SUCCESS) {
/* Ignore hook failures. */
notmuch_run_hook (db_path, "post-insert");
 }
diff --git a/notmuch-new.c b/notmuch-new.c
index 0f50457eb894..a6ca484101ce 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -954,7 +954,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
 int opt_index;
 unsigned int i;
 bool timer_is_active = false;
-bool no_hooks = false;
+bool hooks = true;
 bool quiet = false, verbose = false;
 notmuch_status_t status;
 
@@ -962,7 +962,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
{ .opt_bool = , .name = "quiet" },
{ .opt_bool = , .name = "verbose" },
{ .opt_bool = _files_state.debug, .name = "debug" },
-   { .opt_bool = _hooks, .name = "no-hooks" },
+   { .opt_bool = , .name = "hooks" },
{ .opt_inherit = notmuch_shared_options },
{ }
 };
@@ -995,7 +995,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
}
 }
 
-if (!no_hooks) {
+if (hooks) {
ret = notmuch_run_hook (db_path, "pre-new");
if (ret)
return EXIT_FAILURE;
@@ -1160,7 +1160,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
 
 notmuch_database_destroy (notmuch);
 
-if (!no_hooks && !ret && !interrupted)
+if (hooks && !ret && !interrupted)
ret = notmuch_run_hook (db_path, "post-new");
 
 if (ret || interrupted)
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/2] test: test regexp based new.ignore

2017-10-14 Thread Jani Nikula
Just some basics.
---
 test/T050-new.sh | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 272ed417aa2e..ee9ce08c8e86 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -259,6 +259,28 @@ ln -s i_do_not_exist "${MAIL_DIR}"/broken_link
 output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" "No new mail."
 
+test_begin_subtest "Ignore files and directories specified in new.ignore 
(regexp)"
+notmuch config set new.ignore ".git" "/^bro.*ink\$/" "/ignored.*file/"
+output=$(NOTMUCH_NEW --debug 2>&1 | sort)
+test_expect_equal "$output" \
+"(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/broken_link
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/two/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/two/three/.git
+(D) add_files, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/broken_link
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/two/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/two/three/.git
+(D) add_files, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
+No new mail."
+
 test_begin_subtest "Quiet: No new mail."
 output=$(NOTMUCH_NEW --quiet)
 test_expect_equal "$output" ""
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] test: expand argument parsing sanity checks

2017-10-14 Thread Jani Nikula
Test the various boolean formats and --no- prefixed boolean and
keyword flag arguments.
---
 test/T410-argument-parsing.sh | 28 
 1 file changed, 28 insertions(+)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 4a2b25c6486d..243d0241b9b6 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -37,4 +37,32 @@ positional arg 1 false
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "--boolean=true"
+$TEST_DIRECTORY/arg-test --boolean=true > OUTPUT
+cat < EXPECTED
+boolean 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--boolean=false"
+$TEST_DIRECTORY/arg-test --boolean=false > OUTPUT
+cat < EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--no-boolean"
+$TEST_DIRECTORY/arg-test --no-boolean > OUTPUT
+cat < EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--no-flag"
+$TEST_DIRECTORY/arg-test --flag=one --flag=three --no-flag=three > OUTPUT
+cat < EXPECTED
+flags 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/2] cli/new: support // in new.ignore

2017-10-14 Thread Jani Nikula
Add support for using // style regular expressions in
new.ignore, mixed with the old style verbatim file and directory
basenames. The regex is matched against the relative path from the
database path.
---
 doc/man1/notmuch-config.rst |  21 +--
 notmuch-new.c   | 134 
 2 files changed, 140 insertions(+), 15 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 6a51e64f1517..0af86f9beee4 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -75,11 +75,22 @@ The available configuration items are described below.
 Default: ``unread;inbox``.
 
 **new.ignore**
-A list of file and directory names, without path, that will not
-be searched for messages by **notmuch new**. All the files and
-directories matching any of the names specified here will be
-ignored, regardless of the location in the mail store directory
-hierarchy.
+A list to specify files and directories that will not be
+searched for messages by **notmuch new**. Each entry in the
+list is either:
+
+  A file or a directory name, without path, that will be
+  ignored, regardless of the location in the mail store
+  directory hierarchy.
+
+Or:
+
+  A regular expression delimited with // that will be matched
+  against the path of the file or directory relative to the
+  database path. Matching files and directories will be
+  ignored. The beginning and end of string must be explictly
+  anchored. For example, /.*/foo$/ would match "bar/foo" and
+  "bar/baz/foo", but not "foo" or "bar/foobar".
 
 Default: empty list.
 
diff --git a/notmuch-new.c b/notmuch-new.c
index 0f50457eb894..5a262e419b44 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -42,13 +42,17 @@ enum verbosity {
 };
 
 typedef struct {
+const char *db_path;
+
 int output_is_a_tty;
 enum verbosity verbosity;
 bool debug;
 const char **new_tags;
 size_t new_tags_length;
-const char **new_ignore;
-size_t new_ignore_length;
+const char **ignore_verbatim;
+size_t ignore_verbatim_length;
+regex_t *ignore_regex;
+size_t ignore_regex_length;
 
 int total_files;
 int processed_files;
@@ -240,18 +244,125 @@ _special_directory (const char *entry)
 return strcmp (entry, ".") == 0 || strcmp (entry, "..") == 0;
 }
 
+static bool
+_setup_ignore (notmuch_config_t *config, add_files_state_t *state)
+{
+const char **ignore_list, **ignore;
+int nregex = 0, nverbatim = 0;
+const char **verbatim = NULL;
+regex_t *regex = NULL;
+
+ignore_list = notmuch_config_get_new_ignore (config, NULL);
+if (! ignore_list)
+   return true;
+
+for (ignore = ignore_list; *ignore; ignore++) {
+   const char *s = *ignore;
+   size_t len = strlen (s);
+
+   if (len == 0) {
+   fprintf (stderr, "Error: Empty string in new.ignore list\n");
+   return false;
+   }
+
+   if (s[0] == '/') {
+   regex_t *preg;
+   char *r;
+   int rerr;
+
+   if (len < 3 || s[len - 1] != '/') {
+   fprintf (stderr, "Error: Malformed pattern '%s' in 
new.ignore\n",
+s);
+   return false;
+   }
+
+   r = talloc_strndup (config, s + 1, len - 2);
+   regex = talloc_realloc (config, regex, regex_t, nregex + 1);
+   preg = [nregex];
+
+   rerr = regcomp (preg, r, REG_EXTENDED | REG_NOSUB);
+   if (rerr) {
+   size_t error_size = regerror (rerr, preg, NULL, 0);
+   char *error = talloc_size (r, error_size);
+
+   regerror (rerr, preg, error, error_size);
+
+   fprintf (stderr, "Error: Invalid regex '%s' in new.ignore: 
%s\n",
+r, error);
+   return false;
+   }
+   nregex++;
+
+   talloc_free (r);
+   } else {
+   verbatim = talloc_realloc (config, verbatim, const char *,
+  nverbatim + 1);
+   verbatim[nverbatim++] = s;
+   }
+}
+
+state->ignore_regex = regex;
+state->ignore_regex_length = nregex;
+state->ignore_verbatim = verbatim;
+state->ignore_verbatim_length = nverbatim;
+
+return true;
+}
+
+static char *
+_get_relative_path (const char *db_path, const char *dirpath, const char 
*entry)
+{
+size_t db_path_len = strlen (db_path);
+
+/* paranoia? */
+if (strncmp (dirpath, db_path, db_path_len) != 0) {
+   fprintf (stderr, "Warning: '%s' is not a subdirectory of '%s'\n",
+dirpath, db_path);
+   return NULL;
+}
+
+dirpath += db_path_len;
+while (*dirpath == '/')
+   dirpath++;
+
+if (*dirpath)
+   return talloc_asprintf (NULL, "%s/%s", dirpath, entry);
+else
+   return 

[PATCH 1/3] cli: add support for --no- prefixed boolean and keyword flag arguments

2017-10-14 Thread Jani Nikula
Add transparent support for negating boolean and keyword flag
arguments using --no-argument style on the command line. That is, if
the option description contains a boolean or a keyword flag argument
named "argument", --no-argument will match and negate it.

For boolean arguments this obviously means the logical NOT. For
keyword flag arguments this means bitwise AND of the bitwise NOT,
i.e. masking out the specified bits instead of OR'ing them in.

For example, you can use --no-exclude instead of --exclude=false in
notmuch show. If we had keyword flag arguments with some flags
defaulting to on, say --include=tags in notmuch dump/restore, this
would allow --no-include=tags to switch that off while not affecting
other flags.

As a curiosity, you should be able to warp your brain using
--no-exclude=true meaning false and --no-exclude=false meaning true if
you wish.

Specifying both "argument" and "no-argument" style arguments in the
same option description should be avoided. In this case, --no-argument
would match whichever is specified first, and --argument would only
match "argument".
---
 command-line-arguments.c | 48 +---
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 1ff5aae578c6..69ee1cb07f47 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -11,8 +11,9 @@
 */
 
 static bool
-_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
-
+_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next,
+ const char *arg_str, bool negate)
+{
 const notmuch_keyword_t *keywords;
 
 if (next == '\0') {
@@ -24,7 +25,9 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
if (strcmp (arg_str, keywords->name) != 0)
continue;
 
-   if (arg_desc->opt_flags)
+   if (arg_desc->opt_flags && negate)
+   *arg_desc->opt_flags &= ~keywords->value;
+   else if (arg_desc->opt_flags)
*arg_desc->opt_flags |= keywords->value;
else
*arg_desc->opt_keyword = keywords->value;
@@ -39,7 +42,9 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
 }
 
 static bool
-_process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
+_process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next,
+ const char *arg_str, bool negate)
+{
 bool value;
 
 if (next == '\0' || strcmp (arg_str, "true") == 0) {
@@ -51,7 +56,7 @@ _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
return false;
 }
 
-*arg_desc->opt_bool = value;
+*arg_desc->opt_bool = negate ? !value : value;
 
 return true;
 }
@@ -139,6 +144,8 @@ parse_position_arg (const char *arg_str, int pos_arg_index,
 return false;
 }
 
+#define NEGATIVE_PREFIX "no-"
+
 /*
  * Search for a non-positional (i.e. starting with --) argument matching arg,
  * parse a possible value, and assign to *output_var
@@ -155,6 +162,14 @@ parse_option (int argc, char **argv, const 
notmuch_opt_desc_t *options, int opt_
 assert(options);
 
 const char *arg = _arg + 2; /* _arg starts with -- */
+const char *negative_arg = NULL;
+
+/* See if this is a --no-argument */
+if (strlen (arg) > strlen (NEGATIVE_PREFIX) &&
+   strncmp (arg, NEGATIVE_PREFIX, strlen (NEGATIVE_PREFIX)) == 0) {
+   negative_arg = arg + strlen (NEGATIVE_PREFIX);
+}
+
 const notmuch_opt_desc_t *try;
 
 const char *next_arg = NULL;
@@ -171,11 +186,22 @@ parse_option (int argc, char **argv, const 
notmuch_opt_desc_t *options, int opt_
if (! try->name)
continue;
 
-   if (strncmp (arg, try->name, strlen (try->name)) != 0)
+   char next;
+   const char *value;
+   bool negate = false;
+
+   if (strncmp (arg, try->name, strlen (try->name)) == 0) {
+   next = arg[strlen (try->name)];
+   value = arg + strlen (try->name) + 1;
+   } else if (negative_arg && (try->opt_bool || try->opt_flags) &&
+  strncmp (negative_arg, try->name, strlen (try->name)) == 0) {
+   next = negative_arg[strlen (try->name)];
+   value = negative_arg + strlen (try->name) + 1;
+   /* The argument part of --no-argument matches, negate the result. */
+   negate = true;
+   } else {
continue;
-
-   char next = arg[strlen (try->name)];
-   const char *value = arg + strlen(try->name) + 1;
+   }
 
/*
 * If we have not reached the end of the argument (i.e. the
@@ -194,9 +220,9 @@ parse_option (int argc, char **argv, const 
notmuch_opt_desc_t *options, int opt_
 
bool opt_status = false;
if (try->opt_keyword || try->opt_flags)
-   opt_status = _process_keyword_arg (try, next, value);
+   opt_status = 

[PATCH] cli: allow empty strings for notmuch insert --folder argument

2017-10-14 Thread Jani Nikula
Now that it's easy to add argument specific modifiers in opt
descriptions, add a new .allow_empty field to allow empty strings for
individual string arguments while retaining strict checks
elsewhere. Use this for notmuch insert --folder, where the empty
string means top level folder.

---

This patch addresses id:87y3owr22c@nikula.org
---
 command-line-arguments.c| 2 +-
 command-line-arguments.h| 3 +++
 doc/man1/notmuch-insert.rst | 3 ++-
 notmuch-insert.c| 2 +-
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 1ff5aae578c6..db73ca5efb89 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -81,7 +81,7 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, char 
next, const char *
fprintf (stderr, "Option \"%s\" needs a string argument.\n", 
arg_desc->name);
return false;
 }
-if (arg_str[0] == '\0') {
+if (arg_str[0] == '\0' && ! arg_desc->allow_empty) {
fprintf (stderr, "String argument for option \"%s\" must be 
non-empty.\n", arg_desc->name);
return false;
 }
diff --git a/command-line-arguments.h b/command-line-arguments.h
index 76ca4dcbb276..c0228f7cb634 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -32,6 +32,9 @@ typedef struct notmuch_opt_desc {
 /* Optional, if non-NULL, set to true if the option is present. */
 bool *present;
 
+/* Optional, allow empty strings for opt_string. */
+bool allow_empty;
+
 /* Must be set for opt_keyword and opt_flags. */
 const struct notmuch_keyword *keywords;
 } notmuch_opt_desc_t;
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index f79600d6571f..2f2466a6588b 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -34,7 +34,8 @@ Supported options for **insert** include
 ``--folder=<``\ folder\ **>**
 Deliver the message to the specified folder, relative to the
 top-level directory given by the value of **database.path**. The
-default is to deliver to the top-level directory.
+default is the empty string, which means delivering to the
+top-level directory.
 
 ``--create-folder``
 Try to create the folder named by the ``--folder`` option, if it
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 32be74193472..cff74731 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -463,7 +463,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
 unsigned int i;
 
 notmuch_opt_desc_t options[] = {
-   { .opt_string = , .name = "folder" },
+   { .opt_string = , .name = "folder", .allow_empty = true },
{ .opt_bool = _folder, .name = "create-folder" },
{ .opt_bool = , .name = "keep" },
{ .opt_bool =  _hooks, .name = "no-hooks" },
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/3] nmbug:

2017-10-14 Thread Jani Nikula
On Tue, 10 Oct 2017, "W. Trevor King"  wrote:
> I expect the best time to make that bump is just before we cut our
> next notmuch release.  Once we do, we can update the wiki page [1] to
> suggest nmbug 0.3+ and remove the checkout step from “Getting
> started”.

Could we add a "nmbug required version" metadata to the repository, and
add a version check in nmbug? Obviously this doesn't help with 0.2, but,
if we added this at 0.3, we could require a new nmbug version on the
clients.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [bug] [emacs] notmuch-show: names not shown on some mailing lists

2017-10-14 Thread Jani Nikula
On Tue, 10 Oct 2017, William Casarin  wrote:
> Hey there,
>
> Here's something I've noticed in some mailing list threads in notmuch-show:
>
>   https://jb55.com/s/81d7c740ef60984d.png
>
> It doesn't look like it is showing the correct name.
> Looking at the raw message, the From line looks like this:
>
>   From: Some Person via Some-mailinglist
> 
>   Reply-To: Some Person 
>
> Any way to handle these situations gracefully? Somehow show
> `Some Person` instead of `somemailingl...@lists.linuxfoundation.org`?

The information comes from the cli. Can you reproduce this using some
notmuch show --format=sexp --body=false query, perhaps on just one of
the messages?

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 04/10] index: implement notmuch_indexopts_t with try_decrypt

2017-10-14 Thread Jani Nikula
On Mon, 09 Oct 2017, Daniel Kahn Gillmor <d...@fifthhorseman.net> wrote:
> On Sat 2017-09-23 19:10:18 +0300, Jani Nikula wrote:
>>> --- a/lib/indexopts.c
>>> +++ b/lib/indexopts.c
>>> @@ -21,9 +21,27 @@
>>>  #include "notmuch-private.h"
>>>  
>>>  notmuch_indexopts_t *
>>> -notmuch_database_get_default_indexopts (notmuch_database_t unused (*db))
>>> +notmuch_database_get_default_indexopts (notmuch_database_t *db)
>>>  {
>>> -return NULL;
>>> +return talloc_zero (db, notmuch_indexopts_t);
>>
>> I wonder about the lifetime of indexopts. Should default indexopts be
>> part of the db object, so that your caller above doesn't have to
>> alloc/destroy it for every file?
>
> The caller doesn't have to alloc/destroy it for every file, they can
> alloc it once and pass it in for every file.

My point was, if the caller doesn't do it, it'll get alloced and
destroyed per file.

BR,
Jani.


>
> I'd rather not have the indexopts be part of the db object itself
> explicitly, because i can imagine a longer-running program wanting to
> create two indexopts objects, configuring them differently, and re-using
> one or the other without wanting to modify the database itself.
>
>> Our library interface has a leaky abstraction of the talloc hierarchical
>> refcounting. We don't talk about it in any of the docs, some of it is
>> implied, most of it is completely surprising if the library interface
>> user assumes a traditional C memory allocation model without
>> refcounting.
>
> right, probably the most surprising thing would be if the user got a
> default indexopts from one database object, and then tried to apply it
> to another database object, after having deleted the first database
> object.
>
> However, we *do* talk about it in the docs now, so i think we've given
> fair warning:
>
> -
> /**
>  * get the current default indexing options for a given database.
>  *
>  * This object will survive until the database itself is destroyed,
>  * but the caller may also release it earlier with
>  * notmuch_indexopts_destroy.
>  *
>  * This object represents a set of options on how a message can be
>  * added to the index.  At the moment it is a featureless stub.
>  *
>  * @since libnotmuch 5.1 (notmuch 0.26)
>  */
> notmuch_indexopts_t *
> notmuch_database_get_default_indexopts (notmuch_database_t *db);
> -
>
> --dkg
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/2] lib: convert notmuch_bool_t to stdbool internally

2017-10-07 Thread Jani Nikula
C99 stdbool turned 18 this year. There really is no reason to use our
own, except in the library interface for backward
compatibility. Convert the lib internally to stdbool.
---
 lib/add-message.cc  | 12 +-
 lib/built-with.c|  2 +-
 lib/config.cc   | 10 
 lib/database-private.h  |  6 ++---
 lib/database.cc | 30 
 lib/directory.cc|  8 +++
 lib/filenames.c |  2 +-
 lib/index.cc|  6 ++---
 lib/message-file.c  | 10 
 lib/message-private.h   |  2 +-
 lib/message-property.cc |  6 ++---
 lib/message.cc  | 62 -
 lib/messages.c  |  4 ++--
 lib/notmuch-private.h   | 13 ++-
 lib/query.cc| 42 -
 lib/string-map.c| 26 ++---
 lib/thread.cc   | 14 +--
 17 files changed, 128 insertions(+), 127 deletions(-)

diff --git a/lib/add-message.cc b/lib/add-message.cc
index 73bde7faf049..bce10a0f614c 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -404,7 +404,7 @@ static notmuch_status_t
 _notmuch_database_link_message (notmuch_database_t *notmuch,
notmuch_message_t *message,
notmuch_message_file_t *message_file,
-   notmuch_bool_t is_ghost)
+   bool is_ghost)
 {
 void *local = talloc_new (NULL);
 notmuch_status_t status;
@@ -467,7 +467,7 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
 notmuch_message_t *message = NULL;
 notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, ret2;
 notmuch_private_status_t private_status;
-notmuch_bool_t is_ghost = FALSE, is_new = FALSE;
+bool is_ghost = false, is_new = false;
 
 const char *date;
 const char *from, *to, *subject;
@@ -510,12 +510,12 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
/* We cannot call notmuch_message_get_flag for a new message */
switch (private_status) {
case NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND:
-   is_ghost = FALSE;
-   is_new = TRUE;
+   is_ghost = false;
+   is_new = true;
break;
case NOTMUCH_PRIVATE_STATUS_SUCCESS:
is_ghost = notmuch_message_get_flag (message, 
NOTMUCH_MESSAGE_FLAG_GHOST);
-   is_new = FALSE;
+   is_new = false;
break;
default:
ret = COERCE_STATUS (private_status,
@@ -551,7 +551,7 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
 } catch (const Xapian::Error ) {
_notmuch_database_log (notmuch, "A Xapian exception occurred adding 
message: %s.\n",
 error.get_msg().c_str());
-   notmuch->exception_reported = TRUE;
+   notmuch->exception_reported = true;
ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
goto DONE;
 }
diff --git a/lib/built-with.c b/lib/built-with.c
index 2f1f0b5c1bf3..27384bd044bc 100644
--- a/lib/built-with.c
+++ b/lib/built-with.c
@@ -31,6 +31,6 @@ notmuch_built_with (const char *name)
 } else if (STRNCMP_LITERAL (name, "retry_lock") == 0) {
return HAVE_XAPIAN_DB_RETRY_LOCK;
 } else {
-   return FALSE;
+   return false;
 }
 }
diff --git a/lib/config.cc b/lib/config.cc
index 0703b9bb8fde..da71c16e5adc 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -56,7 +56,7 @@ notmuch_database_set_config (notmuch_database_t *notmuch,
db->set_metadata (CONFIG_PREFIX + key, value);
 } catch (const Xapian::Error ) {
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
-   notmuch->exception_reported = TRUE;
+   notmuch->exception_reported = true;
_notmuch_database_log (notmuch, "Error: A Xapian exception occurred 
setting metadata: %s\n",
   error.get_msg().c_str());
 }
@@ -74,7 +74,7 @@ _metadata_value (notmuch_database_t *notmuch,
value = notmuch->xapian_db->get_metadata (CONFIG_PREFIX + key);
 } catch (const Xapian::Error ) {
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
-   notmuch->exception_reported = TRUE;
+   notmuch->exception_reported = true;
_notmuch_database_log (notmuch, "Error: A Xapian exception occurred 
getting metadata: %s\n",
   error.get_msg().c_str());
 }
@@ -128,7 +128,7 @@ notmuch_database_get_config_list (notmuch_database_t 
*notmuch,
 } catch (const Xapian::Error ) {
_notmuch_database_log (notmuch, "A Xapian exception occurred getting 
metadata iterator: %s.\n",
   error.get_msg().c_str());
-   notmuch->exception_reported = TRUE;
+   notmuch->exception_reported = true;
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 }
 
@@ -145,9 +145,9 @@ notmuch_bool_t
 notmuch_config_list_valid (notmuch_config_list_t *metadata)
 {
 if (metadata->iterator == 

[PATCH 1/2] cli: convert notmuch_bool_t to stdbool

2017-10-07 Thread Jani Nikula
C99 stdbool turned 18 this year. There really is no reason to use our
own, except in the library interface for backward
compatibility. Convert the cli and test binaries to stdbool.
---
 command-line-arguments.c | 58 +++
 command-line-arguments.h | 10 ---
 crypto.c |  6 ++---
 debugger.c   |  8 +++---
 gmime-filter-reply.c | 32 +++---
 mime-node.c  | 10 +++
 notmuch-client.h | 35 
 notmuch-compact.c|  2 +-
 notmuch-config.c | 28 +--
 notmuch-count.c  | 18 ++---
 notmuch-dump.c   | 10 +++
 notmuch-insert.c | 70 
 notmuch-new.c| 30 ++---
 notmuch-reply.c  | 30 ++---
 notmuch-restore.c|  4 +--
 notmuch-search.c | 24 -
 notmuch-setup.c  |  6 ++---
 notmuch-show.c   | 52 +--
 notmuch-tag.c|  6 ++---
 notmuch.c| 12 -
 sprinter-json.c  | 18 ++---
 sprinter-sexp.c  | 16 +--
 sprinter-text.c  |  6 ++---
 sprinter.h   |  6 ++---
 tag-util.c   | 24 -
 tag-util.h   |  8 +++---
 test/arg-test.c  |  6 ++---
 test/hex-xcode.c |  8 +++---
 test/random-corpus.c |  2 +-
 29 files changed, 275 insertions(+), 270 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 3fa8d9044966..1ff5aae578c6 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -6,11 +6,11 @@
 
 /*
   Search the array of keywords for a given argument, assigning the
-  output variable to the corresponding value.  Return FALSE if nothing
+  output variable to the corresponding value.  Return false if nothing
   matches.
 */
 
-static notmuch_bool_t
+static bool
 _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
 
 const notmuch_keyword_t *keywords;
@@ -29,64 +29,64 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
else
*arg_desc->opt_keyword = keywords->value;
 
-   return TRUE;
+   return true;
 }
 if (next != '\0')
fprintf (stderr, "Unknown keyword argument \"%s\" for option 
\"%s\".\n", arg_str, arg_desc->name);
 else
fprintf (stderr, "Option \"%s\" needs a keyword argument.\n", 
arg_desc->name);
-return FALSE;
+return false;
 }
 
-static notmuch_bool_t
+static bool
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
-notmuch_bool_t value;
+bool value;
 
 if (next == '\0' || strcmp (arg_str, "true") == 0) {
-   value = TRUE;
+   value = true;
 } else if (strcmp (arg_str, "false") == 0) {
-   value = FALSE;
+   value = false;
 } else {
fprintf (stderr, "Unknown argument \"%s\" for (boolean) option 
\"%s\".\n", arg_str, arg_desc->name);
-   return FALSE;
+   return false;
 }
 
 *arg_desc->opt_bool = value;
 
-return TRUE;
+return true;
 }
 
-static notmuch_bool_t
+static bool
 _process_int_arg (const notmuch_opt_desc_t *arg_desc, char next, const char 
*arg_str) {
 
 char *endptr;
 if (next == '\0' || arg_str[0] == '\0') {
fprintf (stderr, "Option \"%s\" needs an integer argument.\n", 
arg_desc->name);
-   return FALSE;
+   return false;
 }
 
 *arg_desc->opt_int = strtol (arg_str, , 10);
 if (*endptr == '\0')
-   return TRUE;
+   return true;
 
 fprintf (stderr, "Unable to parse argument \"%s\" for option \"%s\" as an 
integer.\n",
 arg_str, arg_desc->name);
-return FALSE;
+return false;
 }
 
-static notmuch_bool_t
+static bool
 _process_string_arg (const notmuch_opt_desc_t *arg_desc, char next, const char 
*arg_str) {
 
 if (next == '\0') {
fprintf (stderr, "Option \"%s\" needs a string argument.\n", 
arg_desc->name);
-   return FALSE;
+   return false;
 }
 if (arg_str[0] == '\0') {
fprintf (stderr, "String argument for option \"%s\" must be 
non-empty.\n", arg_desc->name);
-   return FALSE;
+   return false;
 }
 *arg_desc->opt_string = arg_str;
-return TRUE;
+return true;
 }
 
 /* Return number of non-NULL opt_* fields in opt_desc. */
@@ -102,8 +102,8 @@ static int _opt_set_count (const notmuch_opt_desc_t 
*opt_desc)
!!opt_desc->opt_position;
 }
 
-/* Return TRUE if opt_desc is valid. */
-static notmuch_bool_t _opt_valid (const notmuch_opt_desc_t *opt_desc)
+/* Return true if opt_desc is valid. */
+static bool _opt_valid (const notmuch_opt_desc_t *opt_desc)
 {
 int n = _opt_set_count (opt_desc);
 
@@ -115,11 +115,11 @@ static notmuch_bool_t _opt_valid (const 
notmuch_opt_desc_t *opt_desc)
 }
 
 /*
-  

[PATCH v2 0/2] cli: support regex in new.ignore

2017-10-06 Thread Jani Nikula
The rest of the patches from id:cover.1504280923.git.j...@nikula.org
with review addressed. Be more strict about parsing new.ignore, and give
sensible error messages on failures. Also improve the text in man page a
bit.

BR,
Jani.

Jani Nikula (2):
  cli/new: support // in new.ignore
  test: test regexp based new.ignore

 doc/man1/notmuch-config.rst |  21 +--
 notmuch-new.c   | 134 
 test/T050-new.sh|  22 
 3 files changed, 162 insertions(+), 15 deletions(-)

-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 1/2] cli/new: support // in new.ignore

2017-10-06 Thread Jani Nikula
Add support for using // style regular expressions in
new.ignore, mixed with the old style verbatim file and directory
basenames. The regex is matched against the relative path from the
database path.
---
 doc/man1/notmuch-config.rst |  21 +--
 notmuch-new.c   | 134 
 2 files changed, 140 insertions(+), 15 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 6a51e64f1517..0af86f9beee4 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -75,11 +75,22 @@ The available configuration items are described below.
 Default: ``unread;inbox``.
 
 **new.ignore**
-A list of file and directory names, without path, that will not
-be searched for messages by **notmuch new**. All the files and
-directories matching any of the names specified here will be
-ignored, regardless of the location in the mail store directory
-hierarchy.
+A list to specify files and directories that will not be
+searched for messages by **notmuch new**. Each entry in the
+list is either:
+
+  A file or a directory name, without path, that will be
+  ignored, regardless of the location in the mail store
+  directory hierarchy.
+
+Or:
+
+  A regular expression delimited with // that will be matched
+  against the path of the file or directory relative to the
+  database path. Matching files and directories will be
+  ignored. The beginning and end of string must be explictly
+  anchored. For example, /.*/foo$/ would match "bar/foo" and
+  "bar/baz/foo", but not "foo" or "bar/foobar".
 
 Default: empty list.
 
diff --git a/notmuch-new.c b/notmuch-new.c
index 50597b75c07e..7e12b73a7883 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -42,13 +42,17 @@ enum verbosity {
 };
 
 typedef struct {
+const char *db_path;
+
 int output_is_a_tty;
 enum verbosity verbosity;
 notmuch_bool_t debug;
 const char **new_tags;
 size_t new_tags_length;
-const char **new_ignore;
-size_t new_ignore_length;
+const char **ignore_verbatim;
+size_t ignore_verbatim_length;
+regex_t *ignore_regex;
+size_t ignore_regex_length;
 
 int total_files;
 int processed_files;
@@ -240,18 +244,125 @@ _special_directory (const char *entry)
 return strcmp (entry, ".") == 0 || strcmp (entry, "..") == 0;
 }
 
+static notmuch_bool_t
+_setup_ignore (notmuch_config_t *config, add_files_state_t *state)
+{
+const char **ignore_list, **ignore;
+int nregex = 0, nverbatim = 0;
+const char **verbatim = NULL;
+regex_t *regex = NULL;
+
+ignore_list = notmuch_config_get_new_ignore (config, NULL);
+if (! ignore_list)
+   return TRUE;
+
+for (ignore = ignore_list; *ignore; ignore++) {
+   const char *s = *ignore;
+   size_t len = strlen (s);
+
+   if (len == 0) {
+   fprintf (stderr, "Error: Empty string in new.ignore list\n");
+   return FALSE;
+   }
+
+   if (s[0] == '/') {
+   regex_t *preg;
+   char *r;
+   int rerr;
+
+   if (len < 3 || s[len - 1] != '/') {
+   fprintf (stderr, "Error: Malformed pattern '%s' in 
new.ignore\n",
+s);
+   return FALSE;
+   }
+
+   r = talloc_strndup (config, s + 1, len - 2);
+   regex = talloc_realloc (config, regex, regex_t, nregex + 1);
+   preg = [nregex];
+
+   rerr = regcomp (preg, r, REG_EXTENDED | REG_NOSUB);
+   if (rerr) {
+   size_t error_size = regerror (rerr, preg, NULL, 0);
+   char *error = talloc_size (r, error_size);
+
+   regerror (rerr, preg, error, error_size);
+
+   fprintf (stderr, "Error: Invalid regex '%s' in new.ignore: 
%s\n",
+r, error);
+   return FALSE;
+   }
+   nregex++;
+
+   talloc_free (r);
+   } else {
+   verbatim = talloc_realloc (config, verbatim, const char *,
+  nverbatim + 1);
+   verbatim[nverbatim++] = s;
+   }
+}
+
+state->ignore_regex = regex;
+state->ignore_regex_length = nregex;
+state->ignore_verbatim = verbatim;
+state->ignore_verbatim_length = nverbatim;
+
+return TRUE;
+}
+
+static char *
+_get_relative_path (const char *db_path, const char *dirpath, const char 
*entry)
+{
+size_t db_path_len = strlen (db_path);
+
+/* paranoia? */
+if (strncmp (dirpath, db_path, db_path_len) != 0) {
+   fprintf (stderr, "Warning: '%s' is not a subdirectory of '%s'\n",
+dirpath, db_path);
+   return NULL;
+}
+
+dirpath += db_path_len;
+while (*dirpath == '/')
+   dirpath++;
+
+if (*dirpath)
+   return talloc_asprintf (NULL, "%s/%s", dirpath, entry);
+

[PATCH v2 2/2] test: test regexp based new.ignore

2017-10-06 Thread Jani Nikula
Just some basics.
---
 test/T050-new.sh | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 272ed417aa2e..ee9ce08c8e86 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -259,6 +259,28 @@ ln -s i_do_not_exist "${MAIL_DIR}"/broken_link
 output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" "No new mail."
 
+test_begin_subtest "Ignore files and directories specified in new.ignore 
(regexp)"
+notmuch config set new.ignore ".git" "/^bro.*ink\$/" "/ignored.*file/"
+output=$(NOTMUCH_NEW --debug 2>&1 | sort)
+test_expect_equal "$output" \
+"(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/broken_link
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/two/ignored_file
+(D) add_files, pass 1: explicitly ignoring ${MAIL_DIR}/one/two/three/.git
+(D) add_files, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/.ignored_hidden_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/broken_link
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/two/ignored_file
+(D) add_files, pass 2: explicitly ignoring ${MAIL_DIR}/one/two/three/.git
+(D) add_files, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
+No new mail."
+
 test_begin_subtest "Quiet: No new mail."
 output=$(NOTMUCH_NEW --quiet)
 test_expect_equal "$output" ""
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: notmuch-emacs: Fcc to top-level directory given by database.path

2017-10-02 Thread Jani Nikula
On Sat, 30 Sep 2017, Jani Nikula <j...@nikula.org> wrote:
> On Sat, 23 Sep 2017, David Bremner <da...@tethera.net> wrote:
>> Do you happen to know if it calls with an empty string as the folder
>> name? It would be consistent with searching for that to insert at the
>> top level.
>
> notmuch insert --folder= or --folder="" does not work:
>
> String argument for option "folder" must be non-empty.
> Unrecognized option: --folder=
>
> It seems that our argument parser doesn't accept empty strings at all,
> which is a bit of a limitation. It would be logical for the empty string
> to work here, and AFAICT the notmuch insert code would handle this if
> the argument was let through the parsing.

Turns out this is trivial to address on top of the arg parsing series:
id:20171002162552.4827-1-j...@nikula.org.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] cli: allow empty strings for notmuch insert --folder argument

2017-10-02 Thread Jani Nikula
Now that it's easy to add argument specific modifiers in opt
descriptions, add a new .allow_empty field to allow empty strings for
individual string arguments while retaining strict checks
elsewhere. Use this for notmuch insert --folder, where the empty
string means top level folder.

---

This patch addresses id:87y3owr22c@nikula.org

Depends on most of the series, but specifically not on the more
controversial patches 13-15.
---
 command-line-arguments.c| 2 +-
 command-line-arguments.h| 3 +++
 doc/man1/notmuch-insert.rst | 3 ++-
 notmuch-insert.c| 2 +-
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 3fa8d9044966..b84bfe8168b5 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -81,7 +81,7 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, char 
next, const char *
fprintf (stderr, "Option \"%s\" needs a string argument.\n", 
arg_desc->name);
return FALSE;
 }
-if (arg_str[0] == '\0') {
+if (arg_str[0] == '\0' && ! arg_desc->allow_empty) {
fprintf (stderr, "String argument for option \"%s\" must be 
non-empty.\n", arg_desc->name);
return FALSE;
 }
diff --git a/command-line-arguments.h b/command-line-arguments.h
index dfc808bdab78..04b04b939cba 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -30,6 +30,9 @@ typedef struct notmuch_opt_desc {
 /* Optional, if non-NULL, set to TRUE if the option is present. */
 notmuch_bool_t *present;
 
+/* Optional, allow empty strings for opt_string. */
+notmuch_bool_t allow_empty;
+
 /* Must be set for opt_keyword and opt_flags. */
 const struct notmuch_keyword *keywords;
 } notmuch_opt_desc_t;
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index f79600d6571f..2f2466a6588b 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -34,7 +34,8 @@ Supported options for **insert** include
 ``--folder=<``\ folder\ **>**
 Deliver the message to the specified folder, relative to the
 top-level directory given by the value of **database.path**. The
-default is to deliver to the top-level directory.
+default is the empty string, which means delivering to the
+top-level directory.
 
 ``--create-folder``
 Try to create the folder named by the ``--folder`` option, if it
diff --git a/notmuch-insert.c b/notmuch-insert.c
index bbbc29ea103d..2758723ab2fb 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -463,7 +463,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
 unsigned int i;
 
 notmuch_opt_desc_t options[] = {
-   { .opt_string = , .name = "folder" },
+   { .opt_string = , .name = "folder", .allow_empty = TRUE },
{ .opt_bool = _folder, .name = "create-folder" },
{ .opt_bool = , .name = "keep" },
{ .opt_bool =  _hooks, .name = "no-hooks" },
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: notmuch-emacs: Fcc to top-level directory given by database.path

2017-10-02 Thread Jani Nikula
On Mon, 02 Oct 2017, Arun Isaac  wrote:
> Mark Walters  writes:
>
>> Incidentally, I think "/" is an alternative for the fcc line which
>> already works, which is "\"/\" in notmuch-fcc-dirs.
>
> Perhaps, notmuch should be made to tolerate a "/" at the beginning of the
> Fcc folder argument. That is, notmuch should not complain about absolute
> paths, and it should interpret these as relative to the root. To
> maintain backward compatibility, we could add a "/" at the beginning if
> it is not already there.
>
> So,
>
> "/" => database.path
> "/sent" => database.path/sent
> "sent" => database.path/sent
>
> etc.
>
> Is this a better idea?

At the cli notmuch insert level, I'd actually rather do the opposite and
be even stricter about folder being relative. I just had to look at the
code for other reasons, and it seems to accept all sorts of weird combos
with "/" and "." that I think should be rejected. Or at the very least
sanitized. Otherwise we end up with filenames with "//" or "/./" in
them, probably confusing notmuch later on.

I'd argue notmuch insert --folder="" should Fcc to the mail store root,
but alas that doesn't work at the cli level. It doesn't appear to work
at the emacs level either, but perhaps notmuch-emacs could translate ""
to dropping the --folder argument? Could even add that as a choice
option in notmuch-fcc-dirs customization. Mark?

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 13/15] cli: add support for --no- prefixed boolean and keyword flag arguments

2017-10-02 Thread Jani Nikula
On Sun, 01 Oct 2017, William Casarin <j...@jb55.com> wrote:
> Jani Nikula <j...@nikula.org> writes:
>
>> @@ -171,11 +186,22 @@ parse_option (int argc, char **argv, const 
>> notmuch_opt_desc_t *options, int opt_
>>  if (! try->name)
>>  continue;
>>  
>> -if (strncmp (arg, try->name, strlen (try->name)) != 0)
>> +char next;
>> +const char *value;
>> +notmuch_bool_t negate = FALSE;
>> +
>> +if (strncmp (arg, try->name, strlen (try->name)) == 0) {
>> +next = arg[strlen (try->name)];
>> +value = arg + strlen (try->name) + 1;
>> +} else if (negative_arg && (try->opt_bool || try->opt_flags) &&
>> +   strncmp (negative_arg, try->name, strlen (try->name)) == 0) {
>> +next = negative_arg[strlen (try->name)];
>> +value = negative_arg + strlen (try->name) + 1;
>> +/* The argument part of --no-argument matches, negate the result. */
>> +negate = TRUE;
>> +} else {
>>  continue;
>> -
>> -char next = arg[strlen (try->name)];
>> -const char *value = arg + strlen(try->name) + 1;
>> +}
>
> nit: I see strlen (try->name) computed 6 times here, any reason not to pull
> this out into a variable?

I pretty much thought the change was so controversial that I wouldn't
bother with that kind of fixes until we'd agreed we want this. Other
than that, agreed.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Jani Nikula
On Sun, 01 Oct 2017, Tomi Ollila <tomi.oll...@iki.fi> wrote:
> On Sun, Oct 01 2017, Jani Nikula wrote:
>
>> On Sun, 01 Oct 2017, Tomi Ollila <tomi.oll...@iki.fi> wrote:
>>>
>>> Good stuff. It just doesn't longer compile on older compilers (so some
>>> note on commit message should be added):
>>
>> Does this on top fix it? I used the unnamed struct just for clarity, and
>> it doesn't serve a functional purpose.
>
> Yes it does!
>
> (output is noisy, but so is it with gcc 4.8.5 which I just tested to work)

What's so noisy about it...? :o

v2 as part of a bigger series at id:cover.1506890421.git.j...@nikula.org.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 10/15] cli: refactor boolean argument processing

2017-10-01 Thread Jani Nikula
Clean up the control flow to prepare for future changes. No functional
changes.
---
 command-line-arguments.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 39940d5fb9fd..ee8be18942d0 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -39,21 +39,20 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
 
 static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
-
-if (next == '\0') {
-   *arg_desc->opt_bool = TRUE;
-   return TRUE;
-}
-if (strcmp (arg_str, "false") == 0) {
-   *arg_desc->opt_bool = FALSE;
-   return TRUE;
-}
-if (strcmp (arg_str, "true") == 0) {
-   *arg_desc->opt_bool = TRUE;
-   return TRUE;
+notmuch_bool_t value;
+
+if (next == '\0' || strcmp (arg_str, "true") == 0) {
+   value = TRUE;
+} else if (strcmp (arg_str, "false") == 0) {
+   value = FALSE;
+} else {
+   fprintf (stderr, "Unknown argument \"%s\" for (boolean) option 
\"%s\".\n", arg_str, arg_desc->name);
+   return FALSE;
 }
-fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", 
arg_str, arg_desc->name);
-return FALSE;
+
+*arg_desc->opt_bool = value;
+
+return TRUE;
 }
 
 static notmuch_bool_t
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 15/15] test: expand argument parsing sanity checks

2017-10-01 Thread Jani Nikula
Test the various boolean formats and --no- prefixed boolean and
keyword flag arguments.
---
 test/T410-argument-parsing.sh | 28 
 1 file changed, 28 insertions(+)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 4a2b25c6486d..243d0241b9b6 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -37,4 +37,32 @@ positional arg 1 false
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "--boolean=true"
+$TEST_DIRECTORY/arg-test --boolean=true > OUTPUT
+cat < EXPECTED
+boolean 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--boolean=false"
+$TEST_DIRECTORY/arg-test --boolean=false > OUTPUT
+cat < EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--no-boolean"
+$TEST_DIRECTORY/arg-test --no-boolean > OUTPUT
+cat < EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--no-flag"
+$TEST_DIRECTORY/arg-test --flag=one --flag=three --no-flag=three > OUTPUT
+cat < EXPECTED
+flags 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 04/15] test: add opt_inherit to arg-test

2017-10-01 Thread Jani Nikula
Just split the arguments to two opt desc arrays.
---
 test/arg-test.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/test/arg-test.c b/test/arg-test.c
index 9d13618bd17c..a379f23e308a 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -14,18 +14,23 @@ int main(int argc, char **argv){
 const char *string_val=NULL;
 notmuch_bool_t bool_val = FALSE;
 
-notmuch_opt_desc_t options[] = {
-   { .opt_bool = _val, .name = "boolean" },
-   { .opt_keyword = _val, .name = "keyword", .keywords =
- (notmuch_keyword_t []){ { "one", 1 },
- { "two", 2 },
- { 0, 0 } } },
+notmuch_opt_desc_t parent_options[] = {
{ .opt_flags = _val, .name = "flag", .keywords =
  (notmuch_keyword_t []){ { "one",   1 << 0},
  { "two",   1 << 1 },
  { "three", 1 << 2 },
  { 0, 0 } } },
{ .opt_int = _val, .name = "int" },
+   { }
+};
+
+notmuch_opt_desc_t options[] = {
+   { .opt_bool = _val, .name = "boolean" },
+   { .opt_keyword = _val, .name = "keyword", .keywords =
+ (notmuch_keyword_t []){ { "one", 1 },
+ { "two", 2 },
+ { 0, 0 } } },
+   { .opt_inherit = parent_options },
{ .opt_string = _val, .name = "string" },
{ .opt_position = _arg1 },
{ .opt_position = _arg2 },
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 07/15] cli: use the arg parser .present feature to handle show --entire-thread

2017-10-01 Thread Jani Nikula
The --entire-thread default depends on other arguments, so we'll have
to figure out if it was explicitly set by the user or not. The arg
parser .present feature helps us clean up the code here.
---
 notmuch-show.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 367536ff9532..d0e86f412e80 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1086,10 +1086,7 @@ notmuch_show_command (notmuch_config_t *config, int 
argc, char *argv[])
 };
 int format = NOTMUCH_FORMAT_NOT_SPECIFIED;
 int exclude = TRUE;
-
-/* This value corresponds to neither true nor false being passed
- * on the command line */
-int entire_thread = -1;
+notmuch_bool_t entire_thread_set = FALSE;
 notmuch_bool_t single_message;
 
 notmuch_opt_desc_t options[] = {
@@ -1102,7 +1099,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, 
char *argv[])
  { 0, 0 } } },
{ .opt_int = _format_version, .name = "format-version" },
{ .opt_bool = , .name = "exclude" },
-   { .opt_bool = _thread, .name = "entire-thread" },
+   { .opt_bool = _thread, .name = "entire-thread",
+ .present = _thread_set },
{ .opt_int = , .name = "part" },
{ .opt_bool = , .name = "decrypt" },
{ .opt_bool = , .name = "verify" },
@@ -1147,14 +1145,9 @@ notmuch_show_command (notmuch_config_t *config, int 
argc, char *argv[])
 
 /* Default is entire-thread = FALSE except for format=json and
  * format=sexp. */
-if (entire_thread != FALSE && entire_thread != TRUE) {
-   if (format == NOTMUCH_FORMAT_JSON || format == NOTMUCH_FORMAT_SEXP)
-   params.entire_thread = TRUE;
-   else
-   params.entire_thread = FALSE;
-} else {
-   params.entire_thread = entire_thread;
-}
+if (! entire_thread_set &&
+   (format == NOTMUCH_FORMAT_JSON || format == NOTMUCH_FORMAT_SEXP))
+   params.entire_thread = TRUE;
 
 if (!params.output_body) {
if (params.part > 0) {
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 09/15] cli: use notmuch_bool_t for boolean argument in show

2017-10-01 Thread Jani Nikula
Pedantically correct, although they're the same underlying type.
---
 notmuch-show.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index d0e86f412e80..3d11a40c6a59 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1085,7 +1085,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, 
char *argv[])
.output_body = TRUE,
 };
 int format = NOTMUCH_FORMAT_NOT_SPECIFIED;
-int exclude = TRUE;
+notmuch_bool_t exclude = TRUE;
 notmuch_bool_t entire_thread_set = FALSE;
 notmuch_bool_t single_message;
 
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 11/15] cli: change while to for in keyword argument processing

2017-10-01 Thread Jani Nikula
Using a for loop makes it easier to use continue, in preparation for
future changes. No functional changes.
---
 command-line-arguments.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index ee8be18942d0..c591dcbec7cc 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -13,14 +13,14 @@
 static notmuch_bool_t
 _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
 
-const notmuch_keyword_t *keywords = arg_desc->keywords;
+const notmuch_keyword_t *keywords;
 
 if (next == '\0') {
/* No keyword given */
arg_str = "";
 }
 
-while (keywords->name) {
+for (keywords = arg_desc->keywords; keywords->name; keywords++) {
if (strcmp (arg_str, keywords->name) == 0) {
if (arg_desc->opt_flags)
*arg_desc->opt_flags |= keywords->value;
@@ -28,7 +28,6 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
*arg_desc->opt_keyword = keywords->value;
return TRUE;
}
-   keywords++;
 }
 if (next != '\0')
fprintf (stderr, "Unknown keyword argument \"%s\" for option 
\"%s\".\n", arg_str, arg_desc->name);
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 13/15] cli: add support for --no- prefixed boolean and keyword flag arguments

2017-10-01 Thread Jani Nikula
Add transparent support for negating boolean and keyword flag
arguments using --no-argument style on the command line. That is, if
the option description contains a boolean or a keyword flag argument
named "argument", --no-argument will match and negate it.

For boolean arguments this obviously means the logical NOT. For
keyword flag arguments this means bitwise AND of the bitwise NOT,
i.e. masking out the specified bits instead of OR'ing them in.

For example, you can use --no-exclude instead of --exclude=false in
notmuch show. If we had keyword flag arguments with some flags
defaulting to on, say --include=tags in notmuch dump/restore, this
would allow --no-include=tags to switch that off while not affecting
other flags.

As a curiosity, you should be able to warp your brain using
--no-exclude=true meaning false and --no-exclude=false meaning true if
you wish.

Specifying both "argument" and "no-argument" style arguments in the
same option description should be avoided. In this case, --no-argument
would match whichever is specified first, and --argument would only
match "argument".
---
 command-line-arguments.c | 48 +---
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 3fa8d9044966..058408a789fb 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -11,8 +11,9 @@
 */
 
 static notmuch_bool_t
-_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
-
+_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next,
+ const char *arg_str, notmuch_bool_t negate)
+{
 const notmuch_keyword_t *keywords;
 
 if (next == '\0') {
@@ -24,7 +25,9 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
if (strcmp (arg_str, keywords->name) != 0)
continue;
 
-   if (arg_desc->opt_flags)
+   if (arg_desc->opt_flags && negate)
+   *arg_desc->opt_flags &= ~keywords->value;
+   else if (arg_desc->opt_flags)
*arg_desc->opt_flags |= keywords->value;
else
*arg_desc->opt_keyword = keywords->value;
@@ -39,7 +42,9 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
 }
 
 static notmuch_bool_t
-_process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
+_process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next,
+ const char *arg_str, notmuch_bool_t negate)
+{
 notmuch_bool_t value;
 
 if (next == '\0' || strcmp (arg_str, "true") == 0) {
@@ -51,7 +56,7 @@ _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
return FALSE;
 }
 
-*arg_desc->opt_bool = value;
+*arg_desc->opt_bool = negate ? !value : value;
 
 return TRUE;
 }
@@ -139,6 +144,8 @@ parse_position_arg (const char *arg_str, int pos_arg_index,
 return FALSE;
 }
 
+#define NEGATIVE_PREFIX "no-"
+
 /*
  * Search for a non-positional (i.e. starting with --) argument matching arg,
  * parse a possible value, and assign to *output_var
@@ -155,6 +162,14 @@ parse_option (int argc, char **argv, const 
notmuch_opt_desc_t *options, int opt_
 assert(options);
 
 const char *arg = _arg + 2; /* _arg starts with -- */
+const char *negative_arg = NULL;
+
+/* See if this is a --no-argument */
+if (strlen (arg) > strlen (NEGATIVE_PREFIX) &&
+   strncmp (arg, NEGATIVE_PREFIX, strlen (NEGATIVE_PREFIX)) == 0) {
+   negative_arg = arg + strlen (NEGATIVE_PREFIX);
+}
+
 const notmuch_opt_desc_t *try;
 
 const char *next_arg = NULL;
@@ -171,11 +186,22 @@ parse_option (int argc, char **argv, const 
notmuch_opt_desc_t *options, int opt_
if (! try->name)
continue;
 
-   if (strncmp (arg, try->name, strlen (try->name)) != 0)
+   char next;
+   const char *value;
+   notmuch_bool_t negate = FALSE;
+
+   if (strncmp (arg, try->name, strlen (try->name)) == 0) {
+   next = arg[strlen (try->name)];
+   value = arg + strlen (try->name) + 1;
+   } else if (negative_arg && (try->opt_bool || try->opt_flags) &&
+  strncmp (negative_arg, try->name, strlen (try->name)) == 0) {
+   next = negative_arg[strlen (try->name)];
+   value = negative_arg + strlen (try->name) + 1;
+   /* The argument part of --no-argument matches, negate the result. */
+   negate = TRUE;
+   } else {
continue;
-
-   char next = arg[strlen (try->name)];
-   const char *value = arg + strlen(try->name) + 1;
+   }
 
/*
 * If we have not reached the end of the argument (i.e. the
@@ -194,9 +220,9 @@ parse_option (int argc, char **argv, const 
notmuch_opt_desc_t *options, int opt_
 
notmuch_bool_t opt_status = FALSE;
if (try->opt_keyword || try->opt_flags)
-   opt_status = 

[PATCH v2 14/15] cli: use the negating boolean support for new and insert --no-hooks

2017-10-01 Thread Jani Nikula
This lets us use the positive hooks variable in code, increasing
clarity.
---
 notmuch-insert.c | 6 +++---
 notmuch-new.c| 8 
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index bbbc29ea103d..7048e8ae2d7f 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -455,7 +455,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
 const char *folder = "";
 notmuch_bool_t create_folder = FALSE;
 notmuch_bool_t keep = FALSE;
-notmuch_bool_t no_hooks = FALSE;
+notmuch_bool_t hooks = TRUE;
 notmuch_bool_t synchronize_flags;
 char *maildir;
 char *newpath;
@@ -466,7 +466,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
{ .opt_string = , .name = "folder" },
{ .opt_bool = _folder, .name = "create-folder" },
{ .opt_bool = , .name = "keep" },
-   { .opt_bool =  _hooks, .name = "no-hooks" },
+   { .opt_bool = , .name = "hooks" },
{ .opt_inherit = notmuch_shared_options },
{ }
 };
@@ -573,7 +573,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
}
 }
 
-if (! no_hooks && status == NOTMUCH_STATUS_SUCCESS) {
+if (hooks && status == NOTMUCH_STATUS_SUCCESS) {
/* Ignore hook failures. */
notmuch_run_hook (db_path, "post-insert");
 }
diff --git a/notmuch-new.c b/notmuch-new.c
index 342e2189d5d3..084cc786ea32 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -948,7 +948,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
 int opt_index;
 unsigned int i;
 notmuch_bool_t timer_is_active = FALSE;
-notmuch_bool_t no_hooks = FALSE;
+notmuch_bool_t hooks = TRUE;
 notmuch_bool_t quiet = FALSE, verbose = FALSE;
 notmuch_status_t status;
 
@@ -956,7 +956,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
{ .opt_bool = , .name = "quiet" },
{ .opt_bool = , .name = "verbose" },
{ .opt_bool = _files_state.debug, .name = "debug" },
-   { .opt_bool = _hooks, .name = "no-hooks" },
+   { .opt_bool = , .name = "hooks" },
{ .opt_inherit = notmuch_shared_options },
{ }
 };
@@ -989,7 +989,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
}
 }
 
-if (!no_hooks) {
+if (hooks) {
ret = notmuch_run_hook (db_path, "pre-new");
if (ret)
return EXIT_FAILURE;
@@ -1154,7 +1154,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
 
 notmuch_database_destroy (notmuch);
 
-if (!no_hooks && !ret && !interrupted)
+if (hooks && !ret && !interrupted)
ret = notmuch_run_hook (db_path, "post-new");
 
 if (ret || interrupted)
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/9] argument parsing fixes and improvements

2017-10-01 Thread Jani Nikula
On Sun, 01 Oct 2017, David Bremner <da...@tethera.net> wrote:
> Jani Nikula <j...@nikula.org> writes:
>> id:20170930213239.15392-1-j...@nikula.org would make it easier to add,
>> say, a notmuch_bool_t *present field to notmuch_opt_desc_t that we could
>> set whenever we see the option and we want to know the difference.
>
> OK, I've queued that patch. Care to add `present` so we can unblock this
> discussion?

Done. I got a bit carried away, but the important stuff is in the
beginning: id:cover.1506890421.git.j...@nikula.org


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 12/15] cli: reduce indent in keyword argument processing

2017-10-01 Thread Jani Nikula
Reducing indent makes future changes easier. No functional changes.
---
 command-line-arguments.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index c591dcbec7cc..3fa8d9044966 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -21,13 +21,15 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
 }
 
 for (keywords = arg_desc->keywords; keywords->name; keywords++) {
-   if (strcmp (arg_str, keywords->name) == 0) {
-   if (arg_desc->opt_flags)
-   *arg_desc->opt_flags |= keywords->value;
-   else
-   *arg_desc->opt_keyword = keywords->value;
-   return TRUE;
-   }
+   if (strcmp (arg_str, keywords->name) != 0)
+   continue;
+
+   if (arg_desc->opt_flags)
+   *arg_desc->opt_flags |= keywords->value;
+   else
+   *arg_desc->opt_keyword = keywords->value;
+
+   return TRUE;
 }
 if (next != '\0')
fprintf (stderr, "Unknown keyword argument \"%s\" for option 
\"%s\".\n", arg_str, arg_desc->name);
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 06/15] test: expand argument parsing tests

2017-10-01 Thread Jani Nikula
Test and use the new .present field, only output the parameters
given. Test space between parameter name and value.
---
 test/T410-argument-parsing.sh | 22 ++
 test/arg-test.c   | 33 ++---
 2 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 4505c58301ea..4a2b25c6486d 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -15,4 +15,26 @@ positional arg 2 pos2
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "sanity check zero values"
+$TEST_DIRECTORY/arg-test --keyword=zero --boolean=false --int=0 > OUTPUT
+cat < EXPECTED
+boolean 0
+keyword 0
+int 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "space instead of = between parameter name and value"
+# Note: spaces aren't allowed for booleans. false turns into a positional arg!
+$TEST_DIRECTORY/arg-test --keyword one --boolean false --string foo --int 7 
--flag one --flag three > OUTPUT
+cat < EXPECTED
+boolean 1
+keyword 1
+flags 5
+int 7
+string foo
+positional arg 1 false
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
diff --git a/test/arg-test.c b/test/arg-test.c
index a379f23e308a..64751df4ada1 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -13,27 +13,30 @@ int main(int argc, char **argv){
 const char *pos_arg2=NULL;
 const char *string_val=NULL;
 notmuch_bool_t bool_val = FALSE;
+notmuch_bool_t fl_set = FALSE, int_set = FALSE, bool_set = FALSE,
+   kw_set = FALSE, string_set = FALSE, pos1_set = FALSE, pos2_set = FALSE;
 
 notmuch_opt_desc_t parent_options[] = {
-   { .opt_flags = _val, .name = "flag", .keywords =
+   { .opt_flags = _val, .name = "flag", .present = _set, .keywords =
  (notmuch_keyword_t []){ { "one",   1 << 0},
  { "two",   1 << 1 },
  { "three", 1 << 2 },
  { 0, 0 } } },
-   { .opt_int = _val, .name = "int" },
+   { .opt_int = _val, .name = "int", .present = _set },
{ }
 };
 
 notmuch_opt_desc_t options[] = {
-   { .opt_bool = _val, .name = "boolean" },
-   { .opt_keyword = _val, .name = "keyword", .keywords =
- (notmuch_keyword_t []){ { "one", 1 },
+   { .opt_bool = _val, .name = "boolean", .present = _set },
+   { .opt_keyword = _val, .name = "keyword", .present = _set, 
.keywords =
+ (notmuch_keyword_t []){ { "zero", 0 },
+ { "one", 1 },
  { "two", 2 },
  { 0, 0 } } },
{ .opt_inherit = parent_options },
-   { .opt_string = _val, .name = "string" },
-   { .opt_position = _arg1 },
-   { .opt_position = _arg2 },
+   { .opt_string = _val, .name = "string", .present = _set },
+   { .opt_position = _arg1, .present = _set },
+   { .opt_position = _arg2, .present = _set },
{ }
 };
 
@@ -42,25 +45,25 @@ int main(int argc, char **argv){
 if (opt_index < 0)
return 1;
 
-if (bool_val)
+if (bool_set)
printf("boolean %d\n", bool_val);
 
-if (kw_val)
+if (kw_set)
printf("keyword %d\n", kw_val);
 
-if (fl_val)
+if (fl_set)
printf("flags %d\n", fl_val);
 
-if (int_val)
+if (int_set)
printf("int %d\n", int_val);
 
-if (string_val)
+if (string_set)
printf("string %s\n", string_val);
 
-if (pos_arg1)
+if (pos1_set)
printf("positional arg 1 %s\n", pos_arg1);
 
-if (pos_arg2)
+if (pos2_set)
printf("positional arg 2 %s\n", pos_arg2);
 
 
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 08/15] hex-xcode: use notmuch_bool_t for boolean arguments

2017-10-01 Thread Jani Nikula
Pedantically correct, although they're the same underlying type.
---
 test/hex-xcode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/hex-xcode.c b/test/hex-xcode.c
index bc2df713b2a3..221ccdb90843 100644
--- a/test/hex-xcode.c
+++ b/test/hex-xcode.c
@@ -45,7 +45,7 @@ main (int argc, char **argv)
 {
 
 int dir = DECODE;
-int omit_newline = FALSE;
+notmuch_bool_t omit_newline = FALSE;
 
 notmuch_opt_desc_t options[] = {
{ .opt_keyword = , .name = "direction", .keywords =
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 05/15] cli: add .present field to opt desc to check if the arg was present

2017-10-01 Thread Jani Nikula
Add pointer to boolean .present field to opt desc, which (if non-NULL)
will be set to TRUE if the argument in question is present on the
command line. Unchanged otherwise.
---
 command-line-arguments.c | 11 ---
 command-line-arguments.h |  3 +++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index f1a5b2324337..39940d5fb9fd 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -128,6 +128,8 @@ parse_position_arg (const char *arg_str, int pos_arg_index,
if (arg_desc->opt_position) {
if (pos_arg_counter == pos_arg_index) {
*arg_desc->opt_position = arg_str;
+   if (arg_desc->present)
+   *arg_desc->present = TRUE;
return TRUE;
}
pos_arg_counter++;
@@ -202,10 +204,13 @@ parse_option (int argc, char **argv, const 
notmuch_opt_desc_t *options, int opt_
else
INTERNAL_ERROR ("unknown or unhandled option \"%s\"", try->name);
 
-   if (opt_status)
-   return opt_index+1;
-   else
+   if (! opt_status)
return -1;
+
+   if (try->present)
+   *try->present = TRUE;
+
+   return opt_index+1;
 }
 return -1;
 }
diff --git a/command-line-arguments.h b/command-line-arguments.h
index ff51abceb117..dfc808bdab78 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -27,6 +27,9 @@ typedef struct notmuch_opt_desc {
 /* Must be set except for opt_inherit and opt_position. */
 const char *name;
 
+/* Optional, if non-NULL, set to TRUE if the option is present. */
+notmuch_bool_t *present;
+
 /* Must be set for opt_keyword and opt_flags. */
 const struct notmuch_keyword *keywords;
 } notmuch_opt_desc_t;
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 03/15] test: add boolean argument to arg-test

2017-10-01 Thread Jani Nikula
Surprisingly it's not there.
---
 test/T410-argument-parsing.sh | 3 ++-
 test/arg-test.c   | 5 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index fad134e305c5..4505c58301ea 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -3,8 +3,9 @@ test_description="argument parsing"
 . ./test-lib.sh || exit 1
 
 test_begin_subtest "sanity check"
-$TEST_DIRECTORY/arg-test  pos1  --keyword=one --string=foo pos2 --int=7 
--flag=one --flag=three > OUTPUT
+$TEST_DIRECTORY/arg-test  pos1  --keyword=one --boolean --string=foo pos2 
--int=7 --flag=one --flag=three > OUTPUT
 cat < EXPECTED
+boolean 1
 keyword 1
 flags 5
 int 7
diff --git a/test/arg-test.c b/test/arg-test.c
index 10dc06834513..9d13618bd17c 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -12,8 +12,10 @@ int main(int argc, char **argv){
 const char *pos_arg1=NULL;
 const char *pos_arg2=NULL;
 const char *string_val=NULL;
+notmuch_bool_t bool_val = FALSE;
 
 notmuch_opt_desc_t options[] = {
+   { .opt_bool = _val, .name = "boolean" },
{ .opt_keyword = _val, .name = "keyword", .keywords =
  (notmuch_keyword_t []){ { "one", 1 },
  { "two", 2 },
@@ -35,6 +37,9 @@ int main(int argc, char **argv){
 if (opt_index < 0)
return 1;
 
+if (bool_val)
+   printf("boolean %d\n", bool_val);
+
 if (kw_val)
printf("keyword %d\n", kw_val);
 
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 01/15] cli: strip trailing "/" from the final maildir path in notmuch insert

2017-10-01 Thread Jani Nikula
Several subtle interconnected changes here:

- If the folder name passed as argument is the empty string "" or
  slash "/", the final maildir path would end up having "//" in it. We
  should strip the final maildir path, not folder.

- The folder variable should really be const char *, another reason
  not to modify it.

- The maildir variable is only const to let us point it at db_path
  directly.

To be able to strip the maildir variable, always allocate it. Default
folder to the empty string "", and don't treat folder not being
present on the command line as anything special.

As a side effect, we also create the cur/new/tmp in the top level
directory if they're not there and --create-folder is given.
---
 notmuch-insert.c | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 648bd944a7b1..040b6aa0de3b 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -452,12 +452,12 @@ notmuch_insert_command (notmuch_config_t *config, int 
argc, char *argv[])
 size_t new_tags_length;
 tag_op_list_t *tag_ops;
 char *query_string = NULL;
-char *folder = NULL;
+const char *folder = "";
 notmuch_bool_t create_folder = FALSE;
 notmuch_bool_t keep = FALSE;
 notmuch_bool_t no_hooks = FALSE;
 notmuch_bool_t synchronize_flags;
-const char *maildir;
+char *maildir;
 char *newpath;
 int opt_index;
 unsigned int i;
@@ -509,23 +509,21 @@ notmuch_insert_command (notmuch_config_t *config, int 
argc, char *argv[])
return EXIT_FAILURE;
 }
 
-if (folder == NULL) {
-   maildir = db_path;
-} else {
-   strip_trailing (folder, '/');
-   if (! is_valid_folder_name (folder)) {
-   fprintf (stderr, "Error: invalid folder name: '%s'\n", folder);
-   return EXIT_FAILURE;
-   }
-   maildir = talloc_asprintf (config, "%s/%s", db_path, folder);
-   if (! maildir) {
-   fprintf (stderr, "Out of memory\n");
-   return EXIT_FAILURE;
-   }
-   if (create_folder && ! maildir_create_folder (config, maildir))
-   return EXIT_FAILURE;
+if (! is_valid_folder_name (folder)) {
+   fprintf (stderr, "Error: invalid folder name: '%s'\n", folder);
+   return EXIT_FAILURE;
+}
+
+maildir = talloc_asprintf (config, "%s/%s", db_path, folder);
+if (! maildir) {
+   fprintf (stderr, "Out of memory\n");
+   return EXIT_FAILURE;
 }
 
+strip_trailing (maildir, '/');
+if (create_folder && ! maildir_create_folder (config, maildir))
+   return EXIT_FAILURE;
+
 /* Set up our handler for SIGINT. We do not set SA_RESTART so that copying
  * from standard input may be interrupted. */
 memset (, 0, sizeof (struct sigaction));
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 02/15] cli: use designated initializers for opt desc

2017-10-01 Thread Jani Nikula
Several changes at once, just to not have to change the same lines
several times over:

- Use designated initializers to initialize opt desc arrays.

- Only initialize the needed fields.

- Remove arg_id (short options) as unused.

- Replace opt_type and output_var with several type safe output
  variables, where the output variable being non-NULL determines the
  type. Introduce checks to ensure only one is set. The downside is
  some waste of const space per argument; this could be saved by
  retaining opt_type and using a union, but that's still pretty
  verbose.

- Fix some variables due to the type safety. Mostly a good thing, but
  leads to some enums being changed to ints. This is pedantically
  correct, but somewhat annoying. We could also cast, but that defeats
  the purpose a bit.

- Terminate the opt desc arrays using {}.

The output variable type safety and the ability to add new fields for
just some output types or arguments are the big wins. For example, if
we wanted to add a variable to set when the argument is present, we
could do so for just the arguments that need it.

Beauty is in the eye of the beholder, but I think this looks nice when
defining the arguments, and reduces some of the verbosity we have
there.

---

v2: don't use unnamed struct in notmuch_opt_desc_t to cater for old
compilers
---
 command-line-arguments.c | 87 ++--
 command-line-arguments.h | 38 -
 notmuch-client.h |  2 +-
 notmuch-compact.c|  8 ++---
 notmuch-count.c  | 16 -
 notmuch-dump.c   | 14 
 notmuch-insert.c | 12 +++
 notmuch-new.c| 12 +++
 notmuch-reindex.c|  4 +--
 notmuch-reply.c  | 12 +++
 notmuch-restore.c| 14 
 notmuch-search.c | 46 -
 notmuch-show.c   | 22 ++--
 notmuch-tag.c| 12 +++
 notmuch.c| 22 ++--
 test/arg-test.c  | 21 ++--
 test/hex-xcode.c | 10 +++---
 test/random-corpus.c | 20 +--
 18 files changed, 185 insertions(+), 187 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index dc517b06ff60..f1a5b2324337 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -22,12 +22,10 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
 
 while (keywords->name) {
if (strcmp (arg_str, keywords->name) == 0) {
-   if (arg_desc->output_var) {
-   if (arg_desc->opt_type == NOTMUCH_OPT_KEYWORD_FLAGS)
-   *((int *)arg_desc->output_var) |= keywords->value;
-   else
-   *((int *)arg_desc->output_var) = keywords->value;
-   }
+   if (arg_desc->opt_flags)
+   *arg_desc->opt_flags |= keywords->value;
+   else
+   *arg_desc->opt_keyword = keywords->value;
return TRUE;
}
keywords++;
@@ -43,15 +41,15 @@ static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
 
 if (next == '\0') {
-   *((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+   *arg_desc->opt_bool = TRUE;
return TRUE;
 }
 if (strcmp (arg_str, "false") == 0) {
-   *((notmuch_bool_t *)arg_desc->output_var) = FALSE;
+   *arg_desc->opt_bool = FALSE;
return TRUE;
 }
 if (strcmp (arg_str, "true") == 0) {
-   *((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+   *arg_desc->opt_bool = TRUE;
return TRUE;
 }
 fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", 
arg_str, arg_desc->name);
@@ -67,7 +65,7 @@ _process_int_arg (const notmuch_opt_desc_t *arg_desc, char 
next, const char *arg
return FALSE;
 }
 
-*((int *)arg_desc->output_var) = strtol (arg_str, , 10);
+*arg_desc->opt_int = strtol (arg_str, , 10);
 if (*endptr == '\0')
return TRUE;
 
@@ -87,10 +85,35 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char *
fprintf (stderr, "String argument for option \"%s\" must be 
non-empty.\n", arg_desc->name);
return FALSE;
 }
-*((const char **)arg_desc->output_var) = arg_str;
+*arg_desc->opt_string = arg_str;
 return TRUE;
 }
 
+/* Return number of non-NULL opt_* fields in opt_desc. */
+static int _opt_set_count (const notmuch_opt_desc_t *opt_desc)
+{
+return
+   !!opt_desc->opt_inherit +
+   !!opt_desc->opt_bool +
+   !!opt_desc->opt_int +
+   !!opt_desc->opt_keyword +
+   !!opt_desc->opt_flags +
+   !!opt_desc->opt_string +
+   !!opt_desc->opt_position;
+}
+
+/* Return TRUE if opt_desc is valid. */
+static notmuch_bool_t _opt_valid (const notmuch_opt_desc_t *opt_desc)
+{
+int n = _opt_set_count (opt_desc);
+
+if (n > 1)
+   INTERNAL_ERROR ("more than one non-NULL 

[PATCH v2 00/15] cli: argument parsing changes

2017-10-01 Thread Jani Nikula
This series combines the designated initializers for argument parsing
from id:20170930213239.15392-1-j...@nikula.org and the argument parsing
refactoring from id:cover.1505853159.git.j...@nikula.org.

Additionally patch 1 handles some const confusion in notmuch-insert
before it becomes a problem in patch 2, and patches 5-7 add support for
the .present field to opt desc discussed in
id:87lgkvrhpa@nikula.org. Some more tests are sprinkled here and
there too.

BR,
Jani.


Jani Nikula (15):
  cli: strip trailing "/" from the final maildir path in notmuch insert
  cli: use designated initializers for opt desc
  test: add boolean argument to arg-test
  test: add opt_inherit to arg-test
  cli: add .present field to opt desc to check if the arg was present
  test: expand argument parsing tests
  cli: use the arg parser .present feature to handle show
--entire-thread
  hex-xcode: use notmuch_bool_t for boolean arguments
  cli: use notmuch_bool_t for boolean argument in show
  cli: refactor boolean argument processing
  cli: change while to for in keyword argument processing
  cli: reduce indent in keyword argument processing
  cli: add support for --no- prefixed boolean and keyword flag arguments
  cli: use the negating boolean support for new and insert --no-hooks
  test: expand argument parsing sanity checks

 command-line-arguments.c  | 174 ++
 command-line-arguments.h  |  41 --
 notmuch-client.h  |   2 +-
 notmuch-compact.c |   8 +-
 notmuch-count.c   |  16 ++--
 notmuch-dump.c|  14 ++--
 notmuch-insert.c  |  48 ++--
 notmuch-new.c |  18 ++---
 notmuch-reindex.c |   4 +-
 notmuch-reply.c   |  12 +--
 notmuch-restore.c |  14 ++--
 notmuch-search.c  |  46 +--
 notmuch-show.c|  41 +-
 notmuch-tag.c |  12 +--
 notmuch.c |  22 +++---
 test/T410-argument-parsing.sh |  53 -
 test/arg-test.c   |  56 +-
 test/hex-xcode.c  |  12 +--
 test/random-corpus.c  |  20 ++---
 19 files changed, 350 insertions(+), 263 deletions(-)

-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Jani Nikula
On Sun, 01 Oct 2017, Tomi Ollila <tomi.oll...@iki.fi> wrote:
> On Sun, Oct 01 2017, Jani Nikula wrote:
>
>> Several changes at once, just to not have to change the same lines
>> several times over:
>>
>> - Use designated initializers to initialize opt desc arrays.
>>
>> - Only initialize the needed fields.
>>
>> - Remove arg_id (short options) as unused.
>>
>> - Replace opt_type and output_var with several type safe output
>>   variables, where the output variable being non-NULL determines the
>>   type. Introduce checks to ensure only one is set. The downside is
>>   some waste of const space per argument; this could be saved by
>>   retaining opt_type and using a union, but that's still pretty
>>   verbose.
>>
>> - Fix some variables due to the type safety. Mostly a good thing, but
>>   leads to some enums being changed to ints. This is pedantically
>>   correct, but somewhat annoying. We could also cast, but that defeats
>>   the purpose a bit.
>>
>> - Terminate the opt desc arrays using {}.
>>
>> The output variable type safety and the ability to add new fields for
>> just some output types or arguments are the big wins. For example, if
>> we wanted to add a variable to set when the argument is present, we
>> could do so for just the arguments that need it.
>>
>> Beauty is in the eye of the beholder, but I think this looks nice when
>> defining the arguments, and reduces some of the verbosity we have
>> there.
>
> Good stuff. It just doesn't longer compile on older compilers (so some
> note on commit message should be added):

Does this on top fix it? I used the unnamed struct just for clarity, and
it doesn't serve a functional purpose.

BR,
Jani.

diff --git a/command-line-arguments.h b/command-line-arguments.h
index 97134ad535ee..799b7fef3f65 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -16,15 +16,13 @@ typedef struct notmuch_keyword {
 /* Describe one option. */
 typedef struct notmuch_opt_desc {
 /* One and only one of these must be set. */
-struct {
-   const struct notmuch_opt_desc *opt_inherit;
-   notmuch_bool_t *opt_bool;
-   int *opt_int;
-   int *opt_keyword;
-   int *opt_flags;
-   const char **opt_string;
-   const char **opt_position;
-};
+const struct notmuch_opt_desc *opt_inherit;
+notmuch_bool_t *opt_bool;
+int *opt_int;
+int *opt_keyword;
+int *opt_flags;
+const char **opt_string;
+const char **opt_position;
 
 /* Must be set except for opt_inherit and opt_position. */
 const char *name;
--

>
> CC  -g -O2 notmuch.o
> notmuch.c:53: error: unknown field ‘opt_bool’ specified in initializer
> notmuch.c:53: warning: missing braces around initializer
> notmuch.c:53: warning: (near initialization for
> ‘notmuch_shared_options[0].’)
> notmuch.c:53: warning: initialization from incompatible pointer type
> notmuch.c:53: warning: missing initializer
> notmuch.c:53: warning: (near initialization for
> ‘notmuch_shared_options[0]..opt_bool’)
> notmuch.c:54: error: unknown field ‘opt_bool’ specified in initializer
> notmuch.c:54: warning: initialization from incompatible pointer type
> notmuch.c:54: warning: missing initializer
> notmuch.c:54: warning: (near initialization for
> ‘notmuch_shared_options[1]..opt_bool’)
> notmuch.c:55: error: unknown field ‘opt_string’ specified in initializer
> notmuch.c:55: warning: initialization from incompatible pointer type
> notmuch.c:55: warning: missing initializer
> notmuch.c:55: warning: (near initialization for
> ‘notmuch_shared_options[2]..opt_bool’)
> notmuch.c:56: warning: missing initializer
> notmuch.c:56: warning: (near initialization for
> ‘notmuch_shared_options[3].’)
> notmuch.c: In function ‘notmuch_minimal_options’:
> notmuch.c:85: error: unknown field ‘opt_inherit’ specified in initializer
> notmuch.c:85: warning: missing braces around initializer
> notmuch.c:85: warning: (near initialization for ‘options[0].’)
> notmuch.c:85: warning: missing initializer
> notmuch.c:85: warning: (near initialization for
> ‘options[0]..opt_bool’)
> notmuch.c:86: warning: missing initializer
> notmuch.c:86: warning: (near initialization for ‘options[1].’)
> notmuch.c: In function ‘main’:
> notmuch.c:414: error: unknown field ‘opt_string’ specified in initializer
> notmuch.c:414: warning: missing braces around initializer
> notmuch.c:414: warning: (near initialization for ‘options[0].’)
> notmuch.c:414: warning: initialization from incompatible pointer type
> notmuch.c:414: warning: missing initializer
> notmuch.c:414: warning: (near initialization for
> ‘options[0]..opt_bool’)
> notmuch.c:415: error: unknown field ‘opt_inherit’ specifi

Re: [PATCH 0/9] argument parsing fixes and improvements

2017-09-30 Thread Jani Nikula
On Sat, 30 Sep 2017, Jani Nikula <j...@nikula.org> wrote:
> Looking at the defaults from another angle, if we don't want the ability
> to set --foo=default explicitly, I still think passing ints as booleans
> to the argument parser and checking if a boolean is neither true nor
> false is the wrong thing to do. I'd also like to convert to stdbool
> more. But those should not be a reason to convert essentially boolean
> arguments to keyword arguments. I think we need a way to have the
> argument parser tell us if an argument was present or not.

id:20170930213239.15392-1-j...@nikula.org would make it easier to add,
say, a notmuch_bool_t *present field to notmuch_opt_desc_t that we could
set whenever we see the option and we want to know the difference.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] cli: use designated initializers for opt desc

2017-09-30 Thread Jani Nikula
Several changes at once, just to not have to change the same lines
several times over:

- Use designated initializers to initialize opt desc arrays.

- Only initialize the needed fields.

- Remove arg_id (short options) as unused.

- Replace opt_type and output_var with several type safe output
  variables, where the output variable being non-NULL determines the
  type. Introduce checks to ensure only one is set. The downside is
  some waste of const space per argument; this could be saved by
  retaining opt_type and using a union, but that's still pretty
  verbose.

- Fix some variables due to the type safety. Mostly a good thing, but
  leads to some enums being changed to ints. This is pedantically
  correct, but somewhat annoying. We could also cast, but that defeats
  the purpose a bit.

- Terminate the opt desc arrays using {}.

The output variable type safety and the ability to add new fields for
just some output types or arguments are the big wins. For example, if
we wanted to add a variable to set when the argument is present, we
could do so for just the arguments that need it.

Beauty is in the eye of the beholder, but I think this looks nice when
defining the arguments, and reduces some of the verbosity we have
there.
---
 command-line-arguments.c | 87 ++--
 command-line-arguments.h | 40 +-
 notmuch-client.h |  2 +-
 notmuch-compact.c|  8 ++---
 notmuch-count.c  | 16 -
 notmuch-dump.c   | 14 
 notmuch-insert.c | 14 
 notmuch-new.c| 12 +++
 notmuch-reindex.c|  4 +--
 notmuch-reply.c  | 12 +++
 notmuch-restore.c| 14 
 notmuch-search.c | 46 -
 notmuch-show.c   | 22 ++--
 notmuch-tag.c| 12 +++
 notmuch.c| 22 ++--
 test/arg-test.c  | 21 ++--
 test/hex-xcode.c | 10 +++---
 test/random-corpus.c | 20 +--
 18 files changed, 188 insertions(+), 188 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index dc517b06ff60..f1a5b2324337 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -22,12 +22,10 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
 
 while (keywords->name) {
if (strcmp (arg_str, keywords->name) == 0) {
-   if (arg_desc->output_var) {
-   if (arg_desc->opt_type == NOTMUCH_OPT_KEYWORD_FLAGS)
-   *((int *)arg_desc->output_var) |= keywords->value;
-   else
-   *((int *)arg_desc->output_var) = keywords->value;
-   }
+   if (arg_desc->opt_flags)
+   *arg_desc->opt_flags |= keywords->value;
+   else
+   *arg_desc->opt_keyword = keywords->value;
return TRUE;
}
keywords++;
@@ -43,15 +41,15 @@ static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
 
 if (next == '\0') {
-   *((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+   *arg_desc->opt_bool = TRUE;
return TRUE;
 }
 if (strcmp (arg_str, "false") == 0) {
-   *((notmuch_bool_t *)arg_desc->output_var) = FALSE;
+   *arg_desc->opt_bool = FALSE;
return TRUE;
 }
 if (strcmp (arg_str, "true") == 0) {
-   *((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+   *arg_desc->opt_bool = TRUE;
return TRUE;
 }
 fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", 
arg_str, arg_desc->name);
@@ -67,7 +65,7 @@ _process_int_arg (const notmuch_opt_desc_t *arg_desc, char 
next, const char *arg
return FALSE;
 }
 
-*((int *)arg_desc->output_var) = strtol (arg_str, , 10);
+*arg_desc->opt_int = strtol (arg_str, , 10);
 if (*endptr == '\0')
return TRUE;
 
@@ -87,10 +85,35 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char *
fprintf (stderr, "String argument for option \"%s\" must be 
non-empty.\n", arg_desc->name);
return FALSE;
 }
-*((const char **)arg_desc->output_var) = arg_str;
+*arg_desc->opt_string = arg_str;
 return TRUE;
 }
 
+/* Return number of non-NULL opt_* fields in opt_desc. */
+static int _opt_set_count (const notmuch_opt_desc_t *opt_desc)
+{
+return
+   !!opt_desc->opt_inherit +
+   !!opt_desc->opt_bool +
+   !!opt_desc->opt_int +
+   !!opt_desc->opt_keyword +
+   !!opt_desc->opt_flags +
+   !!opt_desc->opt_string +
+   !!opt_desc->opt_position;
+}
+
+/* Return TRUE if opt_desc is valid. */
+static notmuch_bool_t _opt_valid (const notmuch_opt_desc_t *opt_desc)
+{
+int n = _opt_set_count (opt_desc);
+
+if (n > 1)
+   INTERNAL_ERROR ("more than one non-NULL opt_* field for argument 
\"%s\"",
+   opt_desc->name);
+
+

Re: [PATCH 0/6] Sort by from and subject

2017-09-30 Thread Jani Nikula
On Sat, 30 Sep 2017, William Casarin <j...@jb55.com> wrote:
> Jani Nikula <j...@nikula.org> writes:
>
>> I think there are two considerations here:
>>
>> First, is this something we want to have? Is this generally useful?
>
> Sorting by from and subject are in most mail clients (mutt, gnus, outlook...)

Which of those display results as threads, and of those that do, how do
they sort the threads? In the notmuch case, the threads would be sorted
based on one of the matching messages. Which one should it be? For
current date based searches, the message used for sorting is also
selected based on date.

If we expand the sorting, perhaps we should also think about sorting by
relevance provided by Xapian. Not saying you'd have to do this, but I'd
like to figure out how all of this should work. The implementation
doesn't look particularly difficult, it's the design that's harder to
get right.

>> There's still the issue of From: and Subject: needing more heuristic for
>> useful sorting that I mentioned in id:87efrm70ai@nikula.org.
>
> I think I understand what you mean in id:87efrm70ai@nikula.org but I
> don't have enough knowledge of notmuch to implement what you're asking
> :(. I believe these are rare cases because I haven't ran into the issue
> you described?

Look at the subject line of this message. Should it be sorted starting
at "Re:", "[PATCH", or "Sort"? You could argue for and against any one
of them. Contrast that with the thread sorting above: If the matching
message in this thread changes from one with vs. without "Re:", the sort
placement of the thread could change considerably.

It's common for some corporate mail systems to switch "Firstname
Lastname" in messages to "Lastname, Firstname". Should we do something
about that?

Arguably we could do the sorting first, and think of ways to improve it
afterwards.

>> Second, if we decide we want this, IMHO the interfaces (both human and
>> the lib) need to split the sort key and sort order from the
>> start. Fixing it later on is not going to be fun.
>
> I agree, I figured that would have been a larger refactor so I decided
> not to mix it in with this one. I'll start working on a branch that
> addresses this.

Please wait for feedback from others first, to not waste effort based on
just my opinions. I don't make the decisions here. :)


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/6] Sort by from and subject

2017-09-30 Thread Jani Nikula
On Mon, 25 Sep 2017, William Casarin  wrote:
> This patch series replaces my original set[1]. I've been using this
> extensivly for about 3 weeks now and I'm pretty happy with it. I've
> added the ability to change sort-order on the fly with the O key
> binding.

I think there are two considerations here:

First, is this something we want to have? Is this generally useful?
There's still the issue of From: and Subject: needing more heuristic for
useful sorting that I mentioned in id:87efrm70ai@nikula.org.

Second, if we decide we want this, IMHO the interfaces (both human and
the lib) need to split the sort key and sort order from the
start. Fixing it later on is not going to be fun.


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/9] argument parsing fixes and improvements

2017-09-30 Thread Jani Nikula
On Mon, 25 Sep 2017, David Bremner  wrote:
> Daniel Kahn Gillmor  writes:
>> So from an implementation point of view, it's definitely cleaner/simpler
>> to have an internally "explicitly unset" state for the CLI flags.
>
> I'm trying to separate-out/defer impliementation questions here, at
> least until I'm clear on the UI.

Patches 1-5 and 8 in this series would be non-committal refactoring and
cleanup in the mean time. ;)

>> From a CLI UI perspective: do we want to be able to send --foo=default
>> for a boolean explicitly?
>
> I have the feeling that maybe Jani does, but I'm not sure (as sometimes
> happens) why my way of thinking about it isn't the only obvious way ;).

I'm undecided. Definitely maybe. Going by "worse is better", the
implementation *does* impact the decision, at least it impacts my
opinion. For example, I don't think we'll open the database before
argument parsing even if that turned out to be "the right thing".

Looking at the defaults from another angle, if we don't want the ability
to set --foo=default explicitly, I still think passing ints as booleans
to the argument parser and checking if a boolean is neither true nor
false is the wrong thing to do. I'd also like to convert to stdbool
more. But those should not be a reason to convert essentially boolean
arguments to keyword arguments. I think we need a way to have the
argument parser tell us if an argument was present or not.


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: notmuch-emacs: Fcc to top-level directory given by database.path

2017-09-30 Thread Jani Nikula
On Sat, 23 Sep 2017, David Bremner  wrote:
> Do you happen to know if it calls with an empty string as the folder
> name? It would be consistent with searching for that to insert at the
> top level.

notmuch insert --folder= or --folder="" does not work:

String argument for option "folder" must be non-empty.
Unrecognized option: --folder=

It seems that our argument parser doesn't accept empty strings at all,
which is a bit of a limitation. It would be logical for the empty string
to work here, and AFAICT the notmuch insert code would handle this if
the argument was let through the parsing.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 5/6] cli/new: support // in new.ignore

2017-09-26 Thread Jani Nikula
On Mon, 25 Sep 2017, David Bremner <da...@tethera.net> wrote:
> Jani Nikula <j...@nikula.org> writes:
>
>> +  A regular expression delimited with // that will be matched
>> +  against the path of the file or directory relative to the
>> +  database path. The beginning and end of string must be
>> +  explictly anchored. For example, /.*/foo$/ would match
>> +  "bar/foo" and "bar/baz/foo", but not "foo" or "bar/foobar".
>
> Is it worth remarking that '/' does not need to be escaped? or more
> interestingly, what happens if it is escaped, do things break?

It just gets passed down to regcomp() with the escape and all. I'm not
sure it's worth trying to exhaustively explain everything.

>>  
>> +static notmuch_bool_t
>> +_setup_ignore (notmuch_config_t *config, add_files_state_t *state)
>> +{
>
> Would be nice to document what this return value means.

Something like:

/* Jani forgot to do anything useful with the return value */

I think it was originally meant to return false on illegal input
(e.g. opening '/' without closing '/') or regcomp() failing, but then I
opted for the more lax approach. xregcomp() warns about failures though.

>
>> +const char **ignore_list, **ignore;
>> +int nregex = 0, nverbatim = 0;
>> +const char **verbatim = NULL;
>> +regex_t *regex = NULL;
>> +
>> +ignore_list = notmuch_config_get_new_ignore (config, NULL);
>> +if (! ignore_list)
>> +return TRUE;
>> +
>> +for (ignore = ignore_list; *ignore; ignore++) {
>> +const char *s = *ignore;
>> +size_t len = strlen (s);
>> +
>> +if (len > 2 && s[0] == '/' && s[len - 1] == '/') {
>
> One thing we eventually settled on in the query parser is that an
> opening '/' without a trailing '/' is an errror. But perhaps it's fine
> to take a more permissive approach here.

I'm fine either way, I just chose to be permissive.

So do I make the function void and drop the return values, or make it
check and return errors?

>
>> +
>> +if (! state->ignore_regex_length)
>> +return FALSE;
>
> It's a nitpick, even by the standards of this review, but I'd prefer an
> explicit '> 0' check.

ITYM (state->ignore_regex_length == 0) but ack.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/2] test: add emacs reply test for subjects with TAB

2017-09-26 Thread Jani Nikula
Expect TABs to be sanitized from the subject line. Known broken.
---
 test/T310-emacs.sh | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index fde11790a600..2ef566bac490 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -401,6 +401,29 @@ Notmuch Test Suite  writes:
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Reply within emacs to a message with TAB in subject"
+test_subtest_known_broken
+test_emacs '(let ((message-hidden-headers ''()))
+   (notmuch-search 
"id:1258471718-6781-1-git-send-email-dotted...@dottedmag.net")
+   (notmuch-test-wait)
+   (notmuch-search-show-thread)
+   (notmuch-test-wait)
+   (notmuch-show-reply-sender)
+   (test-output))'
+sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
+sed -i -e 's/^References: <.*>$/References: /' OUTPUT
+sed -i -e '/^--text follows this line--$/q' OUTPUT
+cat 
+To: Mikhail Gusarov 
+Subject: Re: [notmuch] [PATCH 1/2] Close message file after parsing message 
headers
+In-Reply-To: 
+Fcc: ${MAIL_DIR}/sent
+References: 
+--text follows this line--
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "Reply from alternate address within emacs"
 add_message '[from]="Sender "' \
 [to]=test_suite_ot...@notmuchmail.org
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/2] emacs: sanitize subject in replies

2017-09-26 Thread Jani Nikula
Commit a7964c86d125 ("emacs: Sanitize authors and subjects in search
and show") added sanitization of header information for display. Do
the same for reply subjects.

This fixes the long-standing annoying artefact of certain versions of
mailman using tab as folding whitespace, leading to tabs in reply
subjects.
---
 emacs/notmuch-mua.el | 2 +-
 test/T310-emacs.sh   | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index fd64b362b542..7a341ebf0588 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -218,7 +218,7 @@ mutiple parts get a header."
 else
 collect pair)))
  (notmuch-mua-mail (plist-get reply-headers :To)
-   (plist-get reply-headers :Subject)
+   (notmuch-sanitize (plist-get reply-headers 
:Subject))
(notmuch-headers-plist-to-alist reply-headers)
nil (notmuch-mua-get-switch-function
 
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 2ef566bac490..4456bc659158 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -402,7 +402,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Reply within emacs to a message with TAB in subject"
-test_subtest_known_broken
 test_emacs '(let ((message-hidden-headers ''()))
(notmuch-search 
"id:1258471718-6781-1-git-send-email-dotted...@dottedmag.net")
(notmuch-test-wait)
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


emacs: reply subject sanitization

2017-09-26 Thread Jani Nikula
v2 of id:20170915155716.19597-1-j...@nikula.org, now with test.

BR,
Jani.

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 24/24] devel: add script to test out-of-tree builds

2017-09-25 Thread Jani Nikula
Something I used for 'git bisect run', but we should really add this
as part of our process.

Can also be used for running out-of-tree tests with e.g.:

$ devel/out-of-tree-build-check.sh V=1 test
---
 devel/out-of-tree-build-check.sh | 16 
 1 file changed, 16 insertions(+)
 create mode 100755 devel/out-of-tree-build-check.sh

diff --git a/devel/out-of-tree-build-check.sh b/devel/out-of-tree-build-check.sh
new file mode 100755
index ..917752ff72cb
--- /dev/null
+++ b/devel/out-of-tree-build-check.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+# test out-of-tree builds in a temp directory
+# passes all args to make
+
+set -eu
+
+srcdir="$(cd "$(dirname "$0")"/.. && pwd)"
+builddir=$(mktemp -d)
+
+cd $builddir
+
+$srcdir/configure
+make "$@"
+
+cd $srcdir
+rm -rf $builddir
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


  1   2   3   4   5   6   7   8   9   10   >