Inconsistent output from "notmuch search --output=<foo>"

Inconsistent output from "notmuch search --output=<foo>"

[Carl Worth]

I just committed a bug fix for the missing final newline from "notmuch
search --output=tags". I don't recall who reported the bug, but thanks!

Michal, in trying to add a test for that bug, I found that the current
test suite infrastructure can't catch it because the shell's $()
construct doesn't distinguish whether that final newline is present or
not. I don't see an easy way to fix this, (other than making all tests
put results into files and making test_expect_equal accept those
filenames). Do you see any easy fix?

Meanwhile, while adding the --output= test, I noticed some inconsistency
in the output:

	$ notmuch search --output=threads ... | head -1
        thread:0000000000000c3c

        $ notmuch search --output=messages ... | head -1
        id:1272355278.3878.111.camel@thor.local

        $ notmuch search --output=files ... | head -1
	/path/to/maildir/1272355352.M909256P19063V18F0_0,S=9415

	$ notmuch search --output=tags ... | head -1
	attachment

The inconsistency is the presence of the "thread:" and "id:" prefixes in
the first two cases, (note that there isn't any "tag:" prefix in the
last case). I can't find any good justification for these.

I think the right answer is to drop those prefixes in the output. Does
anybody disagree? Does anyone have any scripts that are already
consuming the output of "notmuch search --output=threads" or "notmuch
search --output=messages" yet?

Note that the --format=json output won't be affected by the change I'm
proposing here.

-Carl

--
carl.d.worth@intel.com

Re: Inconsistent output from "notmuch search --output=<foo>"

[Jameson Rollins]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On Tue, 23 Nov 2010 18:09:03 -0800, Carl Worth <cworth@cworth.org> wrote:
> I think the right answer is to drop those prefixes in the output. Does
> anybody disagree? Does anyone have any scripts that are already
> consuming the output of "notmuch search --output=threads" or "notmuch
> search --output=messages" yet?

Hey, Carl.  I feel like those output options are most useful for
constructing new search strings.  If the prefixes were removed, one
would have to add them back when creating search strings from the
output.  I don't appear to currently be using any of the prefixed output
in scripts, but I have definitely used it to copy and paste threads and
ids for searches.  I guess I sort of think of this output as parallel to
the emacs show-stash-map.

I'm sure I could get used to a change, though.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: Inconsistent output from "notmuch search --output=<foo>"

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 447 bytes --]

On Tue, 23 Nov 2010 18:09:03 -0800, Carl Worth wrote:
> The inconsistency is the presence of the "thread:" and "id:" prefixes in
> the first two cases, (note that there isn't any "tag:" prefix in the
> last case). I can't find any good justification for these.

I use the output of notmuch search --format=threads to feed to another
notmuch tag operation, and if the thread: prefix vanishes, I'll have to find other
ways to do that. :)

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Inconsistent output from "notmuch search --output=<foo>"

[Michal Sojka]

On Wed, 24 Nov 2010, Carl Worth wrote:
> Michal, in trying to add a test for that bug, I found that the current
> test suite infrastructure can't catch it because the shell's $()
> construct doesn't distinguish whether that final newline is present or
> not. I don't see an easy way to fix this, (other than making all tests
> put results into files and making test_expect_equal accept those
> filenames). Do you see any easy fix?

Unfortunately no. I did the same in
id:"1289384870-15199-1-git-send-email-sojkam1@fel.cvut.cz"

In fact I find much easier writing
  cmd > output
  cmd2 >> output
than
  output=$(cmd)
  output+=$(cmd)

-Michal

Re: Inconsistent output from "notmuch search --output=<foo>"

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Wed, 24 Nov 2010 09:38:46 +0100, Sebastian Spaeth <Sebastian@SSpaeth.de> wrote:
> On Tue, 23 Nov 2010 18:09:03 -0800, Carl Worth wrote:
> > The inconsistency is the presence of the "thread:" and "id:" prefixes in
> > the first two cases, (note that there isn't any "tag:" prefix in the
> > last case). I can't find any good justification for these.
> 
> I use the output of notmuch search --format=threads to feed to another
> notmuch tag operation, and if the thread: prefix vanishes, I'll have to find other
> ways to do that. :)

