This fixes races in thread-local and global tagging in notmuch-search
(e.g., "+", "-", "a", "*", etc.). Previously, these would modify tags
of new messages that arrived after the search. Now they only operate
on the messages that were in the threads when the search was
performed. This prevents surprises like archiving messages that
arrived in a thread after the search results were shown.
This eliminates `notmuch-search-find-thread-id-region(-search)'
because these functions strongly encouraged racy usage.
This fixes the two broken tests added by the previous patch.
Previously, this was in notmuch.el, but all of the other notmuch call
wrappers were in notmuch-lib.el. Move `notmuch-call-notmuch-process'
to live with its friends. This happens to fix a missing dependency
from notmuch-tag.el, which required notmuch-lib, but not notmuch.
We want to load notmuch-tree when notmuch is loaded, so include it as
a require in notmuch.el. To avoid circular dependency we need to move
one keybinding from notmuch-tree.el to notmuch.el: it makes sense for
it to be defined there anyway.
Since tree is now loaded by default there is no need to print a
message when it is loaded.
notmuch-help is in notmuch.el not notmuch-lib.el and this is
incovenient for the way pick/tree uses it. I think lib makes more
sense anyway so move it there.
Authors and subjects can contain embedded, encoded control characters
like "\n" and "\t" that mess up display. Transform control characters
into spaces everywhere we display them in search and show.
This is similar to the previous commit, but applies to search.
Search is somewhat more complicated because its tagging operations can
also apply to a region. Hence, this lifts interactive prompting into
a helper function. This also takes advantage of the new ability to
provide a prompt to distinguish tagging a single thread from tagging a
region of threads.
The calling convention for `notmuch-tag' changed in commit 97aa3c06 to
take a list of tag changes instead of a &rest argument, but the call
from `notmuch-search-tag-all' still passed a &rest argument. This
happened to work for interactive calls because tag-changes would be
nil, so the `apply' call would pass only the query string to
`notmuch-tag' and simply omit the &optional tag-changes argument.
Currently notmuch-show looks at the prefix-arg directly via
current-prefix-arg. This changes it to use the interactive
specification.
One test (for elide-toggle functionality) set the prefix arg
directly. Update this test to set the new argument directly.
This improves the function documentation for many interactive
commands, either by improving their documentation string where the
improvement also makes sense for programmatic use or by adding a
'notmuch-doc property where it doesn't.
For nearly all commands that support a prefix argument, this adds a
'notmuch-prefix-doc property to document their prefixed behavior This
omits prefix documentation for a few commands where I thought the
prefixed behavior was too obscure (or too complex to fit in one line).
Traditionally, function documentation strings are intended primarily
for programmers, rather than users. They're written from the
perspective of calling the function, not interactively invoking it.
They're only ever displayed along with the function prototype (and
often refer to argument names). And built-in help commands like
`describe-bindings' show the name of the command, not its
documentation.
The notmuch help system is like `describe-bindings', but tries to be
more user-friendly by displaying documentation strings, rather than
Elisp command names. For most commands, this is fine, but for some
the "programmer description" is inappropriate for interactive use.
This is particularly noticeable for commands that take an optional
prefix argument.
This patch adds support for two symbol properties: notmuch-doc and
notmuch-prefix-doc, which let a command override its interactive
documentation and provide separate documentation for its prefixed
invocation. If notmuch-prefix-doc is present, we add an extra line to
the help giving the prefixed key sequence along with the documentation
for the prefixed command.
In the recent changes for search order handling the default-value of
notmuch-search-oldest-first was used. However, default-value needs a
symbol so the symbol-name needs to be quoted.
This missing quote was causing strange sort-orders in some cases.
The only user-visible effect of this should be that "G" now works in
show mode (previously it was unbound for no apparent reason).
This shared keymap gives us one place to put global commands, which
both forces us to think about what commands should be global, and
ensures their bindings can't diverge (like the missing "G" in show).
This converts notmuch-help to use map-keymap for all keymap traversal.
This generally cleans up and simplifies construction of keymap
documentation, and also makes notmuch-help support anything that can
be in a keymap, including more esoteric stuff like multiple
inheritance.
This unifies the various refresh and poll-and-refresh functions we
have for different modes. Now all modes bind "=" and "G" (except
show, which doesn't bind "G" for some reason) to
`notmuch-refresh-this-buffer' and
`notmuch-poll-and-refresh-this-buffer', respectively.
Since notmuch-hello doesn't need this any more, we can remove this
hack. This also eliminates `notmuch-search-quit', so now all modes
bind "q" to `notmuch-kill-this-buffer'.
Previously, if `notmuch-search' was called interactively (bound to "s"
in search and show, but not hello), it would always use newest-first.
However, `notmuch-hello-search' (bound to "s" in hello) and
`notmuch-hello-widget-search` would call it with the user-configured
sort order. This inconsistency seems unintentional, so change
`notmuch-search' to use the user-configured sort order when called
interactively.
Occasionally, when the user killed the search buffer when the CLI
process was still running, Emacs would run the
notmuch-start-notmuch-sentinel sentinel twice. The first call would
process and delete the error output file and the second would fail
with an "Opening input file: no such file or directory, ..." error
when attempting to access the error file.
Emacs isn't supposed to run the sentinel twice. The reason it does is
rather subtle (and probably a bug in Emacs):
1) When the user kills the search buffer, Emacs invokes
kill_buffer_processes, which sends a SIGHUP to notmuch, but doesn't do
anything else. Meanwhile, suppose the notmuch search process has
printed some more output, but Emacs hasn't consumed it yet (this is
critical and is why this error only happens sometimes).
2) Emacs gets a SIGCHLD from the dying notmuch process, which invokes
handle_child_signal, which sets the new process status, but can't do
anything else because it's a signal handler.
3) Emacs returns to its idle loop, which calls status_notify, which
sees that the notmuch process has a new status. This is where things
get interesting.
3.1) Emacs guarantees that it will run process filters on any
unconsumed output before running the process sentinel, so
status_notify calls read_process_output, which consumes the final
output and calls notmuch-search-process-filter.
3.1.1) notmuch-search-process-filter checks if the search buffer is
still alive and, since it's not, it calls delete-process.
3.1.1.1) delete-process correctly sees that the process is already
dead and doesn't try to send another signal, *but* it still modifies
the status to "killed". To deal with the new status, it calls
status_notify. Dun dun dun. We've seen this function before.
3.1.1.1.1) The *recursive* status_notify invocation sees that the
process has a new status and doesn't have any more output to consume,
so it invokes our sentinel and returns.
3.2) The outer status_notify call (which we're still in) is now done
flushing pending process output, so it *also* invokes our sentinel.
This patch addresses this problem at step 3.1.1, where the filter
calls delete-process, since this is a strange and redundant thing to
do anyway.
Several function docstrings refer to behaviour in docstrings that is
really controlled by notmuch-archive-tags. Add cross references, and
replace hardcoding.
I found several places where a setq is immediately followed by a let
or a let*. This seems to be the pessimal combination, with the
implicit scope of the setq combined with the extra indentation of the let.
I combined these cases into a single let* which I think is easier to read.
I can't see any benefit to the funcall, and it looks like the result
of cut-and-paste from some code that actually used a variable for the
function to call.
In addition to being the Right Thing to do, this noticeably improves
the time taken to display the first page of search results, since it's
roughly an order of magnitude faster than the JSON parser.
Interestingly, it does *not* significantly improve the time to
completely fill a large search buffer because for large search
buffers, the cost of creating author invisibility overlays and
inserting text (which slows down with more overlays) dominates.
However, the time required to display the first page of results is
generally more important to the user experience.
Previously, search started the async notmuch process directly. Now,
it uses `notmuch-start-notmuch'. This simplifies the process sentinel
a bit and means that we no longer have to worry about errors
interleaved with the JSON output.
We also update the tests of Emacs error handling, since the error
output is now separated from the search results buffer.
Apparently Emacs provides a function to stringify errors properly.
Use this in the search sentinel where we have to do our own error
messaging, rather than assuming the first error argument will be the
descriptive string.
This patch extracts the rendering of tags in notmuch-show to
the notmuch-tag file.
This file introduces a `notmuch-tag-formats' variable that associates
each tag to a particular format. This variable can be customized
thanks to the work of Austin Clements. For example,
'(("unread" (propertize tag 'face '(:foreground "red")))
("flagged" (notmuch-tag-format-image tag "star.svg")))
associates a red foreground to the "unread" tag and a star picture to
the "flagged" tag.
Signed-off-by: Damien Cassou <damien.cassou@gmail.com>
We recently switched to popping up a buffer to report CLI errors, but
this was too intrusive, especially for transient errors and especially
since we made fewer things ignore errors. This patch changes this to
display a basic error message in the minibuffer (using Emacs' usual
error handling path) and, if there are additional details, to log
these to a separate error buffer and reference the error buffer from
the minibuffer message. This is more in line with how Emacs typically
handles errors, but makes the details available to the user without
flooding them with the details.
Given this split, we pare down the basic message and make it more
user-friendly, and also make the verbose message even more detailed
(and more debugging-oriented).
This slightly changes the output of an existing test since we now
report non-zero exits with a pop-up buffer instead of at the end of
the search results.
This patch just renames the internal variables for the JSON parser now
it is no longer specific to search mode. It also fixes up the white
space after the previous patch. There should be no functional changes.
This patch splits out the incremental json parser into its own
function.
It moves the main logic of the parser to happen inside the parse
buffer rather than inside the results buffer, but makes sure all
results and all errors are displayed in the results buffer.
It also changes the local parser variables from being buffer
local to the results buffer to being buffer local to the parse buffer,
and sets them up automatically so the caller does not need to.
Finally to keep the diff small this patch does not fix the whitespace,
nor complete the code movement (these are done in subsequent patches)
but it should contain all the functional changes.
Since archiving a thread can now be a user customized set of tag
changes, make reversing this easier. Allow a prefix argument to
notmuch-search-archive-thread to reverse the archiving, similar to the
unarchiving in notmuch-show-archive-message.
Add support for customization of the tag changes that are applied when
a message or a thread is archived. Instead of hard-coded removal of
the "inbox" tag, the user can now specify a list of tag changes to
perform.
* emacs/notmuch.el (notmuch-search-mode):
`notmuch-search-tag-all' currently uses the current query string
instead of `notmuch-search-find-thread-id-region-search', which
might cause a race condition.
The recent change to use json for notmuch-search.el introduced a bug
in the code for keeping position on refresh. The problem is a
comparison between (plist-get result :thread) and a thread-id returned
by notmuch-search-find-thread-id: the latter is prefixed with
"thread:"
We fix this by adding an option to notmuch-search-find-thread-id to
return the bare thread-id. It appears that notmuch-search-refresh-view
is the only caller of notmuch-search that supplies a thread-id so this
change should be safe (but could theoretically break users .emacs
functions).
In commit 5d0883e the function notmuch-search-next-thread was changed.
In particular it only goes to the next message if there is a next
message. This breaks notmuch-show-archive-thread-then-next. Fix this
by going to the "next" message whenever we are on a current message.
At this point, the only remaining functions that don't support
multi-line search result formats are the thread navigation functions.
This patch fixes that by rewriting them in terms of
notmuch-search-result-{beginning,end}.
This changes the behavior of notmuch-search-previous-thread slightly
so that if point isn't at the beginning of a result, it first moves
point to the beginning of the result.
Previously we ignored any notmuch-search-result-format customizations
for tag formatting because we needed to be able to parse back in the
result line and update the tags in place. We no longer do either of
these things, so we can allow customization of this format.
(Coincidentally, previously we still allowed too much customization of
the tags format, since moving it earlier on the line or removing it
from the line would interfere with the tagging mechanism. There is
now no problem with doing such things.)
Since the result object contains everything that the other text
properties recorded, we can remove the other text properties and
simply look in the plist of the appropriate result object.
This simplifies the traversal of regions of results and eliminates the
need for save-excursions (which tend to get in the way of maintaining
point when we make changes to the buffer). It also fixes some strange
corner cases in the old line-based code where results that bordered
the region but were not included in it could be affected by region
commands. Coincidentally, this also essentially enables multi-line
search result formats; the only remaining non-multi-line-capable
functions are notmuch-search-{next,previous}-thread, which are only
used for interactive navigation.
Now that we keep the full thread result object, we can refresh a
result after any changes by simply deleting and reconstructing the
result line from scratch.
A convenient side-effect of this wholesale replacement is that search
now re-applies notmuch-search-line-faces when tags change.
