unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Inconsistent output from "notmuch search --output=<foo>"
@ 2010-11-24  2:09 Carl Worth
  2010-11-24  3:30 ` Jameson Rollins
                   ` (3 more replies)
  0 siblings, 4 replies; 27+ messages in thread
From: Carl Worth @ 2010-11-24  2:09 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Inconsistent output from "notmuch search --output=<foo>"
  2010-11-24  2:09 Inconsistent output from "notmuch search --output=<foo>" Carl Worth
@ 2010-11-24  3:30 ` Jameson Rollins
  2010-11-24  8:38 ` Sebastian Spaeth
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 27+ messages in thread
From: Jameson Rollins @ 2010-11-24  3:30 UTC (permalink / raw)
  To: Carl Worth, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Inconsistent output from "notmuch search --output=<foo>"
  2010-11-24  2:09 Inconsistent output from "notmuch search --output=<foo>" Carl Worth
  2010-11-24  3:30 ` Jameson Rollins
@ 2010-11-24  8:38 ` Sebastian Spaeth
  2010-11-24 10:36   ` David Edmondson
  2010-11-24 19:40   ` Carl Worth
  2010-11-24  9:25 ` Michal Sojka
  2011-03-29  0:39 ` Sebastian Spaeth
  3 siblings, 2 replies; 27+ messages in thread
From: Sebastian Spaeth @ 2010-11-24  8:38 UTC (permalink / raw)
  To: Carl Worth, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Inconsistent output from "notmuch search --output=<foo>"
  2010-11-24  2:09 Inconsistent output from "notmuch search --output=<foo>" Carl Worth
  2010-11-24  3:30 ` Jameson Rollins
  2010-11-24  8:38 ` Sebastian Spaeth
@ 2010-11-24  9:25 ` Michal Sojka
  2011-03-29  0:39 ` Sebastian Spaeth
  3 siblings, 0 replies; 27+ messages in thread
From: Michal Sojka @ 2010-11-24  9:25 UTC (permalink / raw)
  To: Carl Worth, notmuch

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Inconsistent output from "notmuch search --output=<foo>"
  2010-11-24  8:38 ` Sebastian Spaeth
@ 2010-11-24 10:36   ` David Edmondson
  2010-11-24 19:40   ` Carl Worth
  1 sibling, 0 replies; 27+ messages in thread
From: David Edmondson @ 2010-11-24 10:36 UTC (permalink / raw)
  To: Sebastian Spaeth, Carl Worth, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Inconsistent output from "notmuch search --output=<foo>"
  2010-11-24  8:38 ` Sebastian Spaeth
  2010-11-24 10:36   ` David Edmondson
@ 2010-11-24 19:40   ` Carl Worth
  2010-11-25 15:41     ` Jameson Rollins
  1 sibling, 1 reply; 27+ messages in thread
From: Carl Worth @ 2010-11-24 19:40 UTC (permalink / raw)
  To: Sebastian Spaeth, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Inconsistent output from "notmuch search --output=<foo>"
  2010-11-24 19:40   ` Carl Worth