You can trivially insert the 'thread:' prefix if it gets removed.

dme.
-- 
David Edmondson, http://dme.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Inconsistent output from "notmuch search --output=<foo>"

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

On Wed, 24 Nov 2010 09:38:46 +0100, Sebastian Spaeth <Sebastian@SSpaeth.de> wrote:
> On Tue, 23 Nov 2010 18:09:03 -0800, Carl Worth wrote:
> > The inconsistency is the presence of the "thread:" and "id:" prefixes in
> > the first two cases, (note that there isn't any "tag:" prefix in the
> > last case). I can't find any good justification for these.
> 
> I use the output of notmuch search --format=threads to feed to another
> notmuch tag operation, and if the thread: prefix vanishes, I'll have to find other
> ways to do that. :)

Right. The inconsistency is that we don't have output such as:

	tag:attachment
	tag:unread
        ...

From notmuch search --output=tags. While that output would be useful if
you were using the tags to construct a search string, it gets in the way
if you are doing something else with the tag names.

And I can't come up with a strong, objective distinction for the threads
and messages output differing here. (Thread and message IDs are "mostly"
used for searching? How could we define that?)

It is easy to say that a command like "search --output=" is designed
primarily for automated use in scripts, and usage like that does benefit
From consistency. This is in contrast to the various "stash" commands in
the emacs interface which are designed primarily for interactive use,
and there, convenience is more important than consistency.

So my inclination is to remove the prefixes and then recommend that you
do:

	notmuch search --output=threads | sed -e 's/^/thread:'

in your script.

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Inconsistent output from "notmuch search --output=<foo>"

[Jameson Rollins]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

On Wed, 24 Nov 2010 11:40:46 -0800, Carl Worth <cworth@cworth.org> wrote:
> So my inclination is to remove the prefixes and then recommend that you
> do:
> 
> 	notmuch search --output=threads | sed -e 's/^/thread:'
> 
> in your script.

Hey, Carl.  I think that's reasonable.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: Inconsistent output from "notmuch search --output=<foo>"

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

On Tue, 23 Nov 2010 18:09:03 -0800, Carl Worth <cworth@cworth.org> wrote:
> Meanwhile, while adding the --output= test, I noticed some inconsistency
> in the output:

>         $ notmuch search --output=messages ... | head -1
>         id:1272355278.3878.111.camel@thor.local

> The inconsistency is the presence of the "thread:" and "id:" prefixes
> [...] I can't find any good justification for these.

> I think the right answer is to drop those prefixes in the output.

I know I had spoken in favor of these prefixes before, but I discovered
that I find them annoying. I often stash message id's for pasting them
to git-send-email's --in-reply-to option and I always have to manually
remove the id: prefix.

So if you are still interested in getting rid of it, go for it :-)

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] remove prefixes from `--output={threads,messages}' results

[Pieter Praet]

Alter `do_search_threads()' and `do_search_messages()'
to not prepend each result with `thread:' respectively `id:'.

This makes its output consistent with `do_search_tags()'.

See this discussion: id:"871v6b79s0.fsf@yoom.home.cworth.org"

########################################################################

NOTE:

I started adjusting the tests as well, but stopped fairly quickly since
the amount is simply *ludicrous*. Only 9 tests fail initially, but start
ripping out `notmuch_search_sanitize' (as would be sensible) and you'll
see what I mean.

Time isn't the issue (sed flies through it faster than Windoze can BSOD),
but the patch will be so huge and disruptive that even Ubuntu devs might
consider frowning disapprovingly.

