It turns out that now that we pass an open database into the
subcommands, it is easy to check any requested uuid against the
database at the same time as we process the other shared
arguments. This results in overall less boilerplate code, as well as
making a CLI scope function and variable file scope in notmuch.c.
This is the result of running
$ uncrustify --replace --config devel/uncrustify.cfg *.c *.h
in the top level source directory
Line breaks were then adjusted manually to keep argc and argv
together.
This will allow transitioning individual subcommands to the new
configuration framework. Eventually when they are all converted we can
remove the notmuch_config_t * argument.
For now, live with the parameter shadowing in some some subcommands;
it will go away when they are converted.
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.
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.
Fix the following cppcheck errors:
notmuch-count.c:207: error: Resource leak: input
notmuch-tag.c:238: error: Resource leak: input
We know that the program is shutting down here, but it does no harm to
clean up a bit.
Many of the external links found in the notmuch source can be resolved
using https instead of http. This changeset addresses as many as i
could find, without touching the e-mail corpus or expected outputs
found in tests.
This patch removes the restriction on notmuch-tag that disallows using
both --remove-all and --batch. Combining the two options removes tags
on all messages affected by each query before applying the new tags.
I think it would be no real problem to cut and paste the gdb based
error message test from count to the other clients modified here, but
I'm not currently convinced it's worth the trouble since the code path
being tested is almost the the same, and the tests are relatively
heavyweight.
The function notmuch_exit_if_unmatched_db_uuid is split from
notmuch_process_shared_options because it needs an open notmuch
database.
There are two exceptional cases in uuid handling.
1) notmuch config and notmuch setup don't currently open the database,
so it doesn't make sense to check the UUID.
2) notmuch compact opens the database inside the library, so we either
need to open the database just to check uuid, or change the API.
Unfortunately it seems trickier to support --config globally
The non-trivial changes are in notmuch.c; most of the other changes
consists of blindly inserting two lines into every subcommand.
Apart from the status codes for format mismatches, the non-zero exit
status codes have been arbitrary. Make the cli consistently return
either EXIT_SUCCESS or EXIT_FAILURE.
Move an error condition specific to the 'tag' command out of
parse_tag_command_line so that parse_tag_command_line can be used for
the forthcoming 'insert' command.
Add --remove-all option to "notmuch tag" to remove all tags from the
messages matching query before applying the tag changes. This allows
removal of all tags and unconditional setting of the tags of a
message:
$ notmuch tag --remove-all id:foo@example.com
$ notmuch tag --remove-all +foo +bar id:foo@example.com
without having to resort to the complicated (and still quoting
broken):
$ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') \
id:foo@example.com
$ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') \
+foo +bar id:foo@example.com
This allows specifying config file as a top level argument to notmuch,
and generally makes it possible to override config file options in
main(), without having to touch the subcommands.
If the config file does not exist, one will be created for the notmuch
main command and setup and help subcommands. Help is special in this
regard; the config is created just to avoid errors about missing
config, but it will not be saved.
This also makes notmuch config the talloc context for subcommands.
We now have a notmuch_config_is_new() function to query whether a
config was created or not. Change the notmuch_config_open() is_new
parameter into boolean create_new to determine whether the function
should create a new config if one doesn't exist. This reduces the
complexity of the API.
In case last input for batch tagging was either invalid or skippable
line, notmuch command exited with non-zero value.
After this change if there is at least one invalid line, notmuch
command will exit with non-zero value. Additionally, skipped lines
(last or other) doesn't cause non-zero value to be returned.
Checking and propagating tag_op_list_apply() errors is especially
important with batch tagging, as the processing of the batch input
would not stop otherwise. Additionally this sets the exit code, which
is useful in scripts.
Amended by: David Bremner
Add support for batch tagging operations through stdin to "notmuch
tag". This can be enabled with the new --batch command line option to
"notmuch tag". The input must consist of lines of the format:
+<tag>|-<tag> [...] [--] <query> [...]
Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
<tag> MAY be hex encoded with %NN where NN is the hexadecimal value of
the character. Any ' ' and '%' characters in <tag> and MUST be hex
encoded (using %20 and %25, respectively). For future-proofing, any
'"' characters in <tag> SHOULD be hex-encoded.
Any characters that are not part of <tag> or
MUST NOT be hex encoded.
<query> is passed verbatim to Xapian
Leading and trailing space ' ' is ignored. Empty lines and lines
beginning with '#' are ignored.
Signed-off-by: Jani Nikula <jani@nikula.org>
Hacked-like-crazy-by: David Bremner <david@tethera.net>
Command line parsing is factored out into a function
parse_tag_command_line in tag-util.c.
There is some duplicated code eliminated in tag_query, and a bunch of
translation from using the bare tag_op structs to using that tag-utils
API.
This is now a generic boolean term quoting function. It performs
minimal quoting to produce user-friendly queries.
This could live in tag-util as well, but it is really nothing specific
to tags (although the conventions are specific to Xapian).
The API is changed from "caller-allocates" to "readline-like". The
scan for max tag length is pushed down into the quoting routine.
Furthermore, this now combines the term prefix with the quoted term;
arguably this is just as easy to do in the caller, but this will
nicely parallel the boolean term parsing function to be introduced
shortly.
This is an amalgamation of code written by David Bremner and myself.
This disallows adding empty tags, since nothing but confusion follows
in their wake, and disallows adding tags that begin with "-" because
they are also confusing, the tag "-" is impossible to remove using the
CLI, and because the syntax for removing such tags conflicts with long
argument syntax.
This does not place any restrictions on what tags can be removed, as
that would make it difficult for people who have the misfortune of
already having malformed tags to remove these tags.
It has been a long-standing issue that notmuch_database_open doesn't
return any indication of why it failed. This patch changes its
prototype to return a notmuch_status_t and set an out-argument to the
database itself, like other functions that return both a status and an
object.
In the interest of atomicity, this also updates every use in the CLI
so that notmuch still compiles. Since this patch does not update the
bindings, the Python bindings test fails.
To simplify code, keep all tagging operations in a single array
instead of separate add and remove arrays. Apply tag changes in the
order specified on the command line, instead of first removing and
then adding the tags.
This results in a minor functional change: If a tag is both added and
removed, the last specified operation is now used. Previously the tag
was always added. Change the relevant test to reflect the new
behaviour.
Signed-off-by: Jani Nikula <jani@nikula.org>
This ignores the results of the two writes in sigint handlers even
harder than before.
While my libc lacks the declarations that trigger these warnings, this
can be tested by adding the following to notmuch.h:
__attribute__((warn_unused_result))
ssize_t write(int fd, const void *buf, size_t count);
This optimizes the user's tagging query to exclude messages that won't
be affected by the tagging operation, saving computation and IO for
redundant tagging operations.
For example,
notmuch tag +notmuch to:notmuch@notmuchmail.org
will now use the query
( to:notmuch@notmuchmail.org ) and (not tag:"notmuch")
In the past, we've often suggested that people do this exact
transformation by hand for slow tagging operations. This makes that
unnecessary.
previously we deleted the subcommand name from argv before passing to
the subcommand. In this version, the deletion is done in the actual
subcommands. Although this causes some duplication of code, it allows
us to be more flexible about how we parse command line arguments in
the subcommand, including possibly using off-the-shelf routines like
getopt_long that expect the name of the command in argv[0].
gcc (at least as of version 4.6.0) is kind enough to point these out to us,
(when given -Wunused-but-set-variable explicitly or implicitly via -Wunused
or -Wall).
One of these cases was a legitimately unused variable. Two were simply
variables (named ignored) we were assigning only to squelch a warning about
unused function return values. I don't seem to be getting those warnings
even without setting the ignored variable. And the gcc docs. say that the
correct way to squelch that warning is with a cast to (void) anyway.
Instead of having an API for setting a library-wide flag for
synchronization (notmuch_database_set_maildir_sync) we instead
implement maildir synchronization with two new library functions:
notmuch_message_maildir_flags_to_tags
and notmuch_message_tags_to_maildir_flags
These functions are nicely documented here, (though the implementation
does not quite match the documentation yet---as plainly evidenced by
the current results of the test suite).
Since the name of the configuration parameter here is:
maildir.synchronize_flags
the convention is that the functions to get and set this parameter
should match it in name. Hence:
notmuch_config_get_maildir_synchronize_flags
etc. (as opposed to notmuch_config_get_maildir_sync).
This adds group [maildir] and key 'synchronize_flags' to the
configuration file. Its value enables (true) or diables (false) the
synchronization between notmuch tags and maildir flags. By default,
the synchronization is disabled.
It's not neccessary to sort the results before we apply tags. Xapian
contributor Olly Betts says that savings might be bigger with a cold
file cache and (as unsorted implies really sorted by document id) a better
cache locality when applying tags to messages.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
We rename 'has_more' to 'valid' so that it can function whether
iterating in a forward or reverse direction. We also rename
'advance' to 'move_to_next' to setup parallel naming with
the proposed functions 'move_to_first', 'move_to_last', and
'move_to_previous'.
Glibc (at least) provides the warn_unused_result attribute on write,
(if optimizing and _FORTIFY_SOURCE is defined). So we explicitly
ignore the return value in our signal handler, where we couldn't do
anything anyway.
Compile with:
make CFLAGS="-O -D_FORTIFY_SOURCE"
before this commit to see the warning.
We only rarely need to actually open the database for writing, but we
always create a Xapian::WritableDatabase. This has the effect of
preventing searches and like whilst updating the index.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Carl Worth <cworth@cworth.org>
In particular, notmuch tag -inbox "" tended to take a long time to
run, happened if you hit 'a' on a blank line in the search view and
probably didn't have the desired effect.
Signed-off-by: Keith Packard <keithp@keithp.com>