@ 2010-11-25 15:41     ` Jameson Rollins
  0 siblings, 0 replies; 27+ messages in thread
From: Jameson Rollins @ 2010-11-25 15:41 UTC (permalink / raw)
  To: Carl Worth, Sebastian Spaeth, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Inconsistent output from "notmuch search --output=<foo>"
  2010-11-24  2:09 Inconsistent output from "notmuch search --output=<foo>" Carl Worth
                   ` (2 preceding siblings ...)
  2010-11-24  9:25 ` Michal Sojka
@ 2011-03-29  0:39 ` Sebastian Spaeth
  2011-06-30  8:19   ` [PATCH] remove prefixes from `--output={threads,messages}' results Pieter Praet
                     ` (2 more replies)
  3 siblings, 3 replies; 27+ messages in thread
From: Sebastian Spaeth @ 2011-03-29  0:39 UTC (permalink / raw)
  To: Carl Worth, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] remove prefixes from `--output={threads,messages}' results
  2011-03-29  0:39 ` Sebastian Spaeth
@ 2011-06-30  8:19   ` Pieter Praet
  2011-06-30 16:24     ` [PATCH] remove prefixes from `--output={threads, messages}' results Carl Worth
  2011-06-30  8:20   ` [PATCH] add `tag:' prefix to `--output=tags' results Pieter Praet
  2011-06-30  8:20   ` [PATCH] emacs: add keybind and function to stash Message-ID without prefix Pieter Praet
  2 siblings, 1 reply; 27+ messages in thread
From: Pieter Praet @ 2011-06-30  8:19 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH] add `tag:' prefix to `--output=tags' results
  2011-03-29  0:39 ` Sebastian Spaeth
  2011-06-30  8:19   ` [PATCH] remove prefixes from `--output={threads,messages}' results Pieter Praet
@ 2011-06-30  8:20   ` Pieter Praet
  2011-06-30  8:35     ` [PATCH] fix breakage in `notmuch-hello-generate-tag-alist' due to `tag:' prefix Pieter Praet
                       ` (3 more replies)
  2011-06-30  8:20   ` [PATCH] emacs: add keybind and function to stash Message-ID without prefix Pieter Praet
  2 siblings, 4 replies; 27+ messages in thread
From: Pieter Praet @ 2011-06-30  8:20 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH] emacs: add keybind and function to stash Message-ID without prefix
  2011-03-29  0:39 ` Sebastian Spaeth
  2011-06-30  8:19   ` [PATCH] remove prefixes from `--output={threads,messages}' results Pieter Praet
  2011-06-30  8:20   ` [PATCH] add `tag:' prefix to `--output=tags' results Pieter Praet
@ 2011-06-30  8:20   ` Pieter Praet
  2011-06-30  8:23     ` [PATCH] test: stashing in notmuch-{show,search} Pieter Praet
  2 siblings, 1 reply; 27+ messages in thread
From: Pieter Praet @ 2011-06-30  8:20 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH] test: stashing in notmuch-{show,search}
  2011-06-30  8:20   ` [PATCH] emacs: add keybind and function to stash Message-ID without prefix Pieter Praet
@ 2011-06-30  8:23     ` Pieter Praet
  2011-11-13  1:12       ` David Bremner
  0 siblings, 1 reply; 27+ messages in thread
From: Pieter Praet @ 2011-06-30  8:23 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH] fix breakage in `notmuch-hello-generate-tag-alist' due to `tag:' prefix
  2011-06-30  8:20   ` [PATCH] add `tag:' prefix to `--output=tags' results Pieter Praet
@ 2011-06-30  8:35     ` Pieter Praet
  2011-06-30  8:36     ` [PATCH] fix breakage in `notmuch-select-tag-with-completion' " Pieter Praet
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 27+ messages in thread
From: Pieter Praet @ 2011-06-30  8:35 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH] fix breakage in `notmuch-select-tag-with-completion' due to `tag:' prefix
  2011-06-30  8:20   ` [PATCH] add `tag:' prefix to `--output=tags' results Pieter Praet
  2011-06-30  8:35     ` [PATCH] fix breakage in `notmuch-hello-generate-tag-alist' due to `tag:' prefix Pieter Praet
@ 2011-06-30  8:36     ` Pieter Praet
  2011-11-12 15:17     ` [PATCH] add `tag:' prefix to `--output=tags' results David Bremner
  2012-01-16 10:56     ` David Edmondson
  3 siblings, 0 replies; 27+ messages in thread
From: Pieter Praet @ 2011-06-30  8:36 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH] remove prefixes from `--output={threads, messages}' results
  2011-06-30  8:19   ` [PATCH] remove prefixes from `--output={threads,messages}' results Pieter Praet
@ 2011-06-30 16:24     ` Carl Worth
  2011-06-30 17:54       ` Pieter Praet
  0 siblings, 1 reply; 27+ messages in thread
