On Sun, Aug 09 2015, David Bremner <da...@tethera.net> wrote:

> We need some way to extract the uuid/revision of the database, and
> count seems like the least bad choice of current commands.
> The (perhaps weak) argument for count over search is that count
> already reports statistics about the entire database.

The '(perhaps weak)' part could be discussed outside of the commit message ;)
... but read below for (IMO) "real" issues...

> ---
>  notmuch-count.c                | 18 +++++++++++++++++-
>  test/T570-revision-tracking.sh | 12 ++++++++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/notmuch-count.c b/notmuch-count.c
> index 57a88a8..7c61ccb 100644
> --- a/notmuch-count.c
> +++ b/notmuch-count.c
> @@ -25,6 +25,7 @@ enum {
>      OUTPUT_THREADS,
>      OUTPUT_MESSAGES,
>      OUTPUT_FILES,
> +    OUTPUT_LASTMOD,
>  };
>  
>  /* The following is to allow future options to be added more easily */
> @@ -71,6 +72,9 @@ print_count (notmuch_database_t *notmuch, const char 
> *query_str,
>  {
>      notmuch_query_t *query;
>      size_t i;
> +    unsigned long revision;
> +    const char *uuid;
> +    int ret = 0;
>  
>      query = notmuch_query_create (notmuch, query_str);
>      if (query == NULL) {
> @@ -91,11 +95,22 @@ print_count (notmuch_database_t *notmuch, const char 
> *query_str,
>      case OUTPUT_FILES:
>       printf ("%u\n", count_files (query));
>       break;
> +    case OUTPUT_LASTMOD:
> +     if (strcmp (notmuch_query_get_query_string (query), "*") != 0) {
> +         fprintf (stderr, "Error: Only '*' is currently supported "
> +                  " with output=modifications\n");
> +         ret = 1;
> +         goto DONE;
> +     }

I have three comments on this series, the first one is hardest, second
trivial and one could be either way.

> +
> +     revision = notmuch_database_get_revision (notmuch, &uuid);
> +     printf ("%s\t%lu\n", uuid, revision);

Currently, `notmuch count` outputs lines that contain just one integer;
this changes this by introducing output with uuid ([0-9a-f-]) and integer
delimited by tab character.

To put it lightly, this looks "inconsistent" and don't please my aesthetic
eye.

One option (being it worse or better) could be that by default only
lastmod value is printed and with separate option it is prefixed with
database UUID (in every --output option).

BTW: I did not see notmuch-count.rst updated. good for now as it reduces
the amount of work during this bikeshed period ;D

continuing in next email...

Tomi



>      }
>  
> + DONE:
>      notmuch_query_destroy (query);
>  
> -    return 0;
> +    return ret;
>  }
>  
>  static int
> @@ -139,6 +154,7 @@ notmuch_count_command (notmuch_config_t *config, int 
> argc, char *argv[])
>         (notmuch_keyword_t []){ { "threads", OUTPUT_THREADS },
>                                 { "messages", OUTPUT_MESSAGES },
>                                 { "files", OUTPUT_FILES },
> +                               { "modifications", OUTPUT_LASTMOD },
>                                 { 0, 0 } } },
>       { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
>         (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
> diff --git a/test/T570-revision-tracking.sh b/test/T570-revision-tracking.sh
> index e0a5703..008c5d0 100755
> --- a/test/T570-revision-tracking.sh
> +++ b/test/T570-revision-tracking.sh
> @@ -34,4 +34,16 @@ UUID       53
>  EOF
>  test_expect_equal_file EXPECTED CLEAN
>  
> +grep '^[0-9a-f]' OUTPUT > INITIAL_OUTPUT
> +
> +test_begin_subtest "output of count matches test code"
> +notmuch count --output=modifications '*' > OUTPUT
> +test_expect_equal_file INITIAL_OUTPUT OUTPUT
> +
> +test_begin_subtest "modification count increases"
> +before=$(notmuch count --output=modifications '*' | cut -f2)
> +notmuch tag +a-random-tag-8743632 '*'
> +after=$(notmuch count --output=modifications '*' | cut -f2)
> +result=$(($before < $after))
> +test_expect_equal 1 ${result}
>  test_done
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Reply via email to