bc/d67f00967eb21a714f3cf193679b0d9410976d

   1 Return-Path: <tomi.ollila@iki.fi>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5  by arlo.cworth.org (Postfix) with ESMTP id 51A726DE1579\r
   6  for <notmuch@notmuchmail.org>; Mon, 10 Aug 2015 12:28:24 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at cworth.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 1.246\r
  10 X-Spam-Level: *\r
  11 X-Spam-Status: No, score=1.246 tagged_above=-999 required=5 tests=[AWL=-0.150,\r
  12   SPF_NEUTRAL=0.652, URIBL_SBL=0.644, URIBL_SBL_A=0.1] autolearn=disabled\r
  13 Received: from arlo.cworth.org ([127.0.0.1])\r
  14  by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15  with ESMTP id TULiCnjWko46 for <notmuch@notmuchmail.org>;\r
  16  Mon, 10 Aug 2015 12:28:21 -0700 (PDT)\r
  17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
  18  by arlo.cworth.org (Postfix) with ESMTP id 355AC6DE0244\r
  19  for <notmuch@notmuchmail.org>; Mon, 10 Aug 2015 12:28:20 -0700 (PDT)\r
  20 Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
  21  by guru.guru-group.fi (Postfix) with ESMTP id E1FEB1000CE;\r
  22  Mon, 10 Aug 2015 22:28:05 +0300 (EEST)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org\r
  25 Subject: Re: [PATCH 3/5] cli/count: add --output=modifications\r
  26 In-Reply-To: <1439112285-6681-4-git-send-email-david@tethera.net>\r
  27 References: <1439112285-6681-1-git-send-email-david@tethera.net>\r
  28  <1439112285-6681-4-git-send-email-david@tethera.net>\r
  29 User-Agent: Notmuch/0.20.2+58~ge9cd033 (http://notmuchmail.org) Emacs/24.3.1\r
  30  (x86_64-unknown-linux-gnu)\r
  31 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  32  $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  33  !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  34 Date: Mon, 10 Aug 2015 22:28:05 +0300\r
  35 Message-ID: <m21tfb54fu.fsf@guru.guru-group.fi>\r
  36 MIME-Version: 1.0\r
  37 Content-Type: text/plain\r
  38 X-BeenThere: notmuch@notmuchmail.org\r
  39 X-Mailman-Version: 2.1.18\r
  40 Precedence: list\r
  41 List-Id: "Use and development of the notmuch mail system."\r
  42  <notmuch.notmuchmail.org>\r
  43 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  44  <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  45 List-Archive: <http://notmuchmail.org/pipermail/notmuch/>\r
  46 List-Post: <mailto:notmuch@notmuchmail.org>\r
  47 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  48 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  49  <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  50 X-List-Received-Date: Mon, 10 Aug 2015 19:28:24 -0000\r
  51 \r
  52 On Sun, Aug 09 2015, David Bremner <david@tethera.net> wrote:\r
  53 \r
  54 > We need some way to extract the uuid/revision of the database, and\r
  55 > count seems like the least bad choice of current commands.\r
  56 > The (perhaps weak) argument for count over search is that count\r
  57 > already reports statistics about the entire database.\r
  58 \r
  59 The '(perhaps weak)' part could be discussed outside of the commit message ;)\r
  60 ... but read below for (IMO) "real" issues...\r
  61 \r
  62 > ---\r
  63 >  notmuch-count.c                | 18 +++++++++++++++++-\r
  64 >  test/T570-revision-tracking.sh | 12 ++++++++++++\r
  65 >  2 files changed, 29 insertions(+), 1 deletion(-)\r
  66 >\r
  67 > diff --git a/notmuch-count.c b/notmuch-count.c\r
  68 > index 57a88a8..7c61ccb 100644\r
  69 > --- a/notmuch-count.c\r
  70 > +++ b/notmuch-count.c\r
  71 > @@ -25,6 +25,7 @@ enum {\r
  72 >      OUTPUT_THREADS,\r
  73 >      OUTPUT_MESSAGES,\r
  74 >      OUTPUT_FILES,\r
  75 > +    OUTPUT_LASTMOD,\r
  76 >  };\r
  77 >  \r
  78 >  /* The following is to allow future options to be added more easily */\r
  79 > @@ -71,6 +72,9 @@ print_count (notmuch_database_t *notmuch, const char *query_str,\r
  80 >  {\r
  81 >      notmuch_query_t *query;\r
  82 >      size_t i;\r
  83 > +    unsigned long revision;\r
  84 > +    const char *uuid;\r
  85 > +    int ret = 0;\r
  86 >  \r
  87 >      query = notmuch_query_create (notmuch, query_str);\r
  88 >      if (query == NULL) {\r
  89 > @@ -91,11 +95,22 @@ print_count (notmuch_database_t *notmuch, const char *query_str,\r
  90 >      case OUTPUT_FILES:\r
  91 >       printf ("%u\n", count_files (query));\r
  92 >       break;\r
  93 > +    case OUTPUT_LASTMOD:\r
  94 > +     if (strcmp (notmuch_query_get_query_string (query), "*") != 0) {\r
  95 > +         fprintf (stderr, "Error: Only '*' is currently supported "\r
  96 > +                  " with output=modifications\n");\r
  97 > +         ret = 1;\r
  98 > +         goto DONE;\r
  99 > +     }\r
 100 \r
 101 I have three comments on this series, the first one is hardest, second\r
 102 trivial and one could be either way.\r
 103 \r
 104 > +\r
 105 > +     revision = notmuch_database_get_revision (notmuch, &uuid);\r
 106 > +     printf ("%s\t%lu\n", uuid, revision);\r
 107 \r
 108 Currently, `notmuch count` outputs lines that contain just one integer;\r
 109 this changes this by introducing output with uuid ([0-9a-f-]) and integer\r
 110 delimited by tab character.\r
 111 \r
 112 To put it lightly, this looks "inconsistent" and don't please my aesthetic\r
 113 eye.\r
 114 \r
 115 One option (being it worse or better) could be that by default only\r
 116 lastmod value is printed and with separate option it is prefixed with\r
 117 database UUID (in every --output option).\r
 118 \r
 119 BTW: I did not see notmuch-count.rst updated. good for now as it reduces\r
 120 the amount of work during this bikeshed period ;D\r
 121 \r
 122 continuing in next email...\r
 123 \r
 124 Tomi\r
 125 \r
 126 \r
 127 \r
 128 >      }\r
 129 >  \r
 130 > + DONE:\r
 131 >      notmuch_query_destroy (query);\r
 132 >  \r
 133 > -    return 0;\r
 134 > +    return ret;\r
 135 >  }\r
 136 >  \r
 137 >  static int\r
 138 > @@ -139,6 +154,7 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])\r
 139 >         (notmuch_keyword_t []){ { "threads", OUTPUT_THREADS },\r
 140 >                                 { "messages", OUTPUT_MESSAGES },\r
 141 >                                 { "files", OUTPUT_FILES },\r
 142 > +                               { "modifications", OUTPUT_LASTMOD },\r
 143 >                                 { 0, 0 } } },\r
 144 >       { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',\r
 145 >         (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },\r
 146 > diff --git a/test/T570-revision-tracking.sh b/test/T570-revision-tracking.sh\r
 147 > index e0a5703..008c5d0 100755\r
 148 > --- a/test/T570-revision-tracking.sh\r
 149 > +++ b/test/T570-revision-tracking.sh\r
 150 > @@ -34,4 +34,16 @@ UUID       53\r
 151 >  EOF\r
 152 >  test_expect_equal_file EXPECTED CLEAN\r
 153 >  \r
 154 > +grep '^[0-9a-f]' OUTPUT > INITIAL_OUTPUT\r
 155 > +\r
 156 > +test_begin_subtest "output of count matches test code"\r
 157 > +notmuch count --output=modifications '*' > OUTPUT\r
 158 > +test_expect_equal_file INITIAL_OUTPUT OUTPUT\r
 159 > +\r
 160 > +test_begin_subtest "modification count increases"\r
 161 > +before=$(notmuch count --output=modifications '*' | cut -f2)\r
 162 > +notmuch tag +a-random-tag-8743632 '*'\r
 163 > +after=$(notmuch count --output=modifications '*' | cut -f2)\r
 164 > +result=$(($before < $after))\r
 165 > +test_expect_equal 1 ${result}\r
 166 >  test_done\r
 167 > -- \r
 168 > 2.1.4\r
 169 >\r
 170 > _______________________________________________\r
 171 > notmuch mailing list\r
 172 > notmuch@notmuchmail.org\r
 173 > http://notmuchmail.org/mailman/listinfo/notmuch\r