########################################################################

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 notmuch-search.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index faccaf7..c565ae6 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -220,7 +220,7 @@ do_search_threads (const search_format_t *format,
 	thread = notmuch_threads_get (threads);
 
 	if (output == OUTPUT_THREADS) {
-	    format->item_id (thread, "thread:",
+	    format->item_id (thread, "",
 			     notmuch_thread_get_thread_id (thread));
 	} else { /* output == OUTPUT_SUMMARY */
 	    fputs (format->item_start, stdout);
@@ -312,7 +312,7 @@ do_search_messages (const search_format_t *format,
 	    if (! first_message)
 		fputs (format->item_sep, stdout);
 
-	    format->item_id (message, "id:",
+	    format->item_id (message, "",
 			     notmuch_message_get_message_id (message));
 	    first_message = 0;
 	}
-- 
1.7.4.1

[PATCH] add `tag:' prefix to `--output=tags' results

[Pieter Praet]

Alter `do_search_tags()' to prepend each result with `tag:',
and update affected test.

This makes its output consistent with `do_search_threads()' and
`do_search_messages()'.

See this discussion: id:"871v6b79s0.fsf@yoom.home.cworth.org"

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 notmuch-search.c   |    2 +-
 test/search-output |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index faccaf7..5b91a40 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -364,7 +364,7 @@ do_search_tags (notmuch_database_t *notmuch,
 	if (! first_tag)
 	    fputs (format->item_sep, stdout);
 
-	format->item_id (tags, "", tag);
+	format->item_id (tags, "tag:", tag);
 
 	first_tag = 0;
     }
diff --git a/test/search-output b/test/search-output
index 10291c3..37f1b3d 100755
--- a/test/search-output
+++ b/test/search-output
@@ -289,10 +289,10 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "--output=tags"
 notmuch search --output=tags '*' >OUTPUT
 cat <<EOF >EXPECTED
-attachment
-inbox
-signed
-unread
+tag:attachment
+tag:inbox
+tag:signed
+tag:unread
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-- 
1.7.4.1

[PATCH] emacs: add keybind and function to stash Message-ID without prefix

[Pieter Praet]

Add function `notmuch-show-stash-message-id-stripped'
which stashes a Message-ID after ripping off the prefix and quotes,
add bind it to "I" key in `notmuch-show-stash-map'.

Simplifying `notmuch-show-get-message-id' instead might seem better,
but that would require concat'ing in 9 places instead of 1.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 emacs/notmuch-show.el |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6685717..a703732 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -858,6 +858,7 @@ function is used. "
     (define-key map "F" 'notmuch-show-stash-filename)
     (define-key map "f" 'notmuch-show-stash-from)
     (define-key map "i" 'notmuch-show-stash-message-id)
+    (define-key map "I" 'notmuch-show-stash-message-id-stripped)
     (define-key map "s" 'notmuch-show-stash-subject)
     (define-key map "T" 'notmuch-show-stash-tags)
     (define-key map "t" 'notmuch-show-stash-to)
@@ -1408,6 +1409,11 @@ buffer."
   (interactive)
   (notmuch-common-do-stash (notmuch-show-get-message-id)))
 
+(defun notmuch-show-stash-message-id-stripped ()
+  "Copy message ID of current message (sans `id:' prefix) to kill-ring."
+  (interactive)
+  (notmuch-common-do-stash (substring (notmuch-show-get-message-id) 4 -1)))
+
 (defun notmuch-show-stash-subject ()
   "Copy Subject field of current message to kill-ring."
   (interactive)
-- 
1.7.4.1

[PATCH] test: stashing in notmuch-{show,search}

[Pieter Praet]

Should provide full test coverage of the stashing feature.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs                                |   41 +++++++++++++++++++++++++++++
 test/emacs.expected-output/emacs-stashing |    9 ++++++
 2 files changed, 50 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/emacs-stashing

diff --git a/test/emacs b/test/emacs
index 53f455a..46076c6 100755
--- a/test/emacs
+++ b/test/emacs
@@ -347,4 +347,45 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 	    (test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
+test_begin_subtest "Stashing in notmuch-show"
+add_message '[date]="Sat, 01 Jan 2000 12:00:00 -0000"' \
+    '[from]="Some One <someone@somewhere.org>"' \
+    '[to]="Some One Else <notsomeone@somewhere.org>"' \
+    '[cc]="Notmuch <notmuch@notmuchmail.org>"' \
+    '[subject]="Stash my stashables"' \
+    '[id]="bought"' \
+    '[body]="Unable to stash body. Where did you get it in the first place?!?"'
+notmuch tag +stashtest id:${gen_msg_id}
+test_emacs '(notmuch-show "id:\"bought\"")
+        (notmuch-show-stash-date)
+        (notmuch-show-stash-from)
+        (notmuch-show-stash-to)
+        (notmuch-show-stash-cc)
+        (notmuch-show-stash-subject)
+        (notmuch-show-stash-message-id)
+        (notmuch-show-stash-message-id-stripped)
+        (notmuch-show-stash-tags)
+        (notmuch-show-stash-filename)
+        (switch-to-buffer
+          (generate-new-buffer "*test-stashing*"))
+        (dotimes (i 9)
+          (yank)
+          (insert "\n")
+          (rotate-yank-pointer 1))
+        (reverse-region (point-min) (point-max))
+	    (test-output)'
+sed -i -e 's/^.*tmp.emacs\/mail.*$/FILENAME/' OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/emacs-stashing
+
+test_begin_subtest "Stashing in notmuch-search"
+test_emacs '(notmuch-search "id:\"bought\"")
+        (notmuch-test-wait)
+        (notmuch-search-stash-thread-id)
+        (switch-to-buffer
+          (generate-new-buffer "*test-stashing*"))
+        (yank)
+	    (test-output)'
+sed -i -e 's/^thread:.*$/thread:XXX/' OUTPUT
+test_expect_equal $(cat OUTPUT) "thread:XXX"
+
 test_done
diff --git a/test/emacs.expected-output/emacs-stashing b/test/emacs.expected-output/emacs-stashing
new file mode 100644
index 0000000..4923594
--- /dev/null
+++ b/test/emacs.expected-output/emacs-stashing
@@ -0,0 +1,9 @@
+Sat, 01 Jan 2000 12:00:00 -0000
+Some One <someone@somewhere.org>
+Some One Else <notsomeone@somewhere.org>
+Notmuch <notmuch@notmuchmail.org>
+Stash my stashables
+id:"bought"
+bought
+inbox,stashtest
+FILENAME
-- 
1.7.4.1

[PATCH] fix breakage in `notmuch-hello-generate-tag-alist' due to `tag:' prefix

[Pieter Praet]

Even though all tests passed, a previous patch [1] seems to have broken
`notmuch-hello-generate-tag-alist', because the latter expects prefix-less tags.

This is a quick'n'dirty patch, thus probably not fit for consumption.
But it Works(TM).

[1] id:"1309422029-22924-1-git-send-email-pieter@praet.org"

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 emacs/notmuch-hello.el |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 65fde75..4551be1 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -343,6 +343,7 @@ Complete list of currently available key bindings:
   (notmuch-remove-if-not
    #'cdr
    (mapcar (lambda (tag)
+	     (let ((tag (substring tag 4)))
 	     (cons tag
 		   (cond
 		    ((functionp notmuch-hello-tag-list-make-query)
@@ -351,10 +352,11 @@ Complete list of currently available key bindings:
 		    ((stringp notmuch-hello-tag-list-make-query)
 		     (concat "tag:" tag " and ("
 			     notmuch-hello-tag-list-make-query ")"))
-		    (t (concat "tag:" tag)))))
+		    (t (concat "tag:" tag))))))
 	   (notmuch-remove-if-not
 	    (lambda (tag)
-	      (not (member tag notmuch-hello-hide-tags)))
+	      (let ((tag (substring tag 4)))
+	      (not (member tag notmuch-hello-hide-tags))))
 	    (process-lines notmuch-command "search-tags")))))
 
 ;;;###autoload
-- 
1.7.4.1

[PATCH] fix breakage in `notmuch-select-tag-with-completion' due to `tag:' prefix

[Pieter Praet]

Even though all tests passed, a previous patch [1] seems to have broken
`notmuch-select-tag-with-completion', because the latter expects prefix-less tags.

This is a quick'n'dirty patch, thus probably not fit for consumption.
But it Works(TM).

[1] id:"1309422029-22924-1-git-send-email-pieter@praet.org"

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 emacs/notmuch.el |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f11ec24..af66510 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -77,7 +77,11 @@ For example:
 	 (with-output-to-string
 	   (with-current-buffer standard-output
 	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
-    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
+    (completing-read prompt
+		     (mapcar (lambda (tag)
+			       (substring tag 4))
+			     (split-string tag-list "\n+" t))
+		     nil nil nil)))
 
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
-- 
1.7.4.1

Re: [PATCH] remove prefixes from `--output={threads, messages}' results

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 850 bytes --]

On Thu, 30 Jun 2011 10:19:49 +0200, Pieter Praet <pieter@praet.org> wrote:
> Alter `do_search_threads()' and `do_search_messages()'
> to not prepend each result with `thread:' respectively `id:'.

My one concern here is that I've sometimes had a message-id without the
id prefix and run a command like this:

    notmuch search 1309421989-22410-1-git-send-email-pieter@praet.org

And I've gotten confused when I've received no output, (didn't I receive
that mail? what happened?).

But I think this is a separate bug where the right fix is to make any
search terms with no prefix search through *all* prefixed terms
generated from email content. This would allow us to also avoid indexing
some content twice, (currently we store subject, from, and to both with
a prefix and without a prefix).

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] remove prefixes from `--output={threads, messages}' results

[Pieter Praet]

On Thu, 30 Jun 2011 09:24:12 -0700, Carl Worth <cworth@cworth.org> wrote:
Non-text part: multipart/signed
> On Thu, 30 Jun 2011 10:19:49 +0200, Pieter Praet <pieter@praet.org> wrote:
> > Alter `do_search_threads()' and `do_search_messages()'
> > to not prepend each result with `thread:' respectively `id:'.
> 
> My one concern here is that I've sometimes had a message-id without the
> id prefix and run a command like this:
> 
>     notmuch search 1309421989-22410-1-git-send-email-pieter@praet.org
> 
> And I've gotten confused when I've received no output, (didn't I receive
> that mail? what happened?).

And unfortunately, that's not the only concern.

While this patch removes a mere 10 chars, it has (way too) far-reaching
consequences, not only for the test suite, but for the entire codebase.

Truth be told, this was never intended to be merged. I only sent this
patch to give a ~fair chance to what appears to be the general
consensus, but I much prefer the fix for do_search_tags() [1].

In fact, all the followup patches (the seemingly haphazard reply nesting
is intentional) are intended to be used with the do_search_tags() fix,
which I tentatively consider a much cleaner path.

> But I think this is a separate bug where the right fix is to make any
> search terms with no prefix search through *all* prefixed terms
> generated from email content. This would allow us to also avoid indexing
> some content twice, (currently we store subject, from, and to both with
> a prefix and without a prefix).

Indeed a most desirable (albeit perhaps long-term) goal.

Much as I'd like, I won't be able to make myself useful in that area for
the time being, as the only C I consider myself sufficiently capable of
pushing around is the one ending with "offee" :<

> -Carl
> 
> -- 
> carl.d.worth@intel.com
Non-text part: application/pgp-signature

Peace

-- 
Pieter

[1] id:"1309422029-22924-1-git-send-email-pieter@praet.org"

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[David Bremner]

On Thu, 30 Jun 2011 10:20:29 +0200, Pieter Praet <pieter@praet.org> wrote:
> Alter `do_search_tags()' to prepend each result with `tag:',
> and update affected test.
> 
> This makes its output consistent with `do_search_threads()' and
> `do_search_messages()'.

What do people think about this change? Personally I will have change
some scripts to not add "tag:", but it isn't that big of a deal.

d

Re: [PATCH] test: stashing in notmuch-{show,search}

[David Bremner]

On Thu, 30 Jun 2011 10:23:23 +0200, Pieter Praet <pieter@praet.org> wrote:
> Should provide full test coverage of the stashing feature.
> 

I (finally) pushed this pair of patches.

d

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[Jani Nikula]

On Sat, 12 Nov 2011 10:17:30 -0500, David Bremner <david@tethera.net> wrote:
> On Thu, 30 Jun 2011 10:20:29 +0200, Pieter Praet <pieter@praet.org> wrote:
> > Alter `do_search_tags()' to prepend each result with `tag:',
> > and update affected test.
> > 
> > This makes its output consistent with `do_search_threads()' and
> > `do_search_messages()'.
> 
> What do people think about this change? Personally I will have change
> some scripts to not add "tag:", but it isn't that big of a deal.

I'm curious why this change is needed in the first place. What is gained
from this in addition to consistency? It seems I don't have enough list
history to find the referenced discussion.

The command line interface is an API, and this change causes regressions
in all scripts and programs using it, including the emacs ui. (And that
should probably be fixed with something other than "...a quick'n'dirty
patch, thus probably not fit for consumption.")

BR,
Jani.

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On Sat, 12 Nov 2011 10:17:30 -0500, David Bremner <david@tethera.net> wrote:
> What do people think about this change? Personally I will have change
> some scripts to not add "tag:", but it isn't that big of a deal.

I would actually prefer to see the prefixes removed from the messages
and threads output rather than see it added to the tags output.  I think
that's the way to make the output most consistent.  When I ask for
--output=messages I'm asking for the message ids of the messages, not
for search terms for the messages.  I think it should be up to the
consumer to add the prefix if they would like to construct search terms
based on the output.

My 2 cents.  I would be happy to provide a patch to make that change if
people agree to that behavior.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[Dmitry Kurochkin]

On Sun, 13 Nov 2011 15:00:56 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sat, 12 Nov 2011 10:17:30 -0500, David Bremner <david@tethera.net> wrote:
> > What do people think about this change? Personally I will have change
> > some scripts to not add "tag:", but it isn't that big of a deal.
> 
> I would actually prefer to see the prefixes removed from the messages
> and threads output rather than see it added to the tags output.  I think
> that's the way to make the output most consistent.  When I ask for
> --output=messages I'm asking for the message ids of the messages, not
> for search terms for the messages.  I think it should be up to the
> consumer to add the prefix if they would like to construct search terms
> based on the output.
> 

Makes sense to me.

Regards,
  Dmitry

> My 2 cents.  I would be happy to provide a patch to make that change if
> people agree to that behavior.
> 
> jamie.
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[Pieter Praet]

On Sun, 13 Nov 2011 16:14:35 +0200, Jani Nikula <jani@nikula.org> wrote:
> On Sat, 12 Nov 2011 10:17:30 -0500, David Bremner <david@tethera.net> wrote:
> > On Thu, 30 Jun 2011 10:20:29 +0200, Pieter Praet <pieter@praet.org> wrote:
> > > Alter `do_search_tags()' to prepend each result with `tag:',
> > > and update affected test.
> > > 
> > > This makes its output consistent with `do_search_threads()' and
> > > `do_search_messages()'.
> > 
> > What do people think about this change? Personally I will have change
> > some scripts to not add "tag:", but it isn't that big of a deal.
> 
> I'm curious why this change is needed in the first place. What is gained
> from this in addition to consistency? It seems I don't have enough list
> history to find the referenced discussion.
> 

Here's the original thread:
  http://comments.gmane.org/gmane.mail.notmuch.general/3562

> The command line interface is an API, and this change causes regressions
> in all scripts and programs using it, including the emacs ui. [...]

Indeed.  There's something to be said for consistency, but a change like
this is likely to cause more problems than it will solve.

> [...] (And that
> should probably be fixed with something other than "...a quick'n'dirty
> patch, thus probably not fit for consumption.")
> 

Couldn't agree more.  I wrongly assumed that the test suite would
sufficiently cover such fundamental functionality, and upon realizing
that it didn't, I quickly submitted some half-baked fixes for the most
glaringly obvious breakage, right before abandoning the whole deal.

> BR,
> Jani.


Peace

-- 
Pieter

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[Pieter Praet]

On Sun, 13 Nov 2011 15:00:56 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sat, 12 Nov 2011 10:17:30 -0500, David Bremner <david@tethera.net> wrote:
> > What do people think about this change? Personally I will have change
> > some scripts to not add "tag:", but it isn't that big of a deal.
> 
> I would actually prefer to see the prefixes removed from the messages
> and threads output rather than see it added to the tags output.  I think
> that's the way to make the output most consistent.  When I ask for
> --output=messages I'm asking for the message ids of the messages, not
> for search terms for the messages.  I think it should be up to the
> consumer to add the prefix if they would like to construct search terms
> based on the output.
> 
> My 2 cents.  I would be happy to provide a patch to make that change if
> people agree to that behavior.
> 

I agree 100%.  Your rationale is as rational as they come, and the
change itself is really simple (patch available [1]), *but* do consider
the amount of tests (alot!) and third-party programs/scripts that would
need an update.

> jamie.


Peace

-- 
Pieter

[1] id:"1309421989-22410-1-git-send-email-pieter@praet.org"

[PATCH] test: emacs: tidy up "Stashing in notmuch-show" test

[Pieter Praet]

Merge expected output into the actual test, so we can verify the stashed
filename using ${gen_msg_filename} instead of doing sed tricks.
---

Thanks David!

Here's a small update that was hanging out on one of my local branches.


Peace

 test/emacs                                |   14 ++++++++++++--
 test/emacs.expected-output/emacs-stashing |    9 ---------
 2 files changed, 12 insertions(+), 11 deletions(-)
 delete mode 100644 test/emacs.expected-output/emacs-stashing

diff --git a/test/emacs b/test/emacs
index 75a0a74..3bf2e29 100755
--- a/test/emacs
+++ b/test/emacs
@@ -369,8 +369,18 @@ test_emacs '(notmuch-show "id:\"bought\"")
           (rotate-yank-pointer 1))
         (reverse-region (point-min) (point-max))
 	    (test-output)'
-sed -i -e 's/^.*tmp.emacs\/mail.*$/FILENAME/' OUTPUT
-test_expect_equal_file OUTPUT $EXPECTED/emacs-stashing
+cat <<EOF >EXPECTED
+Sat, 01 Jan 2000 12:00:00 -0000
+Some One <someone@somewhere.org>
+Some One Else <notsomeone@somewhere.org>
+Notmuch <notmuch@notmuchmail.org>
+Stash my stashables
+id:"bought"
+bought
+inbox,stashtest
+${gen_msg_filename}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Stashing in notmuch-search"
 test_emacs '(notmuch-search "id:\"bought\"")
diff --git a/test/emacs.expected-output/emacs-stashing b/test/emacs.expected-output/emacs-stashing
deleted file mode 100644
index 4923594..0000000
--- a/test/emacs.expected-output/emacs-stashing
+++ /dev/null
@@ -1,9 +0,0 @@
-Sat, 01 Jan 2000 12:00:00 -0000
-Some One <someone@somewhere.org>
-Some One Else <notsomeone@somewhere.org>
-Notmuch <notmuch@notmuchmail.org>
-Stash my stashables
-id:"bought"
-bought
-inbox,stashtest
-FILENAME
-- 
1.7.7.1

Re: [PATCH] test: emacs: tidy up "Stashing in notmuch-show" test

[David Bremner]

On Wed, 16 Nov 2011 12:38:19 +0100, Pieter Praet <pieter@praet.org> wrote:
> Merge expected output into the actual test, so we can verify the stashed
> filename using ${gen_msg_filename} instead of doing sed tricks.

pushed to master

d

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 98 bytes --]

Given the discussion that followed this patch I'd like to mark it as
'obsolete'.

Any objections?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] add `tag:' prefix to `--output=tags' results

[Pieter Praet]

On Mon, 16 Jan 2012 10:56:03 +0000, David Edmondson <dme@dme.org> wrote:
> Given the discussion that followed this patch I'd like to mark it as
> 'obsolete'.
> 
> Any objections?

None whatsoever :)


Peace

-- 
Pieter