From: David Bremner <david@tethera.net>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH 1/4] cli: show: allow sort order to be specified
In-Reply-To: <1435359035-6767-2-git-send-email-markwalters1009@gmail.com>
References: <1435359035-6767-1-git-send-email-markwalters1009@gmail.com>
 <1435359035-6767-2-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.20.2 (http://notmuchmail.org) Emacs/24.5.1
 (x86_64-pc-linux-gnu)
Date: Fri, 31 Jul 2015 21:11:02 +0200
Message-ID: <87d1z8f8hl.fsf@maritornes.cs.unb.ca>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

Mark Walters <markwalters1009@gmail.com> writes:

> This allows the sort to be specified in the notmuch show command with
> a --sort option.
>
> Note that individual threads are still displayed in oldest first
> order, but if the search has multiple threads then these are ordered
> according to this option. This should mean that most callers won't
> notice the option (e.g. the emacs show mode) as they only call show on
> individual threads, but other users, particularly the emacs tree view,
> can use it.
> ---
>  doc/man1/notmuch-show.rst | 17 +++++++++++++++++
>  notmuch-show.c            |  8 ++++++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/doc/man1/notmuch-show.rst b/doc/man1/notmuch-show.rst
> index 9eb5198..7717b08 100644
> --- a/doc/man1/notmuch-show.rst
> +++ b/doc/man1/notmuch-show.rst
> @@ -97,6 +97,23 @@ Supported options for **show** include
>          intended for programs that invoke **notmuch(1)** internally. If
>          omitted, the latest supported version will be used.
>  
> +    ``--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**).
> +
> +        Note: This only affects the order of messages in different
> +        threads: messages inside a thread will always be presented in
> +        thread order.

This phrasing is pretty confusing to me. What about saying something
like

This option can be used to present _threads_ in either ...

Note: this only affects the ordering of threads: messages inside a
thread will always be presented in thread order.

> However, the order of the threads will be distinct
> +        between these two options (beyond being simply reversed). When
> +        sorting by **oldest-first** the threads will be sorted by the
> +        oldest message in each thread, but when sorting by
> +        **newest-first** the threads will be sorted by the newest
> +        message in each thread.
> +
> +        By default, results will be displayed in reverse chronological
> +        order, (that is, the newest results will be displayed first).
> +
>      ``--part=N``
>          Output the single decoded MIME part N of a single message. The
>          search terms must match only a single message. Message parts are
> diff --git a/notmuch-show.c b/notmuch-show.c
> index b80933a..ec9a915 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -1090,6 +1090,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
>      int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
>      int exclude = EXCLUDE_TRUE;
>      int entire_thread = ENTIRE_THREAD_DEFAULT;
> +    notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;
>  
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
> @@ -1100,10 +1101,15 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
>  				  { "raw", NOTMUCH_FORMAT_RAW },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
> +	{ NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',
> +	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
> +				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
> +				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
>  	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
>  				  { "false", EXCLUDE_FALSE },
>  				  { 0, 0 } } },
> +
looks like extra whitespace here
>  	{ NOTMUCH_OPT_KEYWORD, &entire_thread, "entire-thread", 't',
>  	  (notmuch_keyword_t []){ { "true", ENTIRE_THREAD_TRUE },
>  				  { "false", ENTIRE_THREAD_FALSE },
> @@ -1233,6 +1239,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
>  	size_t search_exclude_tags_length;
>  	unsigned int i;
>  
> +	notmuch_query_set_sort (query, sort);
> +

hard to argue with that part ;)