From: Carl Worth @ 2011-06-30 16:24 UTC (permalink / raw)
  To: Pieter Praet, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] remove prefixes from `--output={threads, messages}' results
  2011-06-30 16:24     ` [PATCH] remove prefixes from `--output={threads, messages}' results Carl Worth
@ 2011-06-30 17:54       ` Pieter Praet
  0 siblings, 0 replies; 27+ messages in thread
From: Pieter Praet @ 2011-06-30 17:54 UTC (permalink / raw)
  To: Carl Worth, notmuch

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"

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2011-06-30  8:20   ` [PATCH] add `tag:' prefix to `--output=tags' results Pieter Praet
  2011-06-30  8:35     ` [PATCH] fix breakage in `notmuch-hello-generate-tag-alist' due to `tag:' prefix Pieter Praet
  2011-06-30  8:36     ` [PATCH] fix breakage in `notmuch-select-tag-with-completion' " Pieter Praet
@ 2011-11-12 15:17     ` David Bremner
  2011-11-13 14:14       ` Jani Nikula
  2011-11-13 23:00       ` Jameson Graef Rollins
  2012-01-16 10:56     ` David Edmondson
  3 siblings, 2 replies; 27+ messages in thread
From: David Bremner @ 2011-11-12 15:17 UTC (permalink / raw)
  To: Pieter Praet, notmuch

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] test: stashing in notmuch-{show,search}
  2011-06-30  8:23     ` [PATCH] test: stashing in notmuch-{show,search} Pieter Praet
@ 2011-11-13  1:12       ` David Bremner
  2011-11-16 11:38         ` [PATCH] test: emacs: tidy up "Stashing in notmuch-show" test Pieter Praet
  0 siblings, 1 reply; 27+ messages in thread
From: David Bremner @ 2011-11-13  1:12 UTC (permalink / raw)
  To: Pieter Praet, notmuch

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2011-11-12 15:17     ` [PATCH] add `tag:' prefix to `--output=tags' results David Bremner
@ 2011-11-13 14:14       ` Jani Nikula
  2011-11-16 11:07         ` Pieter Praet
  2011-11-13 23:00       ` Jameson Graef Rollins
  1 sibling, 1 reply; 27+ messages in thread
From: Jani Nikula @ 2011-11-13 14:14 UTC (permalink / raw)
  To: David Bremner, Pieter Praet, notmuch

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.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2011-11-12 15:17     ` [PATCH] add `tag:' prefix to `--output=tags' results David Bremner
  2011-11-13 14:14       ` Jani Nikula
@ 2011-11-13 23:00       ` Jameson Graef Rollins
  2011-11-14  6:56         ` Dmitry Kurochkin
  2011-11-16 11:28         ` Pieter Praet
  1 sibling, 2 replies; 27+ messages in thread
From: Jameson Graef Rollins @ 2011-11-13 23:00 UTC (permalink / raw)
  To: David Bremner, Pieter Praet, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2011-11-13 23:00       ` Jameson Graef Rollins
@ 2011-11-14  6:56         ` Dmitry Kurochkin
  2011-11-16 11:28         ` Pieter Praet
  1 sibling, 0 replies; 27+ messages in thread
From: Dmitry Kurochkin @ 2011-11-14  6:56 UTC (permalink / raw)
  To: Jameson Graef Rollins, David Bremner, Pieter Praet, notmuch

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2011-11-13 14:14       ` Jani Nikula
@ 2011-11-16 11:07         ` Pieter Praet
  0 siblings, 0 replies; 27+ messages in thread
From: Pieter Praet @ 2011-11-16 11:07 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2011-11-13 23:00       ` Jameson Graef Rollins
  2011-11-14  6:56         ` Dmitry Kurochkin
@ 2011-11-16 11:28         ` Pieter Praet
  1 sibling, 0 replies; 27+ messages in thread
From: Pieter Praet @ 2011-11-16 11:28 UTC (permalink / raw)
  To: Jameson Graef Rollins, David Bremner, notmuch

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"

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] test: emacs: tidy up "Stashing in notmuch-show" test
  2011-11-13  1:12       ` David Bremner
@ 2011-11-16 11:38         ` Pieter Praet
  2011-11-18 19:26           ` David Bremner
  0 siblings, 1 reply; 27+ messages in thread
From: Pieter Praet @ 2011-11-16 11:38 UTC (permalink / raw)
  To: David Bremner; +Cc: Notmuch Mail

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH] test: emacs: tidy up "Stashing in notmuch-show" test
  2011-11-16 11:38         ` [PATCH] test: emacs: tidy up "Stashing in notmuch-show" test Pieter Praet
@ 2011-11-18 19:26           ` David Bremner
  0 siblings, 0 replies; 27+ messages in thread
From: David Bremner @ 2011-11-18 19:26 UTC (permalink / raw)
  To: Pieter Praet; +Cc: Notmuch Mail

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2011-06-30  8:20   ` [PATCH] add `tag:' prefix to `--output=tags' results Pieter Praet
                       ` (2 preceding siblings ...)
  2011-11-12 15:17     ` [PATCH] add `tag:' prefix to `--output=tags' results David Bremner
@ 2012-01-16 10:56     ` David Edmondson
  2012-01-16 10:58       ` Pieter Praet
  3 siblings, 1 reply; 27+ messages in thread
From: David Edmondson @ 2012-01-16 10:56 UTC (permalink / raw)
  To: Pieter Praet, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] add `tag:' prefix to `--output=tags' results
  2012-01-16 10:56     ` David Edmondson
@ 2012-01-16 10:58       ` Pieter Praet
  0 siblings, 0 replies; 27+ messages in thread
From: Pieter Praet @ 2012-01-16 10:58 UTC (permalink / raw)
  To: David Edmondson, notmuch

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2012-01-16 11:00 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-24  2:09 Inconsistent output from "notmuch search --output=<foo>" Carl Worth
2010-11-24  3:30 ` Jameson Rollins
2010-11-24  8:38 ` Sebastian Spaeth
2010-11-24 10:36   ` David Edmondson
2010-11-24 19:40   ` Carl Worth
2010-11-25 15:41     ` Jameson Rollins
2010-11-24  9:25 ` Michal Sojka
2011-03-29  0:39 ` Sebastian Spaeth
2011-06-30  8:19   ` [PATCH] remove prefixes from `--output={threads,messages}' results Pieter Praet
2011-06-30 16:24     ` [PATCH] remove prefixes from `--output={threads, messages}' results Carl Worth
2011-06-30 17:54       ` Pieter Praet
2011-06-30  8:20   ` [PATCH] add `tag:' prefix to `--output=tags' results Pieter Praet
2011-06-30  8:35     ` [PATCH] fix breakage in `notmuch-hello-generate-tag-alist' due to `tag:' prefix Pieter Praet
2011-06-30  8:36     ` [PATCH] fix breakage in `notmuch-select-tag-with-completion' " Pieter Praet
2011-11-12 15:17     ` [PATCH] add `tag:' prefix to `--output=tags' results David Bremner
2011-11-13 14:14       ` Jani Nikula
2011-11-16 11:07         ` Pieter Praet
2011-11-13 23:00       ` Jameson Graef Rollins
2011-11-14  6:56         ` Dmitry Kurochkin
2011-11-16 11:28         ` Pieter Praet
2012-01-16 10:56     ` David Edmondson
2012-01-16 10:58       ` Pieter Praet
2011-06-30  8:20   ` [PATCH] emacs: add keybind and function to stash Message-ID without prefix Pieter Praet
2011-06-30  8:23     ` [PATCH] test: stashing in notmuch-{show,search} Pieter Praet
2011-11-13  1:12       ` David Bremner
2011-11-16 11:38         ` [PATCH] test: emacs: tidy up "Stashing in notmuch-show" test Pieter Praet
2011-11-18 19:26           ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).