[ANN] notmuch-labeler: Improves notmuch way of displaying labels
Hi Damien. Damien Cassou writes: > Hi, > > On Fri, Oct 12, 2012 at 7:07 PM, David Bremner wrote: >> It seeems like a nice UI enchancement, and people would not have to use >> it if they didn't like it, so in principle I guess we should work >> towards integrating it into notmuch upstream. > > > does anyone else agrees? Should I spend time on that or does nobody care? > I do agree! Would be nice to have this feature integrated into notmuch. Regards, Dmitry > >> I'm not an elisp expert, but I did find it strange to use use defadvice >> on your own code. Maybe I'm just too conservative. > > > I let myself advise my own code only when no duplication is involved > and when the dependencies between the modules that contain the > function and its pieces of advice are well defined. I have no problem > using a different mechanism (such a function variable and calling > funcall) if integration into notmuch is desired but not defadvice. > > >> At some point we should probably get a patch series against notmuch >> mainline; I'll let other people comment on whether yes, now is the time >> (and thereby volunteering to review the patches ;). > > Anyone interested in notmuch-labeler? > https://github.com/DamienCassou/notmuch-labeler (there are screenshots > there) > > -- > Damien Cassou > http://damiencassou.seasidehosting.st > > "Success is the ability to go from one failure to another without > losing enthusiasm." > Winston Churchill > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [ANN] notmuch-labeler: Improves notmuch way of displaying labels
Hi Damien. Damien Cassou writes: > Hi, > > On Fri, Oct 12, 2012 at 7:07 PM, David Bremner wrote: >> It seeems like a nice UI enchancement, and people would not have to use >> it if they didn't like it, so in principle I guess we should work >> towards integrating it into notmuch upstream. > > > does anyone else agrees? Should I spend time on that or does nobody care? > I do agree! Would be nice to have this feature integrated into notmuch. Regards, Dmitry > >> I'm not an elisp expert, but I did find it strange to use use defadvice >> on your own code. Maybe I'm just too conservative. > > > I let myself advise my own code only when no duplication is involved > and when the dependencies between the modules that contain the > function and its pieces of advice are well defined. I have no problem > using a different mechanism (such a function variable and calling > funcall) if integration into notmuch is desired but not defadvice. > > >> At some point we should probably get a patch series against notmuch >> mainline; I'll let other people comment on whether yes, now is the time >> (and thereby volunteering to review the patches ;). > > Anyone interested in notmuch-labeler? > https://github.com/DamienCassou/notmuch-labeler (there are screenshots > there) > > -- > Damien Cassou > http://damiencassou.seasidehosting.st > > "Success is the ability to go from one failure to another without > losing enthusiasm." > Winston Churchill > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/3] test: Add a test for HTML email with inline images
Austin Clements writes:
> Quoth Dmitry Kurochkin on Oct 03 at 1:35 pm:
>> Hi Austin.
>>
>> Austin Clements writes:
>>
>> > Currently this test passes in Emacs 23 but fails in Emacs 24 (at least
>> > on some Linux distributions).
>>
>> The test fails for me on Emacs 23.4.1 (Debian unstable):
>>
>> FAIL Rendering HTML mail with images
>> --- emacs.51.OUTPUT 2012-10-03 09:31:33.383529764 +
>> +++ emacs.51.EXPECTED 2012-10-03 09:31:33.383529764 +
>> @@ -6,4 +6,3 @@
>> [ multipart/related ]
>> [ text/html ]
>> *
>> -
>>
>> Did not look into details.
>
> Yes. This test is (in hindsight, unsurprisingly) sensitive to
> whatever HTML renderer Emacs chooses. It looks like you're probably
> using html2text, which outputs nothing for an image. Unfortunately,
> none of the built-in renderers in Emacs 23 are aware of content
> references, which makes this test rather pointless on Emacs 23 unless
> we depend on an external renderer.
>
> The best solution I can think of dynamically chooses shr on Emacs 24
> (since that's really what we're trying to test) and gives up on Emacs
> 23 and forcibly selects html2text (test patch below). Alternatively,
> we could cycle through all of the available renderers, test everything
> that we can, and just ignore everything that we can't run, though that
> would make the test environment-sensitive.
>
Perhaps the test should be skipped if shr is not available, like we do
for missing binaries?
Regards,
Dmitry
> diff --git a/test/emacs b/test/emacs
> index 1f84b91..2ef78bf 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -756,7 +756,7 @@ add_message '[subject]="HTML mail with images"' \
> '[body]="--abcd
> Content-Type: text/html
>
> -
> + smiley
>
> --abcd
> Content-Type: image/gif
> @@ -766,10 +766,13 @@ Content-ID: <330 at goomoji.gmail>
> R0lGODlhDAAMAKIFAF5LAP/zxNyuAP/gaP///wAAACH5BAEAAAUALAAMAAwAAAMl
> WLPcGjDKFYi9lxKBOaGcF35DhWHamZUW0K4mAbiwWtuf0uxFAgA7
> --abcd--"'
> -test_emacs "(notmuch-show \"id:${gen_msg_id}\")
> +test_emacs "(let ((mm-text-html-renderer
> + (if (assq 'shr mm-text-html-renderer-alist)
> + 'shr 'html2text)))
> + (notmuch-show \"id:${gen_msg_id}\"))
> (test-output)"
> # Normalize output for Emacs 23 and Emacs 24
> -sed -i 's/\[cid\]/*/' OUTPUT
> +sed -i 's/^ smiley/* smiley/' OUTPUT
> cat <EXPECTED
> Notmuch Test Suite (2001-01-05) (inbox)
> Subject: HTML mail with images
> @@ -778,7 +781,7 @@ Date: Fri, 05 Jan 2001 15:43:57 +
>
> [ multipart/related ]
> [ text/html ]
> -*
> +* smiley
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
[PATCH 2/3] test: Add a test for HTML email with inline images
Hi Austin. Austin Clements writes: > Currently this test passes in Emacs 23 but fails in Emacs 24 (at least > on some Linux distributions). The test fails for me on Emacs 23.4.1 (Debian unstable): FAIL Rendering HTML mail with images --- emacs.51.OUTPUT 2012-10-03 09:31:33.383529764 + +++ emacs.51.EXPECTED 2012-10-03 09:31:33.383529764 + @@ -6,4 +6,3 @@ [ multipart/related ] [ text/html ] * - Did not look into details. Regards, Dmitry
Re: [PATCH 2/3] test: Add a test for HTML email with inline images
Austin Clements writes:
> Quoth Dmitry Kurochkin on Oct 03 at 1:35 pm:
>> Hi Austin.
>>
>> Austin Clements writes:
>>
>> > Currently this test passes in Emacs 23 but fails in Emacs 24 (at least
>> > on some Linux distributions).
>>
>> The test fails for me on Emacs 23.4.1 (Debian unstable):
>>
>> FAIL Rendering HTML mail with images
>> --- emacs.51.OUTPUT 2012-10-03 09:31:33.383529764 +
>> +++ emacs.51.EXPECTED 2012-10-03 09:31:33.383529764 +
>> @@ -6,4 +6,3 @@
>> [ multipart/related ]
>> [ text/html ]
>> *
>> -
>>
>> Did not look into details.
>
> Yes. This test is (in hindsight, unsurprisingly) sensitive to
> whatever HTML renderer Emacs chooses. It looks like you're probably
> using html2text, which outputs nothing for an image. Unfortunately,
> none of the built-in renderers in Emacs 23 are aware of content
> references, which makes this test rather pointless on Emacs 23 unless
> we depend on an external renderer.
>
> The best solution I can think of dynamically chooses shr on Emacs 24
> (since that's really what we're trying to test) and gives up on Emacs
> 23 and forcibly selects html2text (test patch below). Alternatively,
> we could cycle through all of the available renderers, test everything
> that we can, and just ignore everything that we can't run, though that
> would make the test environment-sensitive.
>
Perhaps the test should be skipped if shr is not available, like we do
for missing binaries?
Regards,
Dmitry
> diff --git a/test/emacs b/test/emacs
> index 1f84b91..2ef78bf 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -756,7 +756,7 @@ add_message '[subject]="HTML mail with images"' \
> '[body]="--abcd
> Content-Type: text/html
>
> -
> + smiley
>
> --abcd
> Content-Type: image/gif
> @@ -766,10 +766,13 @@ Content-ID: <330@goomoji.gmail>
> R0lGODlhDAAMAKIFAF5LAP/zxNyuAP/gaP///wAAACH5BAEAAAUALAAMAAwAAAMl
> WLPcGjDKFYi9lxKBOaGcF35DhWHamZUW0K4mAbiwWtuf0uxFAgA7
> --abcd--"'
> -test_emacs "(notmuch-show \"id:${gen_msg_id}\")
> +test_emacs "(let ((mm-text-html-renderer
> + (if (assq 'shr mm-text-html-renderer-alist)
> + 'shr 'html2text)))
> + (notmuch-show \"id:${gen_msg_id}\"))
> (test-output)"
> # Normalize output for Emacs 23 and Emacs 24
> -sed -i 's/\[cid\]/*/' OUTPUT
> +sed -i 's/^ smiley/* smiley/' OUTPUT
> cat <EXPECTED
> Notmuch Test Suite (2001-01-05) (inbox)
> Subject: HTML mail with images
> @@ -778,7 +781,7 @@ Date: Fri, 05 Jan 2001 15:43:57 +
>
> [ multipart/related ]
> [ text/html ]
> -*
> +* smiley
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/3] test: Add a test for HTML email with inline images
Hi Austin. Austin Clements writes: > Currently this test passes in Emacs 23 but fails in Emacs 24 (at least > on some Linux distributions). The test fails for me on Emacs 23.4.1 (Debian unstable): FAIL Rendering HTML mail with images --- emacs.51.OUTPUT 2012-10-03 09:31:33.383529764 + +++ emacs.51.EXPECTED 2012-10-03 09:31:33.383529764 + @@ -6,4 +6,3 @@ [ multipart/related ] [ text/html ] * - Did not look into details. Regards, Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Add notmuch-remove-duplicates.py script to contrib.
Michal Nazarewicz writes:
>>> On Tue, Sep 04 2012, Dmitry Kurochkin wrote:
>>>> +class MailComparator:
>>>> +"""Checks if mail files are duplicates."""
>>>> +def __init__(self, filename):
>>>> +self.filename = filename
>>>> +self.mail = self.readFile(self.filename)
>>>> +
>>>> +def isDuplicate(self, filename):
>>>> +return self.mail == self.readFile(filename)
>>>> +
>>>> +@staticmethod
>>>> +def readFile(filename):
>>>> +with open(filename) as f:
>>>> +data = ""
>>>> +while True:
>>>> +line = f.readline()
>>>> + for header in IGNORED_HEADERS:
>>>> +if line.startswith(header):
>
>> Michal Nazarewicz writes:
>>> Case of headers should be ignored, but this does not ignore it.
>
> On Tue, Sep 04 2012, Dmitry Kurochkin wrote:
>> It does.
>
> Wait, how? If line is ?received:? how does it starts with ?Received:??
>
Sorry, I misunderstood your comment. It does not ignore the case indeed.
>>>> +if os.path.realpath(comparator.filename) ==
>>>> os.path.realpath(filename):
>>>> +print "Message '%s' has filenames pointing to the
>>>> same file: '%s' '%s'" % (msg.get_message_id(), comparator.filename,
>>>> filename)
>>>
>>> So why aren't those removed?
>>>
>>
>> Because it is the same file indexed twice (probably because of
>> symlinks). We do not want to remove the only message file.
>
> Ah, right, with symlinks this is troublesome, but than again, we can
> check if there is at least one non-symlink. If there is, delete
> everything else, if there is not, delete all but one arbitrarily chosen
> symlink.
>
Sure, we could do that.
>>>> +elif comparator.isDuplicate(filename):
>>>> +os.remove(filename)
>>>> +duplicates_count += 1
>>>> +else:
>>>> +#print "Potential duplicates: %s" % msg.get_message_id()
>>>> +suspected_duplicates_count += 1
>>>> +
>>>> +new_timestamp = time.time()
>>>> +if new_timestamp - timestamp > 1:
>>>> +timestamp = new_timestamp
>>>> +sys.stdout.write("\rProcessed %s messages, removed %s
>>>> duplicates..." % (msg_count, duplicates_count))
>>>> +sys.stdout.flush()
>>>> +
>>>> +print "\rFinished. Processed %s messages, removed %s duplicates." %
>>>> (msg_count, duplicates_count)
>>>> +if duplicates_count > 0:
>>>> +print "You might want to run 'notmuch new' now."
>>>> +
>>>> +if suspected_duplicates_count > 0:
>>>> +print
>>>> +print "Found %s messages with duplicate IDs but different content." %
>>>> suspected_duplicates_count
>>>> +print "Perhaps we should ignore more headers."
>>>
>>> Please consider the following instead (not tested):
>
>> Thanks for reviewing my poor python code :) I am afraid I do not have
>> enough interest in improving it. I just implemented a simple solution
>> for my problem. Though it looks like you already took time to rewrite
>> the script. Would be great if you send it as a proper patch obsoleting
>> this one.
>
> Bah, I'll probably won't have time to properly test it.
>
Same problem :)
Regards,
Dmitry
> --
> Best regards, _ _
> .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
> ..o | Computer Science, Micha? ?mina86? Nazarewicz(o o)
> ooo +--ooO--(_)--Ooo--
[PATCH] Add notmuch-remove-duplicates.py script to contrib.
Hi Michal.
Michal Nazarewicz writes:
> On Tue, Sep 04 2012, Dmitry Kurochkin wrote:
>> The script removes duplicate message files. It takes no options.
>>
>> Files are assumed duplicates if their content is the same except for
>> ignored headers. Currently, the only ignored header is Received:.
>> ---
>> contrib/notmuch-remove-duplicates.py | 95
>> ++
>> 1 file changed, 95 insertions(+)
>> create mode 100755 contrib/notmuch-remove-duplicates.py
>>
>> diff --git a/contrib/notmuch-remove-duplicates.py
>> b/contrib/notmuch-remove-duplicates.py
>> new file mode 100755
>> index 000..dbe2e25
>> --- /dev/null
>> +++ b/contrib/notmuch-remove-duplicates.py
>> @@ -0,0 +1,95 @@
>> +#!/usr/bin/env python
>> +
>> +import sys
>> +
>> +IGNORED_HEADERS = [ "Received:" ]
>> +
>> +if len(sys.argv) != 1:
>> +print "Usage: %s" % sys.argv[0]
>> +print
>> +print "The script removes duplicate message files. Takes no options."
>> +print "Requires notmuch python module."
>> +print
>> +print "Files are assumed duplicates if their content is the same"
>> +print "except for the following headers: %s." % ",
>> ".join(IGNORED_HEADERS)
>> +exit(1)
>
> It's much better put inside a main() function, which is than called only
> if the script is run directly.
>
Good point. My python skill is pretty low :)
>> +
>> +import notmuch
>> +import os
>> +import time
>> +
>> +class MailComparator:
>> +"""Checks if mail files are duplicates."""
>> +def __init__(self, filename):
>> +self.filename = filename
>> +self.mail = self.readFile(self.filename)
>> +
>> +def isDuplicate(self, filename):
>> +return self.mail == self.readFile(filename)
>> +
>> +@staticmethod
>> +def readFile(filename):
>> +with open(filename) as f:
>> +data = ""
>> +while True:
>> +line = f.readline()
>> +for header in IGNORED_HEADERS:
>> +if line.startswith(header):
>
> Case of headers should be ignored, but this does not ignore it.
>
It does.
>> +# skip header continuation lines
>> +while True:
>> +line = f.readline()
>> +if len(line) == 0 or line[0] not in [" ", "\t"]:
>> +break
>> +break
>
> This will ignore line just after the ignored header.
>
The first header line is ignored as well because line is added to data
in else block.
>> +else:
>> +data += line
>> +if line == "\n":
>> +break
>> +data += f.read()
>> +return data
>> +
>> +db = notmuch.Database()
>> +query = db.create_query('*')
>> +print "Number of messages: %s" % query.count_messages()
>> +
>> +files_count = 0
>> +for root, dirs, files in os.walk(db.get_path()):
>> +if not root.startswith(os.path.join(db.get_path(), ".notmuch/")):
>> +files_count += len(files)
>> +print "Number of files: %s" % files_count
>> +print "Estimated number of duplicates: %s" % (files_count -
>> query.count_messages())
>> +
>> +msgs = query.search_messages()
>> +msg_count = 0
>> +suspected_duplicates_count = 0
>> +duplicates_count = 0
>> +timestamp = time.time()
>> +for msg in msgs:
>> +msg_count += 1
>> +if len(msg.get_filenames()) > 1:
>> +filenames = msg.get_filenames()
>> +comparator = MailComparator(filenames.next())
>> +for filename in filenames:
>
> Strictly speaking, you need to compare each file to each file, and not
> just every file to the first file.
>
>> +if os.path.realpath(comparator.filename) ==
>> os.path.realpath(filename):
>> +print "Message '%s' has filenames pointing to the
>> same file: '%s' '%s'" % (msg.get_message_id(), comparator.filename,
>> filename)
>
> So why aren't those removed?
>
Because it is the same file indexed twice (probably because of
symlinks). We do not want to remove the only message file.
>&
[PATCH] Add notmuch-remove-duplicates.py script to contrib.
The script removes duplicate message files. It takes no options.
Files are assumed duplicates if their content is the same except for
ignored headers. Currently, the only ignored header is Received:.
---
contrib/notmuch-remove-duplicates.py | 95 ++
1 file changed, 95 insertions(+)
create mode 100755 contrib/notmuch-remove-duplicates.py
diff --git a/contrib/notmuch-remove-duplicates.py
b/contrib/notmuch-remove-duplicates.py
new file mode 100755
index 000..dbe2e25
--- /dev/null
+++ b/contrib/notmuch-remove-duplicates.py
@@ -0,0 +1,95 @@
+#!/usr/bin/env python
+
+import sys
+
+IGNORED_HEADERS = [ "Received:" ]
+
+if len(sys.argv) != 1:
+print "Usage: %s" % sys.argv[0]
+print
+print "The script removes duplicate message files. Takes no options."
+print "Requires notmuch python module."
+print
+print "Files are assumed duplicates if their content is the same"
+print "except for the following headers: %s." % ", ".join(IGNORED_HEADERS)
+exit(1)
+
+import notmuch
+import os
+import time
+
+class MailComparator:
+"""Checks if mail files are duplicates."""
+def __init__(self, filename):
+self.filename = filename
+self.mail = self.readFile(self.filename)
+
+def isDuplicate(self, filename):
+return self.mail == self.readFile(filename)
+
+@staticmethod
+def readFile(filename):
+with open(filename) as f:
+data = ""
+while True:
+line = f.readline()
+for header in IGNORED_HEADERS:
+if line.startswith(header):
+# skip header continuation lines
+while True:
+line = f.readline()
+if len(line) == 0 or line[0] not in [" ", "\t"]:
+break
+break
+else:
+data += line
+if line == "\n":
+break
+data += f.read()
+return data
+
+db = notmuch.Database()
+query = db.create_query('*')
+print "Number of messages: %s" % query.count_messages()
+
+files_count = 0
+for root, dirs, files in os.walk(db.get_path()):
+if not root.startswith(os.path.join(db.get_path(), ".notmuch/")):
+files_count += len(files)
+print "Number of files: %s" % files_count
+print "Estimated number of duplicates: %s" % (files_count -
query.count_messages())
+
+msgs = query.search_messages()
+msg_count = 0
+suspected_duplicates_count = 0
+duplicates_count = 0
+timestamp = time.time()
+for msg in msgs:
+msg_count += 1
+if len(msg.get_filenames()) > 1:
+filenames = msg.get_filenames()
+comparator = MailComparator(filenames.next())
+for filename in filenames:
+if os.path.realpath(comparator.filename) ==
os.path.realpath(filename):
+print "Message '%s' has filenames pointing to the same file:
'%s' '%s'" % (msg.get_message_id(), comparator.filename, filename)
+elif comparator.isDuplicate(filename):
+os.remove(filename)
+duplicates_count += 1
+else:
+#print "Potential duplicates: %s" % msg.get_message_id()
+suspected_duplicates_count += 1
+
+new_timestamp = time.time()
+if new_timestamp - timestamp > 1:
+timestamp = new_timestamp
+sys.stdout.write("\rProcessed %s messages, removed %s duplicates..." %
(msg_count, duplicates_count))
+sys.stdout.flush()
+
+print "\rFinished. Processed %s messages, removed %s duplicates." %
(msg_count, duplicates_count)
+if duplicates_count > 0:
+print "You might want to run 'notmuch new' now."
+
+if suspected_duplicates_count > 0:
+print
+print "Found %s messages with duplicate IDs but different content." %
suspected_duplicates_count
+print "Perhaps we should ignore more headers."
--
1.7.10.4
Re: [PATCH] Add notmuch-remove-duplicates.py script to contrib.
Michal Nazarewicz writes:
>>> On Tue, Sep 04 2012, Dmitry Kurochkin wrote:
>>>> +class MailComparator:
>>>> +"""Checks if mail files are duplicates."""
>>>> +def __init__(self, filename):
>>>> +self.filename = filename
>>>> +self.mail = self.readFile(self.filename)
>>>> +
>>>> +def isDuplicate(self, filename):
>>>> +return self.mail == self.readFile(filename)
>>>> +
>>>> +@staticmethod
>>>> +def readFile(filename):
>>>> +with open(filename) as f:
>>>> +data = ""
>>>> +while True:
>>>> +line = f.readline()
>>>> + for header in IGNORED_HEADERS:
>>>> +if line.startswith(header):
>
>> Michal Nazarewicz writes:
>>> Case of headers should be ignored, but this does not ignore it.
>
> On Tue, Sep 04 2012, Dmitry Kurochkin wrote:
>> It does.
>
> Wait, how? If line is “received:” how does it starts with “Received:”?
>
Sorry, I misunderstood your comment. It does not ignore the case indeed.
>>>> +if os.path.realpath(comparator.filename) ==
>>>> os.path.realpath(filename):
>>>> +print "Message '%s' has filenames pointing to the
>>>> same file: '%s' '%s'" % (msg.get_message_id(), comparator.filename,
>>>> filename)
>>>
>>> So why aren't those removed?
>>>
>>
>> Because it is the same file indexed twice (probably because of
>> symlinks). We do not want to remove the only message file.
>
> Ah, right, with symlinks this is troublesome, but than again, we can
> check if there is at least one non-symlink. If there is, delete
> everything else, if there is not, delete all but one arbitrarily chosen
> symlink.
>
Sure, we could do that.
>>>> +elif comparator.isDuplicate(filename):
>>>> +os.remove(filename)
>>>> +duplicates_count += 1
>>>> +else:
>>>> +#print "Potential duplicates: %s" % msg.get_message_id()
>>>> +suspected_duplicates_count += 1
>>>> +
>>>> +new_timestamp = time.time()
>>>> +if new_timestamp - timestamp > 1:
>>>> +timestamp = new_timestamp
>>>> +sys.stdout.write("\rProcessed %s messages, removed %s
>>>> duplicates..." % (msg_count, duplicates_count))
>>>> +sys.stdout.flush()
>>>> +
>>>> +print "\rFinished. Processed %s messages, removed %s duplicates." %
>>>> (msg_count, duplicates_count)
>>>> +if duplicates_count > 0:
>>>> +print "You might want to run 'notmuch new' now."
>>>> +
>>>> +if suspected_duplicates_count > 0:
>>>> +print
>>>> +print "Found %s messages with duplicate IDs but different content." %
>>>> suspected_duplicates_count
>>>> +print "Perhaps we should ignore more headers."
>>>
>>> Please consider the following instead (not tested):
>
>> Thanks for reviewing my poor python code :) I am afraid I do not have
>> enough interest in improving it. I just implemented a simple solution
>> for my problem. Though it looks like you already took time to rewrite
>> the script. Would be great if you send it as a proper patch obsoleting
>> this one.
>
> Bah, I'll probably won't have time to properly test it.
>
Same problem :)
Regards,
Dmitry
> --
> Best regards, _ _
> .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
> ..o | Computer Science, Michał “mina86” Nazarewicz(o o)
> ooo +--ooO--(_)--Ooo--
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Add notmuch-remove-duplicates.py script to contrib.
Hi Michal.
Michal Nazarewicz writes:
> On Tue, Sep 04 2012, Dmitry Kurochkin wrote:
>> The script removes duplicate message files. It takes no options.
>>
>> Files are assumed duplicates if their content is the same except for
>> ignored headers. Currently, the only ignored header is Received:.
>> ---
>> contrib/notmuch-remove-duplicates.py | 95
>> ++
>> 1 file changed, 95 insertions(+)
>> create mode 100755 contrib/notmuch-remove-duplicates.py
>>
>> diff --git a/contrib/notmuch-remove-duplicates.py
>> b/contrib/notmuch-remove-duplicates.py
>> new file mode 100755
>> index 000..dbe2e25
>> --- /dev/null
>> +++ b/contrib/notmuch-remove-duplicates.py
>> @@ -0,0 +1,95 @@
>> +#!/usr/bin/env python
>> +
>> +import sys
>> +
>> +IGNORED_HEADERS = [ "Received:" ]
>> +
>> +if len(sys.argv) != 1:
>> +print "Usage: %s" % sys.argv[0]
>> +print
>> +print "The script removes duplicate message files. Takes no options."
>> +print "Requires notmuch python module."
>> +print
>> +print "Files are assumed duplicates if their content is the same"
>> +print "except for the following headers: %s." % ",
>> ".join(IGNORED_HEADERS)
>> +exit(1)
>
> It's much better put inside a main() function, which is than called only
> if the script is run directly.
>
Good point. My python skill is pretty low :)
>> +
>> +import notmuch
>> +import os
>> +import time
>> +
>> +class MailComparator:
>> +"""Checks if mail files are duplicates."""
>> +def __init__(self, filename):
>> +self.filename = filename
>> +self.mail = self.readFile(self.filename)
>> +
>> +def isDuplicate(self, filename):
>> +return self.mail == self.readFile(filename)
>> +
>> +@staticmethod
>> +def readFile(filename):
>> +with open(filename) as f:
>> +data = ""
>> +while True:
>> +line = f.readline()
>> +for header in IGNORED_HEADERS:
>> +if line.startswith(header):
>
> Case of headers should be ignored, but this does not ignore it.
>
It does.
>> +# skip header continuation lines
>> +while True:
>> +line = f.readline()
>> +if len(line) == 0 or line[0] not in [" ", "\t"]:
>> +break
>> +break
>
> This will ignore line just after the ignored header.
>
The first header line is ignored as well because line is added to data
in else block.
>> +else:
>> +data += line
>> +if line == "\n":
>> +break
>> +data += f.read()
>> +return data
>> +
>> +db = notmuch.Database()
>> +query = db.create_query('*')
>> +print "Number of messages: %s" % query.count_messages()
>> +
>> +files_count = 0
>> +for root, dirs, files in os.walk(db.get_path()):
>> +if not root.startswith(os.path.join(db.get_path(), ".notmuch/")):
>> +files_count += len(files)
>> +print "Number of files: %s" % files_count
>> +print "Estimated number of duplicates: %s" % (files_count -
>> query.count_messages())
>> +
>> +msgs = query.search_messages()
>> +msg_count = 0
>> +suspected_duplicates_count = 0
>> +duplicates_count = 0
>> +timestamp = time.time()
>> +for msg in msgs:
>> +msg_count += 1
>> +if len(msg.get_filenames()) > 1:
>> +filenames = msg.get_filenames()
>> +comparator = MailComparator(filenames.next())
>> +for filename in filenames:
>
> Strictly speaking, you need to compare each file to each file, and not
> just every file to the first file.
>
>> +if os.path.realpath(comparator.filename) ==
>> os.path.realpath(filename):
>> +print "Message '%s' has filenames pointing to the
>> same file: '%s' '%s'" % (msg.get_message_id(), comparator.filename,
>> filename)
>
> So why aren't those removed?
>
Because it is the same file indexed twice (probably because of
symlinks). We do not want to remove the only message file.
>&
[PATCH] Add notmuch-remove-duplicates.py script to contrib.
The script removes duplicate message files. It takes no options.
Files are assumed duplicates if their content is the same except for
ignored headers. Currently, the only ignored header is Received:.
---
contrib/notmuch-remove-duplicates.py | 95 ++
1 file changed, 95 insertions(+)
create mode 100755 contrib/notmuch-remove-duplicates.py
diff --git a/contrib/notmuch-remove-duplicates.py
b/contrib/notmuch-remove-duplicates.py
new file mode 100755
index 000..dbe2e25
--- /dev/null
+++ b/contrib/notmuch-remove-duplicates.py
@@ -0,0 +1,95 @@
+#!/usr/bin/env python
+
+import sys
+
+IGNORED_HEADERS = [ "Received:" ]
+
+if len(sys.argv) != 1:
+print "Usage: %s" % sys.argv[0]
+print
+print "The script removes duplicate message files. Takes no options."
+print "Requires notmuch python module."
+print
+print "Files are assumed duplicates if their content is the same"
+print "except for the following headers: %s." % ", ".join(IGNORED_HEADERS)
+exit(1)
+
+import notmuch
+import os
+import time
+
+class MailComparator:
+"""Checks if mail files are duplicates."""
+def __init__(self, filename):
+self.filename = filename
+self.mail = self.readFile(self.filename)
+
+def isDuplicate(self, filename):
+return self.mail == self.readFile(filename)
+
+@staticmethod
+def readFile(filename):
+with open(filename) as f:
+data = ""
+while True:
+line = f.readline()
+for header in IGNORED_HEADERS:
+if line.startswith(header):
+# skip header continuation lines
+while True:
+line = f.readline()
+if len(line) == 0 or line[0] not in [" ", "\t"]:
+break
+break
+else:
+data += line
+if line == "\n":
+break
+data += f.read()
+return data
+
+db = notmuch.Database()
+query = db.create_query('*')
+print "Number of messages: %s" % query.count_messages()
+
+files_count = 0
+for root, dirs, files in os.walk(db.get_path()):
+if not root.startswith(os.path.join(db.get_path(), ".notmuch/")):
+files_count += len(files)
+print "Number of files: %s" % files_count
+print "Estimated number of duplicates: %s" % (files_count -
query.count_messages())
+
+msgs = query.search_messages()
+msg_count = 0
+suspected_duplicates_count = 0
+duplicates_count = 0
+timestamp = time.time()
+for msg in msgs:
+msg_count += 1
+if len(msg.get_filenames()) > 1:
+filenames = msg.get_filenames()
+comparator = MailComparator(filenames.next())
+for filename in filenames:
+if os.path.realpath(comparator.filename) ==
os.path.realpath(filename):
+print "Message '%s' has filenames pointing to the same file:
'%s' '%s'" % (msg.get_message_id(), comparator.filename, filename)
+elif comparator.isDuplicate(filename):
+os.remove(filename)
+duplicates_count += 1
+else:
+#print "Potential duplicates: %s" % msg.get_message_id()
+suspected_duplicates_count += 1
+
+new_timestamp = time.time()
+if new_timestamp - timestamp > 1:
+timestamp = new_timestamp
+sys.stdout.write("\rProcessed %s messages, removed %s duplicates..." %
(msg_count, duplicates_count))
+sys.stdout.flush()
+
+print "\rFinished. Processed %s messages, removed %s duplicates." %
(msg_count, duplicates_count)
+if duplicates_count > 0:
+print "You might want to run 'notmuch new' now."
+
+if suspected_duplicates_count > 0:
+print
+print "Found %s messages with duplicate IDs but different content." %
suspected_duplicates_count
+print "Perhaps we should ignore more headers."
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] cli: notmuch-show with framing newlines between threads in JSON.
Hi Mark.
Mark Walters writes:
> Add newlines between complete threads to make asynchronous parsing
> of the JSON easier.
> ---
>
> notmuch-pick uses the JSON output of notmuch show but, in many cases,
> for many threads. This can take quite a long time when displaying a
> large number of messages (say 20 seconds for the 10,000 messages in
> the notmuch archive). Thus it is desirable to display results
> incrementally in the same way that search currently does.
>
> To make this easier this patch adds newlines between each toplevel
> thread. So the ouput becomes
>
> [
> thread1
> , thread2
> , thread3
> ...
> , last_thread
> ]
>
> Thus the parser can easily tell if it has enough data to do some more
> parsing.
>
> Obviously, this changes the JSON output. This should not break any
> consumer as the JSON parsers should not mind. However, it does break
> several tests.
I think this should be part of the commit message since it explains the
rationale behind the change.
Regards,
Dmitry
> Obviously, I will fix these but I wanted to check if
> people were basically happy with the change first.
>
> Also, should devel/schemata be updated? It seems a little unclear as
> this is not really a "JSON" change as the JSON does not care about the
> newlines.
>
> Best wishes
>
> Mark
>
>
> notmuch-show.c |5 +
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 195e318..4a1d699 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -942,6 +942,8 @@ do_show (void *ctx,
>
> if (format->message_set_start)
> fputs (format->message_set_start, stdout);
> +if (format == &format_json)
> + fputs ("\n", stdout);
>
> for (threads = notmuch_query_search_threads (query);
>notmuch_threads_valid (threads);
> @@ -963,6 +965,9 @@ do_show (void *ctx,
> if (status && !res)
> res = status;
>
> + if (format == &format_json)
> + fputs ("\n", stdout);
> +
> notmuch_thread_destroy (thread);
>
> }
> --
> 1.7.9.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] cli: notmuch-show with framing newlines between threads in JSON.
Hi Mark.
Mark Walters writes:
> Add newlines between complete threads to make asynchronous parsing
> of the JSON easier.
> ---
>
> notmuch-pick uses the JSON output of notmuch show but, in many cases,
> for many threads. This can take quite a long time when displaying a
> large number of messages (say 20 seconds for the 10,000 messages in
> the notmuch archive). Thus it is desirable to display results
> incrementally in the same way that search currently does.
>
> To make this easier this patch adds newlines between each toplevel
> thread. So the ouput becomes
>
> [
> thread1
> , thread2
> , thread3
> ...
> , last_thread
> ]
>
> Thus the parser can easily tell if it has enough data to do some more
> parsing.
>
> Obviously, this changes the JSON output. This should not break any
> consumer as the JSON parsers should not mind. However, it does break
> several tests.
I think this should be part of the commit message since it explains the
rationale behind the change.
Regards,
Dmitry
> Obviously, I will fix these but I wanted to check if
> people were basically happy with the change first.
>
> Also, should devel/schemata be updated? It seems a little unclear as
> this is not really a "JSON" change as the JSON does not care about the
> newlines.
>
> Best wishes
>
> Mark
>
>
> notmuch-show.c |5 +
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 195e318..4a1d699 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -942,6 +942,8 @@ do_show (void *ctx,
>
> if (format->message_set_start)
> fputs (format->message_set_start, stdout);
> +if (format == &format_json)
> + fputs ("\n", stdout);
>
> for (threads = notmuch_query_search_threads (query);
>notmuch_threads_valid (threads);
> @@ -963,6 +965,9 @@ do_show (void *ctx,
> if (status && !res)
> res = status;
>
> + if (format == &format_json)
> + fputs ("\n", stdout);
> +
> notmuch_thread_destroy (thread);
>
> }
> --
> 1.7.9.1
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
FreeBSD Support Patches
Mike Kelly writes: > On Fri, May 25, 2012 at 10:15 AM, Dmitry Kurochkin > wrote: >> Why do we need to explicitly declare Emacs dependency for tests? ?There >> should be no need for it. ?We have "implicit" dependencies which are >> declared once (see test_declare_external_prereq calls at the end of >> test-lib.sh) and are automatically handled when a test tries to use a >> missing binary. ?Explicit dependencies are hard to maintain (e.g. your >> patch adds explicit emacs dependency for crypto test but misses gpg). >> With rare exceptions we should not use explicit dependencies. > > Because not every test actually has those implicit dependencies. For > example, some of the crypto tests depend upon emacs_deliver_message > working correctly for subsequents tests. Those emacs_deliver_message > tests are skipped, but not the ones after it that try to do something > with that injected message. > > For the emacs-* test files, there are some tests that act the same > way. These subtests do not directly depend on Emacs. They depend on other subtests. Currently, we do not support such dependencies. But what you propose is not the solution. We have two options here: make all subtests independent or introduce proper subtests dependencies. The former might require many changes to existing tests and may be hard to enforce. The latter is not trivial as well but is doable. I planned to implement subtests dependencies but never really got to it (and I do not think I will anytime soon). > However, it is also a minor speed improvement to say that, > obviously, none of the emacs tests are going to work, so just don't > bother. > I do not think maintaining an explicit list of dependencies worth a minor speed improvement. Given all above, I understand that your patches fix a common problem in a simple way. And it does not look like we would get proper solution anytime soon. So I am ok with these patches with two comments: * Provide a proper commit message to explain the issue in more detail. * Add an XXX comment for each explicit dependency, something like: // XXX: Workaround for subtests that depend on other subtests (and, // hence, indirectly depend on emacs). Should be removed when we // have proper subtests dependencies. Regards, Dmitry > -- > Mike Kelly
FreeBSD Support Patches
Hi Mike. Mike Kelly writes: > Hi, > > These patches add some changes necessary for a clean build on FreeBSD, > and for most of the tests to pass. > > Also mixed in are a few patches to disable emacs tests when you don't > have emacs, along with tests that depend upon emacs. I could split those > off onto a separate branch if required. > Why do we need to explicitly declare Emacs dependency for tests? There should be no need for it. We have "implicit" dependencies which are declared once (see test_declare_external_prereq calls at the end of test-lib.sh) and are automatically handled when a test tries to use a missing binary. Explicit dependencies are hard to maintain (e.g. your patch adds explicit emacs dependency for crypto test but misses gpg). With rare exceptions we should not use explicit dependencies. Regards, Dmitry > You can find the latest version of these patches on my github repo's > fbsd-support branch: > > https://github.com/pioto/notmuch/compare/master...fbsd-support > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: FreeBSD Support Patches
Mike Kelly writes: > On Fri, May 25, 2012 at 10:15 AM, Dmitry Kurochkin > wrote: >> Why do we need to explicitly declare Emacs dependency for tests? There >> should be no need for it. We have "implicit" dependencies which are >> declared once (see test_declare_external_prereq calls at the end of >> test-lib.sh) and are automatically handled when a test tries to use a >> missing binary. Explicit dependencies are hard to maintain (e.g. your >> patch adds explicit emacs dependency for crypto test but misses gpg). >> With rare exceptions we should not use explicit dependencies. > > Because not every test actually has those implicit dependencies. For > example, some of the crypto tests depend upon emacs_deliver_message > working correctly for subsequents tests. Those emacs_deliver_message > tests are skipped, but not the ones after it that try to do something > with that injected message. > > For the emacs-* test files, there are some tests that act the same > way. These subtests do not directly depend on Emacs. They depend on other subtests. Currently, we do not support such dependencies. But what you propose is not the solution. We have two options here: make all subtests independent or introduce proper subtests dependencies. The former might require many changes to existing tests and may be hard to enforce. The latter is not trivial as well but is doable. I planned to implement subtests dependencies but never really got to it (and I do not think I will anytime soon). > However, it is also a minor speed improvement to say that, > obviously, none of the emacs tests are going to work, so just don't > bother. > I do not think maintaining an explicit list of dependencies worth a minor speed improvement. Given all above, I understand that your patches fix a common problem in a simple way. And it does not look like we would get proper solution anytime soon. So I am ok with these patches with two comments: * Provide a proper commit message to explain the issue in more detail. * Add an XXX comment for each explicit dependency, something like: // XXX: Workaround for subtests that depend on other subtests (and, // hence, indirectly depend on emacs). Should be removed when we // have proper subtests dependencies. Regards, Dmitry > -- > Mike Kelly ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: remove "Testing" from test description in emacs-hello and emacs-show
"Testing" is printed by test/test-lib.sh, so having "Testing" in test description results in duplicate "Testing" in console output. --- test/emacs-hello |2 +- test/emacs-show |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/emacs-hello b/test/emacs-hello index a998dc4..48d1420 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -1,6 +1,6 @@ #!/usr/bin/env bash -test_description="Testing emacs notmuch-hello view" +test_description="emacs notmuch-hello view" . test-lib.sh EXPECTED=$TEST_DIRECTORY/emacs.expected-output diff --git a/test/emacs-show b/test/emacs-show index 5700d2e..2498564 100755 --- a/test/emacs-show +++ b/test/emacs-show @@ -1,6 +1,6 @@ #!/usr/bin/env bash -test_description="Testing emacs notmuch-show view" +test_description="emacs notmuch-show view" . test-lib.sh test_begin_subtest "Hiding Original Message region at beginning of a message" -- 1.7.10
Re: FreeBSD Support Patches
Hi Mike. Mike Kelly writes: > Hi, > > These patches add some changes necessary for a clean build on FreeBSD, > and for most of the tests to pass. > > Also mixed in are a few patches to disable emacs tests when you don't > have emacs, along with tests that depend upon emacs. I could split those > off onto a separate branch if required. > Why do we need to explicitly declare Emacs dependency for tests? There should be no need for it. We have "implicit" dependencies which are declared once (see test_declare_external_prereq calls at the end of test-lib.sh) and are automatically handled when a test tries to use a missing binary. Explicit dependencies are hard to maintain (e.g. your patch adds explicit emacs dependency for crypto test but misses gpg). With rare exceptions we should not use explicit dependencies. Regards, Dmitry > You can find the latest version of these patches on my github repo's > fbsd-support branch: > > https://github.com/pioto/notmuch/compare/master...fbsd-support > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: remove "Testing" from test description in emacs-hello and emacs-show
"Testing" is printed by test/test-lib.sh, so having "Testing" in test description results in duplicate "Testing" in console output. --- test/emacs-hello |2 +- test/emacs-show |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/emacs-hello b/test/emacs-hello index a998dc4..48d1420 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -1,6 +1,6 @@ #!/usr/bin/env bash -test_description="Testing emacs notmuch-hello view" +test_description="emacs notmuch-hello view" . test-lib.sh EXPECTED=$TEST_DIRECTORY/emacs.expected-output diff --git a/test/emacs-show b/test/emacs-show index 5700d2e..2498564 100755 --- a/test/emacs-show +++ b/test/emacs-show @@ -1,6 +1,6 @@ #!/usr/bin/env bash -test_description="Testing emacs notmuch-show view" +test_description="emacs notmuch-show view" . test-lib.sh test_begin_subtest "Hiding Original Message region at beginning of a message" -- 1.7.10 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] emacs: fix custom queries section customization widget in notmuch-hello
The customization widget referred to a non-existing function `notmuch-hello-insert-query-list'. The patch changes it to the correct one - `notmuch-hello-insert-searches'. The relevant test is fixed now. --- emacs/notmuch-hello.el |2 +- test/emacs-hello |1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index f10d98d..684bedc 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -187,7 +187,7 @@ This should only be set by `notmuch-hello-insert-search'.") :tag "Customized queries section (see docstring for details)" :type `(list :tag "" -(const :tag "" notmuch-hello-insert-query-list) +(const :tag "" notmuch-hello-insert-searches) (string :tag "Title for this section") (repeat :tag "Queries" (cons (string :tag "Name") (string :tag "Query"))) diff --git a/test/emacs-hello b/test/emacs-hello index 936d00e..a998dc4 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -52,7 +52,6 @@ test_emacs "(let* ((widget (widget-create 'notmuch-hello-tags-section)) test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-empty-custom-tags-section test_begin_subtest "Empty custom queries section" -test_subtest_known_broken test_emacs "(let* ((widget (widget-create 'notmuch-hello-query-section)) (notmuch-hello-sections (list (widget-value widget (notmuch-hello) -- 1.7.10
[PATCH 1/2] test: add Emacs notmuch-hello tests for custom tags and queries section.
The tests use default values from customization widgets to make sure that these customization widgets work (at least on basic level). The custom queries section test is currently broken. --- test/emacs-hello| 15 +++ .../notmuch-hello-empty-custom-queries-section |3 +++ .../notmuch-hello-empty-custom-tags-section |5 + 3 files changed, 23 insertions(+) create mode 100644 test/emacs.expected-output/notmuch-hello-empty-custom-queries-section create mode 100644 test/emacs.expected-output/notmuch-hello-empty-custom-tags-section diff --git a/test/emacs-hello b/test/emacs-hello index be66ba4..936d00e 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -44,6 +44,21 @@ test_emacs "(let ((notmuch-hello-sections (test-output))" test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts +test_begin_subtest "Empty custom tags section" +test_emacs "(let* ((widget (widget-create 'notmuch-hello-tags-section)) + (notmuch-hello-sections (list (widget-value widget + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-empty-custom-tags-section + +test_begin_subtest "Empty custom queries section" +test_subtest_known_broken +test_emacs "(let* ((widget (widget-create 'notmuch-hello-query-section)) + (notmuch-hello-sections (list (widget-value widget + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-empty-custom-queries-section + test_begin_subtest "Column alignment for tag/queries with long names" tag=a-very-long-tag # length carefully calculated for 80 characters window width notmuch tag +$tag '*' diff --git a/test/emacs.expected-output/notmuch-hello-empty-custom-queries-section b/test/emacs.expected-output/notmuch-hello-empty-custom-queries-section new file mode 100644 index 000..cd0fdf0 --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-empty-custom-queries-section @@ -0,0 +1,3 @@ +: [hide] + + diff --git a/test/emacs.expected-output/notmuch-hello-empty-custom-tags-section b/test/emacs.expected-output/notmuch-hello-empty-custom-tags-section new file mode 100644 index 000..b56fd67 --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-empty-custom-tags-section @@ -0,0 +1,5 @@ +: [hide] + + 4 attachment 7 signed + 52 inbox 52 unread + -- 1.7.10
[PATCH 2/2] emacs: fix custom queries section customization widget in notmuch-hello
The customization widget referred to a non-existing function `notmuch-hello-insert-query-list'. The patch changes it to the correct one - `notmuch-hello-insert-searches'. The relevant test is fixed now. --- emacs/notmuch-hello.el |2 +- test/emacs-hello |1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index f10d98d..684bedc 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -187,7 +187,7 @@ This should only be set by `notmuch-hello-insert-search'.") :tag "Customized queries section (see docstring for details)" :type `(list :tag "" -(const :tag "" notmuch-hello-insert-query-list) +(const :tag "" notmuch-hello-insert-searches) (string :tag "Title for this section") (repeat :tag "Queries" (cons (string :tag "Name") (string :tag "Query"))) diff --git a/test/emacs-hello b/test/emacs-hello index 936d00e..a998dc4 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -52,7 +52,6 @@ test_emacs "(let* ((widget (widget-create 'notmuch-hello-tags-section)) test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-empty-custom-tags-section test_begin_subtest "Empty custom queries section" -test_subtest_known_broken test_emacs "(let* ((widget (widget-create 'notmuch-hello-query-section)) (notmuch-hello-sections (list (widget-value widget (notmuch-hello) -- 1.7.10 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test: add Emacs notmuch-hello tests for custom tags and queries section.
The tests use default values from customization widgets to make sure that these customization widgets work (at least on basic level). The custom queries section test is currently broken. --- test/emacs-hello| 15 +++ .../notmuch-hello-empty-custom-queries-section |3 +++ .../notmuch-hello-empty-custom-tags-section |5 + 3 files changed, 23 insertions(+) create mode 100644 test/emacs.expected-output/notmuch-hello-empty-custom-queries-section create mode 100644 test/emacs.expected-output/notmuch-hello-empty-custom-tags-section diff --git a/test/emacs-hello b/test/emacs-hello index be66ba4..936d00e 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -44,6 +44,21 @@ test_emacs "(let ((notmuch-hello-sections (test-output))" test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts +test_begin_subtest "Empty custom tags section" +test_emacs "(let* ((widget (widget-create 'notmuch-hello-tags-section)) + (notmuch-hello-sections (list (widget-value widget + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-empty-custom-tags-section + +test_begin_subtest "Empty custom queries section" +test_subtest_known_broken +test_emacs "(let* ((widget (widget-create 'notmuch-hello-query-section)) + (notmuch-hello-sections (list (widget-value widget + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-empty-custom-queries-section + test_begin_subtest "Column alignment for tag/queries with long names" tag=a-very-long-tag # length carefully calculated for 80 characters window width notmuch tag +$tag '*' diff --git a/test/emacs.expected-output/notmuch-hello-empty-custom-queries-section b/test/emacs.expected-output/notmuch-hello-empty-custom-queries-section new file mode 100644 index 000..cd0fdf0 --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-empty-custom-queries-section @@ -0,0 +1,3 @@ +: [hide] + + diff --git a/test/emacs.expected-output/notmuch-hello-empty-custom-tags-section b/test/emacs.expected-output/notmuch-hello-empty-custom-tags-section new file mode 100644 index 000..b56fd67 --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-empty-custom-tags-section @@ -0,0 +1,5 @@ +: [hide] + + 4 attachment 7 signed + 52 inbox 52 unread + -- 1.7.10 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/3] Add 'compose' command
Tomi Ollila writes:
> On Wed, Apr 18 2012, Felipe Contreras wrote:
>
>> On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula wrote:
>>> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras >> gmail.com> wrote:
On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula wrote:
> Running "notmuch compose" more than once within a second would result in
> identical message ids for the messages, which is not a good idea. That's
> not likely in interactive use, but the notmuch cli is highly scriptable,
> so someone is bound to hit this.
>
> Some paranoid might also be worried about "leaking" the time you run
> "notmuch compose"... which may be different from the actual time you
> send the message.
It's still better than the current situation; nothing. In any case,
people that have not needed this would not be affected; their UI would
override the Message-ID.
So do you have a better suggestion for a Message-ID?
>>>
>>> The easy way would be to just use g_mime_utils_generate_message_id()
>>> [1]. It doesn't give you any control of the part before @, but I'm not
>>> sure if that really matters.
>>
>> This is what gmime does:
>> g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
>> (unsigned long int) getpid (), count++, fqdn);
>>
>> Which actually has some of the issues you mentioned.
>>
>> I can do the same if you want (add pid and count). The advantage of
>> using our own format is that not only would it be more unique, but it
>> would not have "@fqdn".
>
> getpid() is good. I guess count is useless (always one). Now some ideas
> how to obfuscate time(NULL) (nonce + hash (crc32 good enough?) ?
>
I think the best would be for notmuch to use the gmime function and
change gmime (open a bug, may be provide a patch) to follow the best
practice for Message-ID generation (I guess that would be the document
pointed by Jani [1]).
Regards,
Dmitry
[1] http://www.jwz.org/doc/mid.html
> Tomi
>
>>
>>> Alternatively you can write your own according to e.g. [2]. Glib appears
>>> to have decent and portable support for pseudo random number
>>> generation. But why bother? I'd go with gmime.
>>
>> But gmime doesn't have anything random. I would actually prefer to
>> have something random though.
>
>
>
>>
>> --
>> Felipe Contreras
>> ___
>> notmuch mailing list
>> notmuch at notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/3] Add 'compose' command
Felipe Contreras writes: > On Wed, Apr 18, 2012 at 4:43 PM, Dmitry Kurochkin > wrote: >> Hi Felipe. >> >> Felipe Contreras writes: >> >>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula wrote: >>> >>>> Running "notmuch compose" more than once within a second would result in >>>> identical message ids for the messages, which is not a good idea. That's >>>> not likely in interactive use, but the notmuch cli is highly scriptable, >>>> so someone is bound to hit this. >>>> >>>> Some paranoid might also be worried about "leaking" the time you run >>>> "notmuch compose"... which may be different from the actual time you >>>> send the message. >>> >>> It's still better than the current situation; nothing. In any case, >>> people that have not needed this would not be affected; their UI would >>> override the Message-ID. >>> >> >> I disagree. ?If notmuch CLI generates a Message-ID, it must be a good >> one. ?Otherwise we make users falsely believe that they do have a proper >> Message-ID while in fact they do not. ?And that would bite them sooner >> or later. > > And then they'll report it, and we would fix it. > > Anyway, everything comes from a patch, so, do you have a patch, > pseudo-code, or even a suggestion? > A patch needs some positive reviews to be accepted. Replying to comments with "make a better patch" may not be the best strategy for getting your patches accepted. I do not have a patch. As for a suggestion, I would start with some googling. If you did that, you would probably find the gmime function kindly pointed out by Jani. Regards, Dmitry > -- > Felipe Contreras
[PATCH v2 1/3] Add 'compose' command
Hi Felipe. Felipe Contreras writes: > On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula wrote: > >> Running "notmuch compose" more than once within a second would result in >> identical message ids for the messages, which is not a good idea. That's >> not likely in interactive use, but the notmuch cli is highly scriptable, >> so someone is bound to hit this. >> >> Some paranoid might also be worried about "leaking" the time you run >> "notmuch compose"... which may be different from the actual time you >> send the message. > > It's still better than the current situation; nothing. In any case, > people that have not needed this would not be affected; their UI would > override the Message-ID. > I disagree. If notmuch CLI generates a Message-ID, it must be a good one. Otherwise we make users falsely believe that they do have a proper Message-ID while in fact they do not. And that would bite them sooner or later. Regards, Dmitry > So do you have a better suggestion for a Message-ID? > > -- > Felipe Contreras > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 1/3] Add 'compose' command
Tomi Ollila writes:
> On Wed, Apr 18 2012, Felipe Contreras wrote:
>
>> On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula wrote:
>>> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras
>>> wrote:
On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula wrote:
> Running "notmuch compose" more than once within a second would result in
> identical message ids for the messages, which is not a good idea. That's
> not likely in interactive use, but the notmuch cli is highly scriptable,
> so someone is bound to hit this.
>
> Some paranoid might also be worried about "leaking" the time you run
> "notmuch compose"... which may be different from the actual time you
> send the message.
It's still better than the current situation; nothing. In any case,
people that have not needed this would not be affected; their UI would
override the Message-ID.
So do you have a better suggestion for a Message-ID?
>>>
>>> The easy way would be to just use g_mime_utils_generate_message_id()
>>> [1]. It doesn't give you any control of the part before @, but I'm not
>>> sure if that really matters.
>>
>> This is what gmime does:
>> g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
>> (unsigned long int) getpid (), count++, fqdn);
>>
>> Which actually has some of the issues you mentioned.
>>
>> I can do the same if you want (add pid and count). The advantage of
>> using our own format is that not only would it be more unique, but it
>> would not have "@fqdn".
>
> getpid() is good. I guess count is useless (always one). Now some ideas
> how to obfuscate time(NULL) (nonce + hash (crc32 good enough?) ?
>
I think the best would be for notmuch to use the gmime function and
change gmime (open a bug, may be provide a patch) to follow the best
practice for Message-ID generation (I guess that would be the document
pointed by Jani [1]).
Regards,
Dmitry
[1] http://www.jwz.org/doc/mid.html
> Tomi
>
>>
>>> Alternatively you can write your own according to e.g. [2]. Glib appears
>>> to have decent and portable support for pseudo random number
>>> generation. But why bother? I'd go with gmime.
>>
>> But gmime doesn't have anything random. I would actually prefer to
>> have something random though.
>
>
>
>>
>> --
>> Felipe Contreras
>> ___
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 1/3] Add 'compose' command
Felipe Contreras writes: > On Wed, Apr 18, 2012 at 4:43 PM, Dmitry Kurochkin > wrote: >> Hi Felipe. >> >> Felipe Contreras writes: >> >>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula wrote: >>> >>>> Running "notmuch compose" more than once within a second would result in >>>> identical message ids for the messages, which is not a good idea. That's >>>> not likely in interactive use, but the notmuch cli is highly scriptable, >>>> so someone is bound to hit this. >>>> >>>> Some paranoid might also be worried about "leaking" the time you run >>>> "notmuch compose"... which may be different from the actual time you >>>> send the message. >>> >>> It's still better than the current situation; nothing. In any case, >>> people that have not needed this would not be affected; their UI would >>> override the Message-ID. >>> >> >> I disagree. If notmuch CLI generates a Message-ID, it must be a good >> one. Otherwise we make users falsely believe that they do have a proper >> Message-ID while in fact they do not. And that would bite them sooner >> or later. > > And then they'll report it, and we would fix it. > > Anyway, everything comes from a patch, so, do you have a patch, > pseudo-code, or even a suggestion? > A patch needs some positive reviews to be accepted. Replying to comments with "make a better patch" may not be the best strategy for getting your patches accepted. I do not have a patch. As for a suggestion, I would start with some googling. If you did that, you would probably find the gmime function kindly pointed out by Jani. Regards, Dmitry > -- > Felipe Contreras ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 1/3] Add 'compose' command
Hi Felipe. Felipe Contreras writes: > On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula wrote: > >> Running "notmuch compose" more than once within a second would result in >> identical message ids for the messages, which is not a good idea. That's >> not likely in interactive use, but the notmuch cli is highly scriptable, >> so someone is bound to hit this. >> >> Some paranoid might also be worried about "leaking" the time you run >> "notmuch compose"... which may be different from the actual time you >> send the message. > > It's still better than the current situation; nothing. In any case, > people that have not needed this would not be affected; their UI would > override the Message-ID. > I disagree. If notmuch CLI generates a Message-ID, it must be a good one. Otherwise we make users falsely believe that they do have a proper Message-ID while in fact they do not. And that would bite them sooner or later. Regards, Dmitry > So do you have a better suggestion for a Message-ID? > > -- > Felipe Contreras > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[RFC PATCH 3/4] emacs: add notmuch hello refresh hook to display message count change
Jani Nikula writes: > On Tue, 17 Apr 2012 13:13:15 +0400, Dmitry Kurochkin gmail.com> wrote: >> Jani Nikula writes: >> >> > Add a notmuch hello refresh hook to display a message about change in >> > message count in the database since the notmuch-hello buffer was last >> > refreshed manually (no-display is nil). >> >> I like this idea. But IMO we should avoid another call to notmuch >> count. Notmuch-hello buffer already displays the message count on the >> first line. I would propose to implement this functionality not as a >> hook but as part of the section which outputs the first line. We can >> add an option to disable it if you prefer but I do not think it is >> needed. This is less flexible than a hook, but IMO it is not a big >> issue. > > And what if someone has disabled the header section but would want the > message? Well, that is what I meant by "less flexible than a hook". > Also, you'd have to pass no-display there too. Sure, we can pass it to sections. > IMHO one call to > notmuch count is not a big issue, especially for an optional feature. > And having it as a hook very nicely isolates the feature from everything > else. > I think this feature should be enabled by default. I guess you are right that it is not a big issue. I still think we would be better without it (and we still can isolate the feature), but I would not object to having the extra call. Regards, Dmitry > Jani. > > >> >> Regards, >> Dmitry >> >> > --- >> > emacs/notmuch-hello.el | 23 +++ >> > 1 files changed, 23 insertions(+), 0 deletions(-) >> > >> > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el >> > index 0596bbe..13da146 100644 >> > --- a/emacs/notmuch-hello.el >> > +++ b/emacs/notmuch-hello.el >> > @@ -148,6 +148,7 @@ International Bureau of Weights and Measures." >> > (defcustom notmuch-hello-refresh-hook nil >> >"Functions called after updating a `notmuch-hello' buffer." >> >:type 'hook >> > + :options '(notmuch-hello-refresh-status-message) >> >:group 'notmuch-hello >> >:group 'notmuch-hooks) >> > >> > @@ -729,6 +730,28 @@ following: >> > (let ((fill-column (- (window-width) notmuch-hello-indent))) >> >(center-region start (point) >> > >> > +(defvar notmuch-hello-refresh-count 0 >> > + "Number of messages in the database when `notmuch-hello' was last run. >> > + >> > +Used internally by `notmuch-hello-refresh-status-message'.") >> > + >> > +(defun notmuch-hello-refresh-status-message (no-display) >> > + "Hook to display a status message when refreshing notmuch-hello buffer." >> > + (unless no-display >> > +(let* ((new-count >> > + (string-to-number (car (process-lines notmuch-command "count" >> > + (diff-count (- new-count notmuch-hello-refresh-count))) >> > + (if (= notmuch-hello-refresh-count 0) >> > +(message "You have %s messages." >> > + (notmuch-hello-nice-number new-count)) >> > + (if (not (= diff-count 0)) >> > + (if (>= diff-count 0) >> > + (message "You have %s more messages since last refresh." >> > + (notmuch-hello-nice-number diff-count)) >> > +(message "You have %s fewer messages since last refresh." >> > + (notmuch-hello-nice-number (- diff-count)) >> > + (setq notmuch-hello-refresh-count new-count >> > + >> > ;;;###autoload >> > (defun notmuch-hello (&optional no-display) >> >"Run notmuch and display saved searches, known tags, etc." >> > -- >> > 1.7.1 >> > >> > ___ >> > notmuch mailing list >> > notmuch at notmuchmail.org >> > http://notmuchmail.org/mailman/listinfo/notmuch
[RFC PATCH 2/4] emacs: add no-display arg to notmuch-hello-refresh-hook
Jani Nikula writes: > On Tue, 17 Apr 2012 13:06:13 +0400, Dmitry Kurochkin gmail.com> wrote: >> Can you please give some explanaition why is this needed? Would this >> change break custom hooks that do not expect any arguments? > > For patch 3/4. We don't want to display a message if someone calls > (notmuch-hello-update t) from some script, and notmuch-hello buffer > isn't even visible. > > Yes, it would break custom hooks. And a bunch of tests. But I think it > would be useful for custom hooks too, for the same reason as above. > Makes sense. I think it should be mentioned in the commit message. Especially the fact that it breaks existing hooks. We should mention it in the NEWS later as well. Regards, Dmitry > Jani. > > > >> >> Regards, >> Dmitry >> >> Jani Nikula writes: >> >> > --- >> > emacs/notmuch-hello.el |2 +- >> > 1 files changed, 1 insertions(+), 1 deletions(-) >> > >> > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el >> > index 9cd907a..0596bbe 100644 >> > --- a/emacs/notmuch-hello.el >> > +++ b/emacs/notmuch-hello.el >> > @@ -776,7 +776,7 @@ following: >> > (widget-setup) >> > >> > (goto-char final-target-pos)) >> > - (run-hooks 'notmuch-hello-refresh-hook) >> > + (run-hook-with-args 'notmuch-hello-refresh-hook no-display) >> >(setq notmuch-hello-first-run nil)) >> > >> > (defun notmuch-folder () >> > -- >> > 1.7.1 >> > >> > ___ >> > notmuch mailing list >> > notmuch at notmuchmail.org >> > http://notmuchmail.org/mailman/listinfo/notmuch
[RFC PATCH 4/4] emacs: add notmuch hello refresh hook to place point in a widget
Jani Nikula writes: > Add support for putting point to a widget after refresh through a > hook. This approximates the old behaviour. I may be wrong, but this looks to me like a hack that cannot work well. See my first reply in the thread for ideas on how to better implement this functionality. Regards, Dmitry > --- > emacs/notmuch-hello.el |8 +++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 13da146..07e64d4 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -148,7 +148,8 @@ International Bureau of Weights and Measures." > (defcustom notmuch-hello-refresh-hook nil >"Functions called after updating a `notmuch-hello' buffer." >:type 'hook > - :options '(notmuch-hello-refresh-status-message) > + :options '(notmuch-hello-refresh-status-message > + notmuch-hello-refresh-point-to-widget) >:group 'notmuch-hello >:group 'notmuch-hooks) > > @@ -752,6 +753,11 @@ Used internally by > `notmuch-hello-refresh-status-message'.") > (notmuch-hello-nice-number (- diff-count)) >(setq notmuch-hello-refresh-count new-count > > +(defun notmuch-hello-refresh-point-to-widget (no-display) > + "Hook to place point to widget after notmuch-hello refresh." > + (widget-backward 1) > + (widget-forward 1)) > + > ;;;###autoload > (defun notmuch-hello (&optional no-display) >"Run notmuch and display saved searches, known tags, etc." > -- > 1.7.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[RFC PATCH 3/4] emacs: add notmuch hello refresh hook to display message count change
Jani Nikula writes: > Add a notmuch hello refresh hook to display a message about change in > message count in the database since the notmuch-hello buffer was last > refreshed manually (no-display is nil). I like this idea. But IMO we should avoid another call to notmuch count. Notmuch-hello buffer already displays the message count on the first line. I would propose to implement this functionality not as a hook but as part of the section which outputs the first line. We can add an option to disable it if you prefer but I do not think it is needed. This is less flexible than a hook, but IMO it is not a big issue. Regards, Dmitry > --- > emacs/notmuch-hello.el | 23 +++ > 1 files changed, 23 insertions(+), 0 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 0596bbe..13da146 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -148,6 +148,7 @@ International Bureau of Weights and Measures." > (defcustom notmuch-hello-refresh-hook nil >"Functions called after updating a `notmuch-hello' buffer." >:type 'hook > + :options '(notmuch-hello-refresh-status-message) >:group 'notmuch-hello >:group 'notmuch-hooks) > > @@ -729,6 +730,28 @@ following: > (let ((fill-column (- (window-width) notmuch-hello-indent))) >(center-region start (point) > > +(defvar notmuch-hello-refresh-count 0 > + "Number of messages in the database when `notmuch-hello' was last run. > + > +Used internally by `notmuch-hello-refresh-status-message'.") > + > +(defun notmuch-hello-refresh-status-message (no-display) > + "Hook to display a status message when refreshing notmuch-hello buffer." > + (unless no-display > +(let* ((new-count > + (string-to-number (car (process-lines notmuch-command "count" > +(diff-count (- new-count notmuch-hello-refresh-count))) > + (if (= notmuch-hello-refresh-count 0) > + (message "You have %s messages." > +(notmuch-hello-nice-number new-count)) > + (if (not (= diff-count 0)) > + (if (>= diff-count 0) > + (message "You have %s more messages since last refresh." > + (notmuch-hello-nice-number diff-count)) > + (message "You have %s fewer messages since last refresh." > +(notmuch-hello-nice-number (- diff-count)) > + (setq notmuch-hello-refresh-count new-count > + > ;;;###autoload > (defun notmuch-hello (&optional no-display) >"Run notmuch and display saved searches, known tags, etc." > -- > 1.7.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[RFC PATCH 2/4] emacs: add no-display arg to notmuch-hello-refresh-hook
Can you please give some explanaition why is this needed? Would this change break custom hooks that do not expect any arguments? Regards, Dmitry Jani Nikula writes: > --- > emacs/notmuch-hello.el |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 9cd907a..0596bbe 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -776,7 +776,7 @@ following: > (widget-setup) > > (goto-char final-target-pos)) > - (run-hooks 'notmuch-hello-refresh-hook) > + (run-hook-with-args 'notmuch-hello-refresh-hook no-display) >(setq notmuch-hello-first-run nil)) > > (defun notmuch-folder () > -- > 1.7.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[RFC PATCH 1/4] emacs: simplify point placement in notmuch-hello
Hi Jani. Jani Nikula writes: > notmuch-hello (called also through notmuch-hello-update, bound to '=' > by default) tries to find the widget under or following point before > refresh, and put the point back to the widget afterwards. The code has > gotten a bit complicated, and has at least the following issues: > > 1) All the individual section functions have to include code to >support point placement. If there is no such support, point is >dropped to the search box. Only saved searches and all tags >sections support point placement. > > 2) Point placement is based on widget-value. If there are two widgets >with the same widget-value (for example a saved search with the >same name as a tag) the point is moved to the earlier one. > > 3) When first entering notmuch-hello notmuch-hello-target is nil, and >point is dropped to the search box. > > This patch simplifies the code by removing all point placement based > on widgets. Point is simply saved before refresh, and put back to > where it was. Sometimes, but not very often, this would have the > appearance of moving the point relative to the nearest widgets. IMHO > this is a minor problem compared to the issues listed above. > > A downside is that there's no visual cue (point movement) to indicate > that refresh has finished. Then again, neither was there before, if > point was at the beginning of a widget. Thanks for looking into this. This is an annoying issue indeed. And I was thinking about fixing it myself. I am not sure I like the approach of moving the cursor to the same position. It is common that buffer content would change significantly after a refresh (e.g. after I archived all new mail). That would mean the cursor would just randomly jump somewhere. IMO we should allow smart cursor positioning which means that logic should go to individual sections. I would propose the following plan: 1. Remove special case for search box. No section should be special. Moreover it is possible to remove it (I did it) and in that case the cursor would be left at the end of the buffer. By default, the cursor should be moved to the beginning of the buffer. 2. Replace current cursor positioning logic with section specific code. I.e. `notmuch-hello' would not do any cursor positioning (except for item 1) but queries and tags section would save required state when a button is clicked and the same section would use this state to restore cursor position on refresh. What state should be saved would depend on the section but we should at least save the section name/ID. If during refresh no section set the cursor position, then the cursor is moved to the beginning of the buffer. 3. Provide a custom variable to set the default section to move the cursor to. I.e. set the section name/ID part of the state from item 2. Again, details on what the default position inside the section is would depend on the section. For search box, it would be the input field. For queries/tags it would be the first tag. Item 1 is pretty simple. The rest may be more tricky. What do you think? Regards, Dmitry > --- > emacs/notmuch-hello.el | 70 +++ > 1 files changed, 17 insertions(+), 53 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 71d37b8..9cd907a 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -154,11 +154,6 @@ International Bureau of Weights and Measures." > (defvar notmuch-hello-url "http://notmuchmail.org"; >"The `notmuch' web site.") > > -(defvar notmuch-hello-search-pos nil > - "Position of search widget, if any. > - > -This should only be set by `notmuch-hello-insert-search'.") > - > (defvar notmuch-hello-custom-section-options >'((:filter (string :tag "Filter for each tag")) > (:filter-count (string :tag "Different filter to generate message > counts")) > @@ -209,11 +204,8 @@ function produces a section simply by adding content to > the current > buffer. A section should not end with an empty line, because a > newline will be inserted after each section by `notmuch-hello'. > > -Each function should take no arguments. If the produced section > -includes `notmuch-hello-target' (i.e. cursor should be positioned > -inside this section), the function should return this element's > -position. > -Otherwise, it should return nil. > +Each function should take no arguments. The return value is > +ignored. > > For convenience an element can also be a list of the form (FUNC ARG1 > ARG2 .. ARGN) in which case FUNC will be applied to the rest of the > @@ -240,15 +232,6 @@ supported for \"Customized queries section\" items." > notmuch-hello-query-section > (function :tag "Custom section" > > -(defvar notmuch-hello-target nil > - "Button text at position of point before rebuilding the notmuch-buffer. > - > -This variable contains the text of the
Re: [RFC PATCH 3/4] emacs: add notmuch hello refresh hook to display message count change
Jani Nikula writes: > On Tue, 17 Apr 2012 13:13:15 +0400, Dmitry Kurochkin > wrote: >> Jani Nikula writes: >> >> > Add a notmuch hello refresh hook to display a message about change in >> > message count in the database since the notmuch-hello buffer was last >> > refreshed manually (no-display is nil). >> >> I like this idea. But IMO we should avoid another call to notmuch >> count. Notmuch-hello buffer already displays the message count on the >> first line. I would propose to implement this functionality not as a >> hook but as part of the section which outputs the first line. We can >> add an option to disable it if you prefer but I do not think it is >> needed. This is less flexible than a hook, but IMO it is not a big >> issue. > > And what if someone has disabled the header section but would want the > message? Well, that is what I meant by "less flexible than a hook". > Also, you'd have to pass no-display there too. Sure, we can pass it to sections. > IMHO one call to > notmuch count is not a big issue, especially for an optional feature. > And having it as a hook very nicely isolates the feature from everything > else. > I think this feature should be enabled by default. I guess you are right that it is not a big issue. I still think we would be better without it (and we still can isolate the feature), but I would not object to having the extra call. Regards, Dmitry > Jani. > > >> >> Regards, >> Dmitry >> >> > --- >> > emacs/notmuch-hello.el | 23 +++ >> > 1 files changed, 23 insertions(+), 0 deletions(-) >> > >> > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el >> > index 0596bbe..13da146 100644 >> > --- a/emacs/notmuch-hello.el >> > +++ b/emacs/notmuch-hello.el >> > @@ -148,6 +148,7 @@ International Bureau of Weights and Measures." >> > (defcustom notmuch-hello-refresh-hook nil >> >"Functions called after updating a `notmuch-hello' buffer." >> >:type 'hook >> > + :options '(notmuch-hello-refresh-status-message) >> >:group 'notmuch-hello >> >:group 'notmuch-hooks) >> > >> > @@ -729,6 +730,28 @@ following: >> > (let ((fill-column (- (window-width) notmuch-hello-indent))) >> >(center-region start (point) >> > >> > +(defvar notmuch-hello-refresh-count 0 >> > + "Number of messages in the database when `notmuch-hello' was last run. >> > + >> > +Used internally by `notmuch-hello-refresh-status-message'.") >> > + >> > +(defun notmuch-hello-refresh-status-message (no-display) >> > + "Hook to display a status message when refreshing notmuch-hello buffer." >> > + (unless no-display >> > +(let* ((new-count >> > + (string-to-number (car (process-lines notmuch-command "count" >> > + (diff-count (- new-count notmuch-hello-refresh-count))) >> > + (if (= notmuch-hello-refresh-count 0) >> > +(message "You have %s messages." >> > + (notmuch-hello-nice-number new-count)) >> > + (if (not (= diff-count 0)) >> > + (if (>= diff-count 0) >> > + (message "You have %s more messages since last refresh." >> > + (notmuch-hello-nice-number diff-count)) >> > +(message "You have %s fewer messages since last refresh." >> > + (notmuch-hello-nice-number (- diff-count)) >> > + (setq notmuch-hello-refresh-count new-count >> > + >> > ;;;###autoload >> > (defun notmuch-hello (&optional no-display) >> >"Run notmuch and display saved searches, known tags, etc." >> > -- >> > 1.7.1 >> > >> > ___ >> > notmuch mailing list >> > notmuch@notmuchmail.org >> > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [RFC PATCH 2/4] emacs: add no-display arg to notmuch-hello-refresh-hook
Jani Nikula writes: > On Tue, 17 Apr 2012 13:06:13 +0400, Dmitry Kurochkin > wrote: >> Can you please give some explanaition why is this needed? Would this >> change break custom hooks that do not expect any arguments? > > For patch 3/4. We don't want to display a message if someone calls > (notmuch-hello-update t) from some script, and notmuch-hello buffer > isn't even visible. > > Yes, it would break custom hooks. And a bunch of tests. But I think it > would be useful for custom hooks too, for the same reason as above. > Makes sense. I think it should be mentioned in the commit message. Especially the fact that it breaks existing hooks. We should mention it in the NEWS later as well. Regards, Dmitry > Jani. > > > >> >> Regards, >> Dmitry >> >> Jani Nikula writes: >> >> > --- >> > emacs/notmuch-hello.el |2 +- >> > 1 files changed, 1 insertions(+), 1 deletions(-) >> > >> > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el >> > index 9cd907a..0596bbe 100644 >> > --- a/emacs/notmuch-hello.el >> > +++ b/emacs/notmuch-hello.el >> > @@ -776,7 +776,7 @@ following: >> > (widget-setup) >> > >> > (goto-char final-target-pos)) >> > - (run-hooks 'notmuch-hello-refresh-hook) >> > + (run-hook-with-args 'notmuch-hello-refresh-hook no-display) >> >(setq notmuch-hello-first-run nil)) >> > >> > (defun notmuch-folder () >> > -- >> > 1.7.1 >> > >> > ___ >> > notmuch mailing list >> > notmuch@notmuchmail.org >> > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [RFC PATCH 4/4] emacs: add notmuch hello refresh hook to place point in a widget
Jani Nikula writes: > Add support for putting point to a widget after refresh through a > hook. This approximates the old behaviour. I may be wrong, but this looks to me like a hack that cannot work well. See my first reply in the thread for ideas on how to better implement this functionality. Regards, Dmitry > --- > emacs/notmuch-hello.el |8 +++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 13da146..07e64d4 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -148,7 +148,8 @@ International Bureau of Weights and Measures." > (defcustom notmuch-hello-refresh-hook nil >"Functions called after updating a `notmuch-hello' buffer." >:type 'hook > - :options '(notmuch-hello-refresh-status-message) > + :options '(notmuch-hello-refresh-status-message > + notmuch-hello-refresh-point-to-widget) >:group 'notmuch-hello >:group 'notmuch-hooks) > > @@ -752,6 +753,11 @@ Used internally by > `notmuch-hello-refresh-status-message'.") > (notmuch-hello-nice-number (- diff-count)) >(setq notmuch-hello-refresh-count new-count > > +(defun notmuch-hello-refresh-point-to-widget (no-display) > + "Hook to place point to widget after notmuch-hello refresh." > + (widget-backward 1) > + (widget-forward 1)) > + > ;;;###autoload > (defun notmuch-hello (&optional no-display) >"Run notmuch and display saved searches, known tags, etc." > -- > 1.7.1 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [RFC PATCH 3/4] emacs: add notmuch hello refresh hook to display message count change
Jani Nikula writes: > Add a notmuch hello refresh hook to display a message about change in > message count in the database since the notmuch-hello buffer was last > refreshed manually (no-display is nil). I like this idea. But IMO we should avoid another call to notmuch count. Notmuch-hello buffer already displays the message count on the first line. I would propose to implement this functionality not as a hook but as part of the section which outputs the first line. We can add an option to disable it if you prefer but I do not think it is needed. This is less flexible than a hook, but IMO it is not a big issue. Regards, Dmitry > --- > emacs/notmuch-hello.el | 23 +++ > 1 files changed, 23 insertions(+), 0 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 0596bbe..13da146 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -148,6 +148,7 @@ International Bureau of Weights and Measures." > (defcustom notmuch-hello-refresh-hook nil >"Functions called after updating a `notmuch-hello' buffer." >:type 'hook > + :options '(notmuch-hello-refresh-status-message) >:group 'notmuch-hello >:group 'notmuch-hooks) > > @@ -729,6 +730,28 @@ following: > (let ((fill-column (- (window-width) notmuch-hello-indent))) >(center-region start (point) > > +(defvar notmuch-hello-refresh-count 0 > + "Number of messages in the database when `notmuch-hello' was last run. > + > +Used internally by `notmuch-hello-refresh-status-message'.") > + > +(defun notmuch-hello-refresh-status-message (no-display) > + "Hook to display a status message when refreshing notmuch-hello buffer." > + (unless no-display > +(let* ((new-count > + (string-to-number (car (process-lines notmuch-command "count" > +(diff-count (- new-count notmuch-hello-refresh-count))) > + (if (= notmuch-hello-refresh-count 0) > + (message "You have %s messages." > +(notmuch-hello-nice-number new-count)) > + (if (not (= diff-count 0)) > + (if (>= diff-count 0) > + (message "You have %s more messages since last refresh." > + (notmuch-hello-nice-number diff-count)) > + (message "You have %s fewer messages since last refresh." > +(notmuch-hello-nice-number (- diff-count)) > + (setq notmuch-hello-refresh-count new-count > + > ;;;###autoload > (defun notmuch-hello (&optional no-display) >"Run notmuch and display saved searches, known tags, etc." > -- > 1.7.1 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [RFC PATCH 2/4] emacs: add no-display arg to notmuch-hello-refresh-hook
Can you please give some explanaition why is this needed? Would this change break custom hooks that do not expect any arguments? Regards, Dmitry Jani Nikula writes: > --- > emacs/notmuch-hello.el |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 9cd907a..0596bbe 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -776,7 +776,7 @@ following: > (widget-setup) > > (goto-char final-target-pos)) > - (run-hooks 'notmuch-hello-refresh-hook) > + (run-hook-with-args 'notmuch-hello-refresh-hook no-display) >(setq notmuch-hello-first-run nil)) > > (defun notmuch-folder () > -- > 1.7.1 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [RFC PATCH 1/4] emacs: simplify point placement in notmuch-hello
Hi Jani. Jani Nikula writes: > notmuch-hello (called also through notmuch-hello-update, bound to '=' > by default) tries to find the widget under or following point before > refresh, and put the point back to the widget afterwards. The code has > gotten a bit complicated, and has at least the following issues: > > 1) All the individual section functions have to include code to >support point placement. If there is no such support, point is >dropped to the search box. Only saved searches and all tags >sections support point placement. > > 2) Point placement is based on widget-value. If there are two widgets >with the same widget-value (for example a saved search with the >same name as a tag) the point is moved to the earlier one. > > 3) When first entering notmuch-hello notmuch-hello-target is nil, and >point is dropped to the search box. > > This patch simplifies the code by removing all point placement based > on widgets. Point is simply saved before refresh, and put back to > where it was. Sometimes, but not very often, this would have the > appearance of moving the point relative to the nearest widgets. IMHO > this is a minor problem compared to the issues listed above. > > A downside is that there's no visual cue (point movement) to indicate > that refresh has finished. Then again, neither was there before, if > point was at the beginning of a widget. Thanks for looking into this. This is an annoying issue indeed. And I was thinking about fixing it myself. I am not sure I like the approach of moving the cursor to the same position. It is common that buffer content would change significantly after a refresh (e.g. after I archived all new mail). That would mean the cursor would just randomly jump somewhere. IMO we should allow smart cursor positioning which means that logic should go to individual sections. I would propose the following plan: 1. Remove special case for search box. No section should be special. Moreover it is possible to remove it (I did it) and in that case the cursor would be left at the end of the buffer. By default, the cursor should be moved to the beginning of the buffer. 2. Replace current cursor positioning logic with section specific code. I.e. `notmuch-hello' would not do any cursor positioning (except for item 1) but queries and tags section would save required state when a button is clicked and the same section would use this state to restore cursor position on refresh. What state should be saved would depend on the section but we should at least save the section name/ID. If during refresh no section set the cursor position, then the cursor is moved to the beginning of the buffer. 3. Provide a custom variable to set the default section to move the cursor to. I.e. set the section name/ID part of the state from item 2. Again, details on what the default position inside the section is would depend on the section. For search box, it would be the input field. For queries/tags it would be the first tag. Item 1 is pretty simple. The rest may be more tricky. What do you think? Regards, Dmitry > --- > emacs/notmuch-hello.el | 70 +++ > 1 files changed, 17 insertions(+), 53 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index 71d37b8..9cd907a 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -154,11 +154,6 @@ International Bureau of Weights and Measures." > (defvar notmuch-hello-url "http://notmuchmail.org"; >"The `notmuch' web site.") > > -(defvar notmuch-hello-search-pos nil > - "Position of search widget, if any. > - > -This should only be set by `notmuch-hello-insert-search'.") > - > (defvar notmuch-hello-custom-section-options >'((:filter (string :tag "Filter for each tag")) > (:filter-count (string :tag "Different filter to generate message > counts")) > @@ -209,11 +204,8 @@ function produces a section simply by adding content to > the current > buffer. A section should not end with an empty line, because a > newline will be inserted after each section by `notmuch-hello'. > > -Each function should take no arguments. If the produced section > -includes `notmuch-hello-target' (i.e. cursor should be positioned > -inside this section), the function should return this element's > -position. > -Otherwise, it should return nil. > +Each function should take no arguments. The return value is > +ignored. > > For convenience an element can also be a list of the form (FUNC ARG1 > ARG2 .. ARGN) in which case FUNC will be applied to the rest of the > @@ -240,15 +232,6 @@ supported for \"Customized queries section\" items." > notmuch-hello-query-section > (function :tag "Custom section" > > -(defvar notmuch-hello-target nil > - "Button text at position of point before rebuilding the notmuch-buffer. > - > -This variable contains the text of the
[PATCH] emacs: get rid of trailing spaces in notmuch-hello view
David Bremner writes: > Dmitry Kurochkin writes: > >> I am going to remove needs-review tag from this one since there seems to >> be no one interested in reviewing it. The patch itself may not be >> trivial, but it is pretty simple and does not make any critical changes. >> So I believe it is safe to push it. If there are any issues with the >> patch discovered later, I am committed to fix them. > > Sorry, but if I can't take in the patch at a glance, it needs some > review before I am willing to push it. > Ok. Tagged as needs-review again. Let's see if I can attract some attention to the patch on IRC. Regards, Dmitry > d
[PATCH] emacs: get rid of trailing spaces in notmuch-hello view
Hi all. I am going to remove needs-review tag from this one since there seems to be no one interested in reviewing it. The patch itself may not be trivial, but it is pretty simple and does not make any critical changes. So I believe it is safe to push it. If there are any issues with the patch discovered later, I am committed to fix them. Regards, Dmitry
Re: [PATCH] emacs: get rid of trailing spaces in notmuch-hello view
David Bremner writes: > Dmitry Kurochkin writes: > >> I am going to remove needs-review tag from this one since there seems to >> be no one interested in reviewing it. The patch itself may not be >> trivial, but it is pretty simple and does not make any critical changes. >> So I believe it is safe to push it. If there are any issues with the >> patch discovered later, I am committed to fix them. > > Sorry, but if I can't take in the patch at a glance, it needs some > review before I am willing to push it. > Ok. Tagged as needs-review again. Let's see if I can attract some attention to the patch on IRC. Regards, Dmitry > d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: get rid of trailing spaces in notmuch-hello view
Hi all. I am going to remove needs-review tag from this one since there seems to be no one interested in reviewing it. The patch itself may not be trivial, but it is pretty simple and does not make any critical changes. So I believe it is safe to push it. If there are any issues with the patch discovered later, I am committed to fix them. Regards, Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/6] emacs: modify notmuch-search-tag to not prompt if tags provided as argument
Jameson Graef Rollins writes: > If the argument is a full string or a list, the function will assume > this is a tag string or list and will not prompt the user. If the > argument is nil or the exact strings "-" or "+" then the user will be > prompted. > > The function doc is updated accordingly. > --- > emacs/notmuch.el | 16 > 1 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index 0ab8fc2..3b78584 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -646,13 +646,21 @@ added or removed for all threads in the region from BEG > to END." > (notmuch-update-tags (notmuch-search-get-tags) tag-changes)) > (forward-line)) > > -(defun notmuch-search-tag (&optional initial-input) > - "Change tags for the currently selected thread or region." > +(defun notmuch-search-tag (&optional tags) > + "Change tags for the currently selected thread or region. > + > +If TAGS is a string or list it will be interpreted as tags to > +apply to the selected messages. If TAGS is nil or either of the > +strings `-' or `+' the user will be prompted to enter tags (with > +tab completion)." The TAGS argument name may be confusing. Other tagging functions tend to use TAG-CHANGES for these. Can you please change the argument name here for consistency? >(interactive) >(let* ((beg (if (region-active-p) (region-beginning) (point))) >(end (if (region-active-p) (region-end) (point))) > - (search-string (notmuch-search-find-thread-id-region-search beg end)) > - (tags (notmuch-read-tag-changes initial-input search-string))) > + (search-string (notmuch-search-find-thread-id-region-search beg end))) > +(if (string-or-null-p tags) > + (if (or (string= tags "-") (string= tags "+") (eq tags nil)) Should we add a check for an empty string? Please use `null'. > + (setq tags (notmuch-read-tag-changes tags search-string)) > + (setq tags (list tags Should we change `notmuch-tag' to accept strings instead of handling it here? Regards, Dmitry > (apply 'notmuch-search-tag-region beg end tags))) > > (defun notmuch-search-add-tag () > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/6] emacs: have tag-completion return all tags for nil input
Jameson Graef Rollins writes: > Previously the function would fail if the initial input was nil. Now > it will return a list of all tags, which obviously makes much more > sense. > --- > emacs/notmuch.el |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index 38ae005..0ab8fc2 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -85,6 +85,8 @@ For example: > `notmuch-read-tag-changes' function.") > > (defun notmuch-tag-completions (&optional search-terms) > + (if (eq search-terms nil) Please use `null'. Regards, Dmitry > + (setq search-terms (list "*"))) >(split-string > (with-output-to-string > (with-current-buffer standard-output > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/6] emacs: modify notmuch-search-tag to not prompt if tags provided as argument
Jameson Graef Rollins writes: > If the argument is a full string or a list, the function will assume > this is a tag string or list and will not prompt the user. If the > argument is nil or the exact strings "-" or "+" then the user will be > prompted. > > The function doc is updated accordingly. > --- > emacs/notmuch.el | 16 > 1 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index 0ab8fc2..3b78584 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -646,13 +646,21 @@ added or removed for all threads in the region from BEG > to END." > (notmuch-update-tags (notmuch-search-get-tags) tag-changes)) > (forward-line)) > > -(defun notmuch-search-tag (&optional initial-input) > - "Change tags for the currently selected thread or region." > +(defun notmuch-search-tag (&optional tags) > + "Change tags for the currently selected thread or region. > + > +If TAGS is a string or list it will be interpreted as tags to > +apply to the selected messages. If TAGS is nil or either of the > +strings `-' or `+' the user will be prompted to enter tags (with > +tab completion)." The TAGS argument name may be confusing. Other tagging functions tend to use TAG-CHANGES for these. Can you please change the argument name here for consistency? >(interactive) >(let* ((beg (if (region-active-p) (region-beginning) (point))) >(end (if (region-active-p) (region-end) (point))) > - (search-string (notmuch-search-find-thread-id-region-search beg end)) > - (tags (notmuch-read-tag-changes initial-input search-string))) > + (search-string (notmuch-search-find-thread-id-region-search beg end))) > +(if (string-or-null-p tags) > + (if (or (string= tags "-") (string= tags "+") (eq tags nil)) Should we add a check for an empty string? Please use `null'. > + (setq tags (notmuch-read-tag-changes tags search-string)) > + (setq tags (list tags Should we change `notmuch-tag' to accept strings instead of handling it here? Regards, Dmitry > (apply 'notmuch-search-tag-region beg end tags))) > > (defun notmuch-search-add-tag () > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/6] emacs: have tag-completion return all tags for nil input
Jameson Graef Rollins writes: > Previously the function would fail if the initial input was nil. Now > it will return a list of all tags, which obviously makes much more > sense. > --- > emacs/notmuch.el |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index 38ae005..0ab8fc2 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -85,6 +85,8 @@ For example: > `notmuch-read-tag-changes' function.") > > (defun notmuch-tag-completions (&optional search-terms) > + (if (eq search-terms nil) Please use `null'. Regards, Dmitry > + (setq search-terms (list "*"))) >(split-string > (with-output-to-string > (with-current-buffer standard-output > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test: add broken test for long names in Emacs notmuch-hello view
Hi all. It has been a while since these patches were posted. They are pretty simple, just a single line (and a single char) is changed in actual code. So I am going to remove the needs-review tag. Regards, Dmitry
Re: [PATCH 1/2] test: add broken test for long names in Emacs notmuch-hello view
Hi all. It has been a while since these patches were posted. They are pretty simple, just a single line (and a single char) is changed in actual code. So I am going to remove the needs-review tag. Regards, Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[BUG/PATCH 2/2] emacs: Fix replying from alternate addresses
Adam Wolfe Gordon writes: > The bug was that notmuch-mua-mail used `mail-header` to check whether > it was passed a "From" header. The implementation of `mail-header` > must try to compare symbols instead of strings when looking for > headers, as it was returning nil when a From header was present. This > is probably because the mail functions construct headers as alists > with symbols for the header names, while our code uses strings for the > header names. > > Since we don't use `mail-header` anywhere else, and `message-mail` is > perfectly happy to accept string header names, the fix is just to use > `assoc` to look for the From header, so that the strings get compared > properly. > --- LGTM. Thanks for fixing this. Regards, Dmitry > emacs/notmuch-mua.el |4 ++-- > test/emacs |1 - > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el > index 6aae3a0..9805d79 100644 > --- a/emacs/notmuch-mua.el > +++ b/emacs/notmuch-mua.el > @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'." >(when (not (string= "" user-agent)) > (push (cons "User-Agent" user-agent) other-headers > > - (unless (mail-header 'From other-headers) > + (unless (assoc "From" other-headers) > (push (cons "From" (concat > (notmuch-user-name) " <" (notmuch-user-primary-email) > ">")) other-headers)) > > @@ -250,7 +250,7 @@ the From: address first." >(interactive "P") >(let ((other-headers >(when (or prompt-for-sender notmuch-always-prompt-for-sender) > -(list (cons 'From (notmuch-mua-prompt-for-sender)) > +(list (cons "From" (notmuch-mua-prompt-for-sender)) > (notmuch-mua-mail nil nil other-headers))) > > (defun notmuch-mua-new-forward-message (&optional prompt-for-sender) > diff --git a/test/emacs b/test/emacs > index fa5d706..08db1ee 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -275,7 +275,6 @@ EOF > test_expect_equal_file OUTPUT EXPECTED > > test_begin_subtest "Reply from alternate address within emacs" > -test_subtest_known_broken > add_message '[from]="Sender "' \ >[to]=test_suite_other at notmuchmail.org \ >[subject]=notmuch-reply-test \ > -- > 1.7.5.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[BUG/PATCH 1/2] test: Tests for reply from alternate addresses in emacs
Adam Wolfe Gordon writes:
> Since the recent reply changes were pushed, there has been a bug that
> causes emacs to always reply from the primary address, even if the
> JSON or default CLI reply output uses an alternate address.
>
> This adds two tests to the emacs test library based on the two "Reply
> form..." tests in the reply test library. One is currently marked
> broken.
> ---
> test/emacs | 52
> 1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 8a28705..fa5d706 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -274,6 +274,58 @@ Notmuch Test Suite
> writes:
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> +test_begin_subtest "Reply from alternate address within emacs"
> +test_subtest_known_broken
> +add_message '[from]="Sender "' \
> + [to]=test_suite_other at notmuchmail.org \
> + [subject]=notmuch-reply-test \
> + '[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> + '[body]="reply from alternate address"'
Please remove subject, data and body parameters. add_message would
provide sane default values.
> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat +From: Notmuch Test Suite
> +To: Sender
> +Subject: Re: notmuch-reply-test
> +In-Reply-To:
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender writes:
> +
> +> reply from alternate address
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "Reply from address in named group list within emacs"
> +add_message '[from]="Sender "' \
> +'[to]=group:test_suite at notmuchmail.org,someone at
> example.com\;' \
> + [cc]=test_suite_other at notmuchmail.org \
> + [subject]=notmuch-reply-test \
> +'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> +'[body]="Reply from address in named group list"'
Same here.
Regards,
Dmitry
> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat +From: Notmuch Test Suite
> +To: Sender , someone at example.com
> +Subject: Re: notmuch-reply-test
> +In-Reply-To:
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender writes:
> +
> +> Reply from address in named group list
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> test_begin_subtest "Reply within emacs to a multipart/mixed message"
> test_emacs '(notmuch-show "id:20091118002059.067214ed at hikari")
> (notmuch-show-reply)
> --
> 1.7.5.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses
Adam Wolfe Gordon writes: > The bug was that notmuch-mua-mail used `mail-header` to check whether > it was passed a "From" header. The implementation of `mail-header` > must try to compare symbols instead of strings when looking for > headers, as it was returning nil when a From header was present. This > is probably because the mail functions construct headers as alists > with symbols for the header names, while our code uses strings for the > header names. > > Since we don't use `mail-header` anywhere else, and `message-mail` is > perfectly happy to accept string header names, the fix is just to use > `assoc` to look for the From header, so that the strings get compared > properly. > --- LGTM. Thanks for fixing this. Regards, Dmitry > emacs/notmuch-mua.el |4 ++-- > test/emacs |1 - > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el > index 6aae3a0..9805d79 100644 > --- a/emacs/notmuch-mua.el > +++ b/emacs/notmuch-mua.el > @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'." >(when (not (string= "" user-agent)) > (push (cons "User-Agent" user-agent) other-headers > > - (unless (mail-header 'From other-headers) > + (unless (assoc "From" other-headers) > (push (cons "From" (concat > (notmuch-user-name) " <" (notmuch-user-primary-email) > ">")) other-headers)) > > @@ -250,7 +250,7 @@ the From: address first." >(interactive "P") >(let ((other-headers >(when (or prompt-for-sender notmuch-always-prompt-for-sender) > -(list (cons 'From (notmuch-mua-prompt-for-sender)) > +(list (cons "From" (notmuch-mua-prompt-for-sender)) > (notmuch-mua-mail nil nil other-headers))) > > (defun notmuch-mua-new-forward-message (&optional prompt-for-sender) > diff --git a/test/emacs b/test/emacs > index fa5d706..08db1ee 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -275,7 +275,6 @@ EOF > test_expect_equal_file OUTPUT EXPECTED > > test_begin_subtest "Reply from alternate address within emacs" > -test_subtest_known_broken > add_message '[from]="Sender "' \ >[to]=test_suite_ot...@notmuchmail.org \ >[subject]=notmuch-reply-test \ > -- > 1.7.5.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
Re: [BUG/PATCH 1/2] test: Tests for reply from alternate addresses in emacs
Adam Wolfe Gordon writes:
> Since the recent reply changes were pushed, there has been a bug that
> causes emacs to always reply from the primary address, even if the
> JSON or default CLI reply output uses an alternate address.
>
> This adds two tests to the emacs test library based on the two "Reply
> form..." tests in the reply test library. One is currently marked
> broken.
> ---
> test/emacs | 52
> 1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 8a28705..fa5d706 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -274,6 +274,58 @@ Notmuch Test Suite writes:
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> +test_begin_subtest "Reply from alternate address within emacs"
> +test_subtest_known_broken
> +add_message '[from]="Sender "' \
> + [to]=test_suite_ot...@notmuchmail.org \
> + [subject]=notmuch-reply-test \
> + '[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> + '[body]="reply from alternate address"'
Please remove subject, data and body parameters. add_message would
provide sane default values.
> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat +From: Notmuch Test Suite
> +To: Sender
> +Subject: Re: notmuch-reply-test
> +In-Reply-To:
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender writes:
> +
> +> reply from alternate address
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "Reply from address in named group list within emacs"
> +add_message '[from]="Sender "' \
> +'[to]=group:test_su...@notmuchmail.org,some...@example.com\;' \
> + [cc]=test_suite_ot...@notmuchmail.org \
> + [subject]=notmuch-reply-test \
> +'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> +'[body]="Reply from address in named group list"'
Same here.
Regards,
Dmitry
> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat +From: Notmuch Test Suite
> +To: Sender , some...@example.com
> +Subject: Re: notmuch-reply-test
> +In-Reply-To:
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender writes:
> +
> +> Reply from address in named group list
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> test_begin_subtest "Reply within emacs to a multipart/mixed message"
> test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
> (notmuch-show-reply)
> --
> 1.7.5.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
[PATCH] emacs: get rid of trailing spaces in notmuch-hello view
This patch removes trailing spaces in notmuch-hello view. A side effect of this change is that tag/query buttons no longer include a space at the end. This means that pressing RET when the point is at the first character after the tag/query button no longer works (note that this is the standard behavior for buttons). We may change this behavior in the future (without adding trailing spaces back) if people would find this change inconvenient. --- emacs/notmuch-hello.el | 30 test/emacs |2 +- test/emacs.expected-output/notmuch-hello |2 +- .../emacs.expected-output/notmuch-hello-long-names |6 ++-- .../notmuch-hello-new-section |2 +- .../notmuch-hello-section-counts |4 +- .../notmuch-hello-section-hidden-tag |2 +- .../notmuch-hello-section-with-empty |2 +- .../emacs.expected-output/notmuch-hello-with-empty |2 +- 9 files changed, 23 insertions(+), 29 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index 28f39f1..17f7edd 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -429,7 +429,8 @@ Such a list can be computed with `notmuch-hello-query-counts'." (let* ((widest (notmuch-hello-longest-label searches)) (tags-and-width (notmuch-hello-tags-per-line widest)) (tags-per-line (car tags-and-width)) -(widest (cdr tags-and-width)) +(column-width (cdr tags-and-width)) +(column-indent 0) (count 0) (reordered-list (notmuch-hello-reflect searches tags-per-line)) ;; Hack the display of the buttons used. @@ -441,32 +442,25 @@ Such a list can be computed with `notmuch-hello-query-counts'." (mapc (lambda (elem) ;; (not elem) indicates an empty slot in the matrix. (when elem + (if (> column-indent 0) + (widget-insert (make-string column-indent ? ))) (let* ((name (first elem)) (query (second elem)) -(msg-count (third elem)) -(formatted-name (format "%s " name))) +(msg-count (third elem))) (widget-insert (format "%8s " (notmuch-hello-nice-number msg-count))) - (if (string= formatted-name notmuch-hello-target) + (if (string= name notmuch-hello-target) (setq found-target-pos (point-marker))) (widget-create 'push-button :notify #'notmuch-hello-widget-search :notmuch-search-terms query - formatted-name) - (unless (eq (% count tags-per-line) (1- tags-per-line)) - ;; If this is not the last tag on the line, insert - ;; enough space to consume the rest of the column. - ;; Because the button for the name is `(1+ (length - ;; name))' long (due to the trailing space) we can - ;; just insert `(- widest (length name))' spaces - the - ;; column separator is included in the button if - ;; `(equal widest (length name)'. - (widget-insert (make-string (max 0 - (- widest (length name))) - ? ) + name) + (setq column-indent + (1+ (max 0 (- column-width (length name))) (setq count (1+ count)) - (if (eq (% count tags-per-line) 0) - (widget-insert "\n"))) + (when (eq (% count tags-per-line) 0) + (setq column-indent 0) + (widget-insert "\n"))) reordered-list) ;; If the last line was not full (and hence did not include a diff --git a/test/emacs b/test/emacs index 29a489c..42780af 100755 --- a/test/emacs +++ b/test/emacs @@ -39,7 +39,7 @@ test_begin_subtest "Navigation of notmuch-hello to search results" test_emacs '(notmuch-hello) (goto-char (point-min)) (re-search-forward "inbox") - (widget-button-press (point)) + (widget-button-press (1- (point))) (notmuch-test-wait) (test-output)' test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-view-inbox diff --git a/test/emacs.expected-output/notmuch-hello b/test/emacs.expected-output/notmuch-hello index 1470790..2d69891 100644 --- a/test/emacs.expected-output/notmuch-hello +++ b/test/emacs.expected-output/notmuch-hello @@ -2,7 +2,7 @@ Saved searches: [edit] - 52 inbox 52 unread + 52 inbox 52 unread Search: . diff --git a/test/emacs.expected-output/notmuch-hello-lo
[PATCH v2 2/2] test: use subtest name for generated message subject by default
Before the change, messages generated by generate_message() used "Test
message #N" for default subject where N is the generated messages
counter. Since message subject is commonly present in expected
results, there is a chance of breaking other tests when a new
generate_message() call is added. The patch changes default subject
value for generated messages to subtest name if it is available. If
subtest name is not available (i.e. message is generated during test
initialization), the old default value is used (in this case it is
fine to have the counter in the subject).
Another benefit of this change is a sane default value for subject in
generated messages, which would allow to simplify code like:
test_begin_subtest "test for a cool feature"
add_message [subject]="message for test for a cool feature"
---
test/encoding|2 +-
test/search-folder-coherence |2 +-
test/test-lib.sh |6 +-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/test/encoding b/test/encoding
index 98abf77..2e1326e 100755
--- a/test/encoding
+++ b/test/encoding
@@ -9,7 +9,7 @@ output=$(notmuch show id:${gen_msg_id} 2>&1 |
notmuch_show_sanitize)
test_expect_equal "$output" " message{ id:msg-001 at notmuch-test-suite
depth:0 match:1 excluded:0 filename:/XXX/mail/msg-001
header{
Notmuch Test Suite (2001-01-05) (inbox unread)
-Subject: Test message #1
+Subject: Message with text of unknown charset
From: Notmuch Test Suite
To: Notmuch Test Suite
Date: Fri, 05 Jan 2001 15:43:57 +
diff --git a/test/search-folder-coherence b/test/search-folder-coherence
index f8119cb..3f6ec76 100755
--- a/test/search-folder-coherence
+++ b/test/search-folder-coherence
@@ -32,7 +32,7 @@ test_expect_equal_file OUTPUT EXPECTED
test_begin_subtest "Test matches folder:spam"
output=$(notmuch search folder:spam)
-test_expect_equal "$output" "thread:0001 2001-01-05 [1/1]
Notmuch Test Suite; Test message #1 (inbox unread)"
+test_expect_equal "$output" "thread:0001 2001-01-05 [1/1]
Notmuch Test Suite; Single new message (inbox unread)"
test_begin_subtest "Remove folder:spam copy of email"
rm $dir/spam/$(basename $file_x)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 2781506..06aaea2 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -318,7 +318,11 @@ generate_message ()
fi
if [ -z "${template[subject]}" ]; then
- template[subject]="Test message #${gen_msg_cnt}"
+ if [ -n "$test_subtest_name" ]; then
+ template[subject]="$test_subtest_name"
+ else
+ template[subject]="Test message #${gen_msg_cnt}"
+ fi
fi
if [ -z "${template[date]}" ]; then
--
1.7.9.1
[PATCH v2 1/2] test: remove "Generate some messages" test from raw
Before the change, the first subtest in raw format tests just generated messages and checked that they are added successfully. This is not really a raw format test, it is creating of environment required for other subtests to run. The patch removes the first subtest from raw and replaces it with bare add_message calls, similar to how it is done in other tests. TODO: we should check that test environment was created successfully. Currently, many tests do add_message(), notmuch new and other calls without checking the results. We should come up with a general solution for this, i.e. if any command during test initialization fails, all tests should be skipped with appropriate error message. --- test/raw |7 ++- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/raw b/test/raw index 0171e64..de0b867 100755 --- a/test/raw +++ b/test/raw @@ -3,11 +3,8 @@ test_description='notmuch show --format=raw' . ./test-lib.sh -test_begin_subtest "Generate some messages" -generate_message -generate_message -output=$(NOTMUCH_NEW) -test_expect_equal "$output" "Added 2 new messages to the database." +add_message +add_message test_begin_subtest "Attempt to show multiple raw messages" output=$(notmuch show --format=raw "*" 2>&1) -- 1.7.9.1
[PATCH v2 0/2] test: use subtest name for generated message subject
Changes in v2: * rebase on master Regards, Dmitry
[PATCH] emacs: get rid of trailing spaces in notmuch-hello view
This patch removes trailing spaces in notmuch-hello view. A side effect of this change is that tag/query buttons no longer include a space at the end. This means that pressing RET when the point is at the first character after the tag/query button no longer works (note that this is the standard behavior for buttons). We may change this behavior in the future (without adding trailing spaces back) if people would find this change inconvenient. --- emacs/notmuch-hello.el | 30 test/emacs |2 +- test/emacs.expected-output/notmuch-hello |2 +- .../emacs.expected-output/notmuch-hello-long-names |6 ++-- .../notmuch-hello-new-section |2 +- .../notmuch-hello-section-counts |4 +- .../notmuch-hello-section-hidden-tag |2 +- .../notmuch-hello-section-with-empty |2 +- .../emacs.expected-output/notmuch-hello-with-empty |2 +- 9 files changed, 23 insertions(+), 29 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index 28f39f1..17f7edd 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -429,7 +429,8 @@ Such a list can be computed with `notmuch-hello-query-counts'." (let* ((widest (notmuch-hello-longest-label searches)) (tags-and-width (notmuch-hello-tags-per-line widest)) (tags-per-line (car tags-and-width)) -(widest (cdr tags-and-width)) +(column-width (cdr tags-and-width)) +(column-indent 0) (count 0) (reordered-list (notmuch-hello-reflect searches tags-per-line)) ;; Hack the display of the buttons used. @@ -441,32 +442,25 @@ Such a list can be computed with `notmuch-hello-query-counts'." (mapc (lambda (elem) ;; (not elem) indicates an empty slot in the matrix. (when elem + (if (> column-indent 0) + (widget-insert (make-string column-indent ? ))) (let* ((name (first elem)) (query (second elem)) -(msg-count (third elem)) -(formatted-name (format "%s " name))) +(msg-count (third elem))) (widget-insert (format "%8s " (notmuch-hello-nice-number msg-count))) - (if (string= formatted-name notmuch-hello-target) + (if (string= name notmuch-hello-target) (setq found-target-pos (point-marker))) (widget-create 'push-button :notify #'notmuch-hello-widget-search :notmuch-search-terms query - formatted-name) - (unless (eq (% count tags-per-line) (1- tags-per-line)) - ;; If this is not the last tag on the line, insert - ;; enough space to consume the rest of the column. - ;; Because the button for the name is `(1+ (length - ;; name))' long (due to the trailing space) we can - ;; just insert `(- widest (length name))' spaces - the - ;; column separator is included in the button if - ;; `(equal widest (length name)'. - (widget-insert (make-string (max 0 - (- widest (length name))) - ? ) + name) + (setq column-indent + (1+ (max 0 (- column-width (length name))) (setq count (1+ count)) - (if (eq (% count tags-per-line) 0) - (widget-insert "\n"))) + (when (eq (% count tags-per-line) 0) + (setq column-indent 0) + (widget-insert "\n"))) reordered-list) ;; If the last line was not full (and hence did not include a diff --git a/test/emacs b/test/emacs index 29a489c..42780af 100755 --- a/test/emacs +++ b/test/emacs @@ -39,7 +39,7 @@ test_begin_subtest "Navigation of notmuch-hello to search results" test_emacs '(notmuch-hello) (goto-char (point-min)) (re-search-forward "inbox") - (widget-button-press (point)) + (widget-button-press (1- (point))) (notmuch-test-wait) (test-output)' test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-view-inbox diff --git a/test/emacs.expected-output/notmuch-hello b/test/emacs.expected-output/notmuch-hello index 1470790..2d69891 100644 --- a/test/emacs.expected-output/notmuch-hello +++ b/test/emacs.expected-output/notmuch-hello @@ -2,7 +2,7 @@ Saved searches: [edit] - 52 inbox 52 unread + 52 inbox 52 unread Search: . diff --git a/test/emacs.expected-output/notmuch-hel
[PATCH v2 2/2] test: use subtest name for generated message subject by default
Before the change, messages generated by generate_message() used "Test
message #N" for default subject where N is the generated messages
counter. Since message subject is commonly present in expected
results, there is a chance of breaking other tests when a new
generate_message() call is added. The patch changes default subject
value for generated messages to subtest name if it is available. If
subtest name is not available (i.e. message is generated during test
initialization), the old default value is used (in this case it is
fine to have the counter in the subject).
Another benefit of this change is a sane default value for subject in
generated messages, which would allow to simplify code like:
test_begin_subtest "test for a cool feature"
add_message [subject]="message for test for a cool feature"
---
test/encoding|2 +-
test/search-folder-coherence |2 +-
test/test-lib.sh |6 +-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/test/encoding b/test/encoding
index 98abf77..2e1326e 100755
--- a/test/encoding
+++ b/test/encoding
@@ -9,7 +9,7 @@ output=$(notmuch show id:${gen_msg_id} 2>&1 |
notmuch_show_sanitize)
test_expect_equal "$output" " message{ id:msg-001@notmuch-test-suite depth:0
match:1 excluded:0 filename:/XXX/mail/msg-001
header{
Notmuch Test Suite (2001-01-05) (inbox unread)
-Subject: Test message #1
+Subject: Message with text of unknown charset
From: Notmuch Test Suite
To: Notmuch Test Suite
Date: Fri, 05 Jan 2001 15:43:57 +
diff --git a/test/search-folder-coherence b/test/search-folder-coherence
index f8119cb..3f6ec76 100755
--- a/test/search-folder-coherence
+++ b/test/search-folder-coherence
@@ -32,7 +32,7 @@ test_expect_equal_file OUTPUT EXPECTED
test_begin_subtest "Test matches folder:spam"
output=$(notmuch search folder:spam)
-test_expect_equal "$output" "thread:0001 2001-01-05 [1/1]
Notmuch Test Suite; Test message #1 (inbox unread)"
+test_expect_equal "$output" "thread:0001 2001-01-05 [1/1]
Notmuch Test Suite; Single new message (inbox unread)"
test_begin_subtest "Remove folder:spam copy of email"
rm $dir/spam/$(basename $file_x)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 2781506..06aaea2 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -318,7 +318,11 @@ generate_message ()
fi
if [ -z "${template[subject]}" ]; then
- template[subject]="Test message #${gen_msg_cnt}"
+ if [ -n "$test_subtest_name" ]; then
+ template[subject]="$test_subtest_name"
+ else
+ template[subject]="Test message #${gen_msg_cnt}"
+ fi
fi
if [ -z "${template[date]}" ]; then
--
1.7.9.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/2] test: remove "Generate some messages" test from raw
Before the change, the first subtest in raw format tests just generated messages and checked that they are added successfully. This is not really a raw format test, it is creating of environment required for other subtests to run. The patch removes the first subtest from raw and replaces it with bare add_message calls, similar to how it is done in other tests. TODO: we should check that test environment was created successfully. Currently, many tests do add_message(), notmuch new and other calls without checking the results. We should come up with a general solution for this, i.e. if any command during test initialization fails, all tests should be skipped with appropriate error message. --- test/raw |7 ++- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/raw b/test/raw index 0171e64..de0b867 100755 --- a/test/raw +++ b/test/raw @@ -3,11 +3,8 @@ test_description='notmuch show --format=raw' . ./test-lib.sh -test_begin_subtest "Generate some messages" -generate_message -generate_message -output=$(NOTMUCH_NEW) -test_expect_equal "$output" "Added 2 new messages to the database." +add_message +add_message test_begin_subtest "Attempt to show multiple raw messages" output=$(notmuch show --format=raw "*" 2>&1) -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 0/2] test: use subtest name for generated message subject
Changes in v2: * rebase on master Regards, Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] emacs: fix off-by-one error in notmuch-hello column alignment
Expected results for few tests are fixed, the relevant test is unmarked broken. --- emacs/notmuch-hello.el |2 +- test/emacs-hello |1 - .../notmuch-hello-new-section |2 +- .../notmuch-hello-section-with-empty |2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index e9caade..28f39f1 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -461,7 +461,7 @@ Such a list can be computed with `notmuch-hello-query-counts'." ;; just insert `(- widest (length name))' spaces - the ;; column separator is included in the button if ;; `(equal widest (length name)'. - (widget-insert (make-string (max 1 + (widget-insert (make-string (max 0 (- widest (length name))) ? ) (setq count (1+ count)) diff --git a/test/emacs-hello b/test/emacs-hello index 9e5d045..be66ba4 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -45,7 +45,6 @@ test_emacs "(let ((notmuch-hello-sections test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts test_begin_subtest "Column alignment for tag/queries with long names" -test_subtest_known_broken tag=a-very-long-tag # length carefully calculated for 80 characters window width notmuch tag +$tag '*' test_emacs '(notmuch-hello) diff --git a/test/emacs.expected-output/notmuch-hello-new-section b/test/emacs.expected-output/notmuch-hello-new-section index c64d712..6a339aa 100644 --- a/test/emacs.expected-output/notmuch-hello-new-section +++ b/test/emacs.expected-output/notmuch-hello-new-section @@ -1,4 +1,4 @@ Test: [hide] - 52 inbox + 52 inbox diff --git a/test/emacs.expected-output/notmuch-hello-section-with-empty b/test/emacs.expected-output/notmuch-hello-section-with-empty index 8209fed..dc2568d 100644 --- a/test/emacs.expected-output/notmuch-hello-section-with-empty +++ b/test/emacs.expected-output/notmuch-hello-section-with-empty @@ -1,4 +1,4 @@ Test-with-empty: [hide] - 52 inbox + 52 inbox -- 1.7.9.1
[PATCH 1/2] test: add broken test for long names in Emacs notmuch-hello view
Currently, the column alignment in Emacs notmuch-hello is broken for tags/queries with long names. --- test/emacs-hello |9 + .../emacs.expected-output/notmuch-hello-long-names | 18 ++ 2 files changed, 27 insertions(+), 0 deletions(-) create mode 100644 test/emacs.expected-output/notmuch-hello-long-names diff --git a/test/emacs-hello b/test/emacs-hello index b235e3a..9e5d045 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -44,4 +44,13 @@ test_emacs "(let ((notmuch-hello-sections (test-output))" test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts +test_begin_subtest "Column alignment for tag/queries with long names" +test_subtest_known_broken +tag=a-very-long-tag # length carefully calculated for 80 characters window width +notmuch tag +$tag '*' +test_emacs '(notmuch-hello) +(test-output)' +notmuch tag -$tag '*' +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-long-names + test_done diff --git a/test/emacs.expected-output/notmuch-hello-long-names b/test/emacs.expected-output/notmuch-hello-long-names new file mode 100644 index 000..be6d2c5 --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-long-names @@ -0,0 +1,18 @@ + Welcome to notmuch. You have 52 messages. + +Saved searches: [edit] + + 52 inbox 52 unread + +Search: . + +All tags: [hide] + + 52 a-very-long-tag 52 inbox 52 unread + 4 attachment 7 signed + +Type a search query and hit RET to view matching threads. + Edit saved searches with the `edit' button. + Hit RET or click on a saved search or tag name to view matching threads. + `=' to refresh this screen. `s' to search messages. `q' to quit. + Customize this page. -- 1.7.9.1
[PATCH 2/2] emacs: fix off-by-one error in notmuch-hello column alignment
Expected results for few tests are fixed, the relevant test is unmarked broken. --- emacs/notmuch-hello.el |2 +- test/emacs-hello |1 - .../notmuch-hello-new-section |2 +- .../notmuch-hello-section-with-empty |2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index e9caade..28f39f1 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -461,7 +461,7 @@ Such a list can be computed with `notmuch-hello-query-counts'." ;; just insert `(- widest (length name))' spaces - the ;; column separator is included in the button if ;; `(equal widest (length name)'. - (widget-insert (make-string (max 1 + (widget-insert (make-string (max 0 (- widest (length name))) ? ) (setq count (1+ count)) diff --git a/test/emacs-hello b/test/emacs-hello index 9e5d045..be66ba4 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -45,7 +45,6 @@ test_emacs "(let ((notmuch-hello-sections test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts test_begin_subtest "Column alignment for tag/queries with long names" -test_subtest_known_broken tag=a-very-long-tag # length carefully calculated for 80 characters window width notmuch tag +$tag '*' test_emacs '(notmuch-hello) diff --git a/test/emacs.expected-output/notmuch-hello-new-section b/test/emacs.expected-output/notmuch-hello-new-section index c64d712..6a339aa 100644 --- a/test/emacs.expected-output/notmuch-hello-new-section +++ b/test/emacs.expected-output/notmuch-hello-new-section @@ -1,4 +1,4 @@ Test: [hide] - 52 inbox + 52 inbox diff --git a/test/emacs.expected-output/notmuch-hello-section-with-empty b/test/emacs.expected-output/notmuch-hello-section-with-empty index 8209fed..dc2568d 100644 --- a/test/emacs.expected-output/notmuch-hello-section-with-empty +++ b/test/emacs.expected-output/notmuch-hello-section-with-empty @@ -1,4 +1,4 @@ Test-with-empty: [hide] - 52 inbox + 52 inbox -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test: add broken test for long names in Emacs notmuch-hello view
Currently, the column alignment in Emacs notmuch-hello is broken for tags/queries with long names. --- test/emacs-hello |9 + .../emacs.expected-output/notmuch-hello-long-names | 18 ++ 2 files changed, 27 insertions(+), 0 deletions(-) create mode 100644 test/emacs.expected-output/notmuch-hello-long-names diff --git a/test/emacs-hello b/test/emacs-hello index b235e3a..9e5d045 100755 --- a/test/emacs-hello +++ b/test/emacs-hello @@ -44,4 +44,13 @@ test_emacs "(let ((notmuch-hello-sections (test-output))" test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts +test_begin_subtest "Column alignment for tag/queries with long names" +test_subtest_known_broken +tag=a-very-long-tag # length carefully calculated for 80 characters window width +notmuch tag +$tag '*' +test_emacs '(notmuch-hello) +(test-output)' +notmuch tag -$tag '*' +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-long-names + test_done diff --git a/test/emacs.expected-output/notmuch-hello-long-names b/test/emacs.expected-output/notmuch-hello-long-names new file mode 100644 index 000..be6d2c5 --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-long-names @@ -0,0 +1,18 @@ + Welcome to notmuch. You have 52 messages. + +Saved searches: [edit] + + 52 inbox 52 unread + +Search: . + +All tags: [hide] + + 52 a-very-long-tag 52 inbox 52 unread + 4 attachment 7 signed + +Type a search query and hit RET to view matching threads. + Edit saved searches with the `edit' button. + Hit RET or click on a saved search or tag name to view matching threads. + `=' to refresh this screen. `s' to search messages. `q' to quit. + Customize this page. -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function
On Mon, 5 Mar 2012 22:55:54 +0200, Jani Nikula wrote: > On Mar 5, 2012 5:43 PM, "Dmitry Kurochkin" > wrote: > > > > On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe > wrote: > > > On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin < > dmitry.kurochkin at gmail.com> wrote: > > > > On Thu, 1 Mar 2012 21:24:38 +0100, Daniel Schoepe > > > schoepe.org> > wrote: > > > > > notmuch-saved-search-sort-function might destructively modify its > > > > > input (`sort' does that, for instance), so it should not be given > > > > > notmuch-saved-searches directly. > > > > > --- > > > > > > > > -1 > > > > > > > > I think we should require `notmuch-saved-search-sort-function' not to > > > > have side effects. Current documentation should be more clear about > > > > this. We need to fix `notmuch-sort-saved-searches' to copy the list > > > > before calling `sort'. But we should not do it in > > > > `notmuch-hello-insert-saved-searches' for any sorting function (which > > > > may not need this copying). > > > > > > My reasoning was that since sort is such a common function, many users > > > will probably use sort for their own sorting functions, not realizing > > > that it has side effects. This will lead to confusing behavior that's > > > not so easy to track down. > > > > > > Copying the list of saved searches when running notmuch-hello does not > > > seem be relevant to performance to me, since it's a) not called that > > > often and b) the list of saved searches will rarely exceed 30 elements. > > > > > > Hence, this way we can avoid some headaches for users who define their > > > own sorting functions at a negligible (performance) cost. Incidentally, > > > this is also how notmuch-hello did it before the user-defined sections > > > patches. > > > > > > > I do not buy the argument that we should help users who implement their > > own sorting functions but do not read documentation for functions they > > use. Apparently, those who implemented the `sort' function had similar > > ideas. And I do not think it is our job to add workarounds for it. > > > > An alternative (and IMO better) solution would be to allow customization > > of compare function used for sorting instead of the sorting function > > itself. > > Providing the customization of the sort function is more powerful than the > compare function. In the case of saved searches I can imagine people might > want to partially use the original order while sort the rest (e.g. > important ones first in predefined order, others sorted). Valid point. > In fact this also > allows dropping out some elements. And renaming. And changing the queries... > > (I had something like that in mind originally but then settled with just > capitalizing the important ones to show them first.) > All of these are invalid usages of `notmuch-saved-search-sort-function'. The function is meant for sorting only (hence the name). So the code might assume that the function does only sorting. I do not understand why we need such functionality (renaming, capitalizing, etc.). You can just rename the query itself if you want to. Should be easier IMO. But if we need such functionality, we should not misuse sorting function for it. We can add `notmuch-saved-searches' function which would return saved searches list (sorted, renamed and mangled in any other way). By default it would return `notmuch-saved-searches' variable as is. Regards, Dmitry > BR, > Jani. > > > > > Regards, > > Dmitry > > > > > Cheers, > > > Daniel > > ___ > > notmuch mailing list > > notmuch at notmuchmail.org > > http://notmuchmail.org/mailman/listinfo/notmuch Non-text part: text/html
[PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function
On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe wrote: > On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin gmail.com> wrote: > > On Thu, 1 Mar 2012 21:24:38 +0100, Daniel Schoepe > > wrote: > > > notmuch-saved-search-sort-function might destructively modify its > > > input (`sort' does that, for instance), so it should not be given > > > notmuch-saved-searches directly. > > > --- > > > > -1 > > > > I think we should require `notmuch-saved-search-sort-function' not to > > have side effects. Current documentation should be more clear about > > this. We need to fix `notmuch-sort-saved-searches' to copy the list > > before calling `sort'. But we should not do it in > > `notmuch-hello-insert-saved-searches' for any sorting function (which > > may not need this copying). > > My reasoning was that since sort is such a common function, many users > will probably use sort for their own sorting functions, not realizing > that it has side effects. This will lead to confusing behavior that's > not so easy to track down. > > Copying the list of saved searches when running notmuch-hello does not > seem be relevant to performance to me, since it's a) not called that > often and b) the list of saved searches will rarely exceed 30 elements. > > Hence, this way we can avoid some headaches for users who define their > own sorting functions at a negligible (performance) cost. Incidentally, > this is also how notmuch-hello did it before the user-defined sections > patches. > I do not buy the argument that we should help users who implement their own sorting functions but do not read documentation for functions they use. Apparently, those who implemented the `sort' function had similar ideas. And I do not think it is our job to add workarounds for it. An alternative (and IMO better) solution would be to allow customization of compare function used for sorting instead of the sorting function itself. Regards, Dmitry > Cheers, > Daniel
Re: [PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function
On Mon, 5 Mar 2012 22:55:54 +0200, Jani Nikula wrote: > On Mar 5, 2012 5:43 PM, "Dmitry Kurochkin" > wrote: > > > > On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe > wrote: > > > On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin < > dmitry.kuroch...@gmail.com> wrote: > > > > On Thu, 1 Mar 2012 21:24:38 +0100, Daniel Schoepe > wrote: > > > > > notmuch-saved-search-sort-function might destructively modify its > > > > > input (`sort' does that, for instance), so it should not be given > > > > > notmuch-saved-searches directly. > > > > > --- > > > > > > > > -1 > > > > > > > > I think we should require `notmuch-saved-search-sort-function' not to > > > > have side effects. Current documentation should be more clear about > > > > this. We need to fix `notmuch-sort-saved-searches' to copy the list > > > > before calling `sort'. But we should not do it in > > > > `notmuch-hello-insert-saved-searches' for any sorting function (which > > > > may not need this copying). > > > > > > My reasoning was that since sort is such a common function, many users > > > will probably use sort for their own sorting functions, not realizing > > > that it has side effects. This will lead to confusing behavior that's > > > not so easy to track down. > > > > > > Copying the list of saved searches when running notmuch-hello does not > > > seem be relevant to performance to me, since it's a) not called that > > > often and b) the list of saved searches will rarely exceed 30 elements. > > > > > > Hence, this way we can avoid some headaches for users who define their > > > own sorting functions at a negligible (performance) cost. Incidentally, > > > this is also how notmuch-hello did it before the user-defined sections > > > patches. > > > > > > > I do not buy the argument that we should help users who implement their > > own sorting functions but do not read documentation for functions they > > use. Apparently, those who implemented the `sort' function had similar > > ideas. And I do not think it is our job to add workarounds for it. > > > > An alternative (and IMO better) solution would be to allow customization > > of compare function used for sorting instead of the sorting function > > itself. > > Providing the customization of the sort function is more powerful than the > compare function. In the case of saved searches I can imagine people might > want to partially use the original order while sort the rest (e.g. > important ones first in predefined order, others sorted). Valid point. > In fact this also > allows dropping out some elements. And renaming. And changing the queries... > > (I had something like that in mind originally but then settled with just > capitalizing the important ones to show them first.) > All of these are invalid usages of `notmuch-saved-search-sort-function'. The function is meant for sorting only (hence the name). So the code might assume that the function does only sorting. I do not understand why we need such functionality (renaming, capitalizing, etc.). You can just rename the query itself if you want to. Should be easier IMO. But if we need such functionality, we should not misuse sorting function for it. We can add `notmuch-saved-searches' function which would return saved searches list (sorted, renamed and mangled in any other way). By default it would return `notmuch-saved-searches' variable as is. Regards, Dmitry > BR, > Jani. > > > > > Regards, > > Dmitry > > > > > Cheers, > > > Daniel > > ___ > > notmuch mailing list > > notmuch@notmuchmail.org > > http://notmuchmail.org/mailman/listinfo/notmuch Non-text part: text/html ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function
On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe wrote: > On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin > wrote: > > On Thu, 1 Mar 2012 21:24:38 +0100, Daniel Schoepe > > wrote: > > > notmuch-saved-search-sort-function might destructively modify its > > > input (`sort' does that, for instance), so it should not be given > > > notmuch-saved-searches directly. > > > --- > > > > -1 > > > > I think we should require `notmuch-saved-search-sort-function' not to > > have side effects. Current documentation should be more clear about > > this. We need to fix `notmuch-sort-saved-searches' to copy the list > > before calling `sort'. But we should not do it in > > `notmuch-hello-insert-saved-searches' for any sorting function (which > > may not need this copying). > > My reasoning was that since sort is such a common function, many users > will probably use sort for their own sorting functions, not realizing > that it has side effects. This will lead to confusing behavior that's > not so easy to track down. > > Copying the list of saved searches when running notmuch-hello does not > seem be relevant to performance to me, since it's a) not called that > often and b) the list of saved searches will rarely exceed 30 elements. > > Hence, this way we can avoid some headaches for users who define their > own sorting functions at a negligible (performance) cost. Incidentally, > this is also how notmuch-hello did it before the user-defined sections > patches. > I do not buy the argument that we should help users who implement their own sorting functions but do not read documentation for functions they use. Apparently, those who implemented the `sort' function had similar ideas. And I do not think it is our job to add workarounds for it. An alternative (and IMO better) solution would be to allow customization of compare function used for sorting instead of the sorting function itself. Regards, Dmitry > Cheers, > Daniel ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function
On Thu, 1 Mar 2012 21:24:38 +0100, Daniel Schoepe wrote: > notmuch-saved-search-sort-function might destructively modify its > input (`sort' does that, for instance), so it should not be given > notmuch-saved-searches directly. > --- -1 I think we should require `notmuch-saved-search-sort-function' not to have side effects. Current documentation should be more clear about this. We need to fix `notmuch-sort-saved-searches' to copy the list before calling `sort'. But we should not do it in `notmuch-hello-insert-saved-searches' for any sorting function (which may not need this copying). Regards, Dmitry > emacs/notmuch-hello.el |5 - > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index aad373d..e089290 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -575,7 +575,10 @@ Complete list of currently available key bindings: >(let ((searches (notmuch-hello-query-counts > (if notmuch-saved-search-sort-function > (funcall notmuch-saved-search-sort-function > - notmuch-saved-searches) > + ;; Use a copy, since the sorting > + ;; function may have side effects, > + ;; e.g. if it just `sort's the input. > + (copy-sequence notmuch-saved-searches)) >notmuch-saved-searches) > :show-empty-searches notmuch-show-empty-saved-searches)) > found-target-pos) > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] emacs-hello: Do not calculate the count of the messages in hidden sections
Hi Michal. On Sat, 18 Feb 2012 23:12:29 +0100, Michal Sojka wrote: > The result is that hello screen shows much faster when some sections are > hidden. > --- The patch looks good to me. Please do not send new patches as replies to other patch threads. It makes it difficult to track, especially when there are multiple versions. You can always add an id: reference to the related thread. Regards, Dmitry > emacs/notmuch-hello.el | 20 ++-- > 1 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index aad373d..e9caade 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -695,16 +695,16 @@ Supports the following entries in OPTIONS as a plist: > (notmuch-hello-update)) >"hide")) > (widget-insert "\n") > -(let (target-pos > - (searches (apply 'notmuch-hello-query-counts query-alist options))) > - (when (and (not is-hidden) > -(or (not (plist-get options :hide-if-empty)) > - searches)) > - (widget-insert "\n") > - (setq target-pos > - (notmuch-hello-insert-buttons searches)) > - (indent-rigidly start (point) notmuch-hello-indent) > - target-pos > +(let (target-pos) > + (when (not is-hidden) > + (let ((searches (apply 'notmuch-hello-query-counts query-alist > options))) > + (when (or (not (plist-get options :hide-if-empty)) > + searches) > + (widget-insert "\n") > + (setq target-pos > + (notmuch-hello-insert-buttons searches)) > + (indent-rigidly start (point) notmuch-hello-indent > + target-pos))) > > (defun notmuch-hello-insert-tags-section (&optional title &rest options) >"Insert a section displaying all tags with message counts. > -- > 1.7.7.3 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function
On Thu, 1 Mar 2012 21:24:38 +0100, Daniel Schoepe wrote: > notmuch-saved-search-sort-function might destructively modify its > input (`sort' does that, for instance), so it should not be given > notmuch-saved-searches directly. > --- -1 I think we should require `notmuch-saved-search-sort-function' not to have side effects. Current documentation should be more clear about this. We need to fix `notmuch-sort-saved-searches' to copy the list before calling `sort'. But we should not do it in `notmuch-hello-insert-saved-searches' for any sorting function (which may not need this copying). Regards, Dmitry > emacs/notmuch-hello.el |5 - > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index aad373d..e089290 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -575,7 +575,10 @@ Complete list of currently available key bindings: >(let ((searches (notmuch-hello-query-counts > (if notmuch-saved-search-sort-function > (funcall notmuch-saved-search-sort-function > - notmuch-saved-searches) > + ;; Use a copy, since the sorting > + ;; function may have side effects, > + ;; e.g. if it just `sort's the input. > + (copy-sequence notmuch-saved-searches)) >notmuch-saved-searches) > :show-empty-searches notmuch-show-empty-saved-searches)) > found-target-pos) > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs-hello: Do not calculate the count of the messages in hidden sections
Hi Michal. On Sat, 18 Feb 2012 23:12:29 +0100, Michal Sojka wrote: > The result is that hello screen shows much faster when some sections are > hidden. > --- The patch looks good to me. Please do not send new patches as replies to other patch threads. It makes it difficult to track, especially when there are multiple versions. You can always add an id: reference to the related thread. Regards, Dmitry > emacs/notmuch-hello.el | 20 ++-- > 1 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index aad373d..e9caade 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -695,16 +695,16 @@ Supports the following entries in OPTIONS as a plist: > (notmuch-hello-update)) >"hide")) > (widget-insert "\n") > -(let (target-pos > - (searches (apply 'notmuch-hello-query-counts query-alist options))) > - (when (and (not is-hidden) > -(or (not (plist-get options :hide-if-empty)) > - searches)) > - (widget-insert "\n") > - (setq target-pos > - (notmuch-hello-insert-buttons searches)) > - (indent-rigidly start (point) notmuch-hello-indent) > - target-pos > +(let (target-pos) > + (when (not is-hidden) > + (let ((searches (apply 'notmuch-hello-query-counts query-alist > options))) > + (when (or (not (plist-get options :hide-if-empty)) > + searches) > + (widget-insert "\n") > + (setq target-pos > + (notmuch-hello-insert-buttons searches)) > + (indent-rigidly start (point) notmuch-hello-indent > + target-pos))) > > (defun notmuch-hello-insert-tags-section (&optional title &rest options) >"Insert a section displaying all tags with message counts. > -- > 1.7.7.3 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test: remove "Generate some messages" test from raw
Hi David. On Wed, 15 Feb 2012 22:33:51 +0400, Dmitry Kurochkin wrote: > Hi all. > > Are there any objections to these changes? > > Otherwise, I would ask David to push it as "minor, boring and trivial" > changes. > Since there are no objections, I removed the needs-review tag from these patches. David, I believe the patches are good to be pushed. Regards, Dmitry > Regards, > Dmitry
[PATCH v10 1/2] emacs: User-defined sections in notmuch-hello
Hi Michal. On Thu, 01 Mar 2012 15:57:32 +0100, Michal Sojka wrote: > On Thu, 01 Mar 2012, David Bremner wrote: > > > > Pushed, finally. Thanks for both of your hard work on this. > > Hi, > > is anybody willing to review > id:"1329603149-6047-1-git-send-email-sojkam1 at fel.cvut.cz"? I plan to review it soon. Regards, Dmitry > I'd like to > see it merged togetger with these patches as it makes my work with > notmuch-hello much more pleasant. And others > (id:"8762f7etfo.fsf at qmul.ac.uk") would probably like this patch too. > > -Michal
Re: [PATCH 1/2] test: remove "Generate some messages" test from raw
Hi David. On Wed, 15 Feb 2012 22:33:51 +0400, Dmitry Kurochkin wrote: > Hi all. > > Are there any objections to these changes? > > Otherwise, I would ask David to push it as "minor, boring and trivial" > changes. > Since there are no objections, I removed the needs-review tag from these patches. David, I believe the patches are good to be pushed. Regards, Dmitry > Regards, > Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v10 1/2] emacs: User-defined sections in notmuch-hello
Hi Michal. On Thu, 01 Mar 2012 15:57:32 +0100, Michal Sojka wrote: > On Thu, 01 Mar 2012, David Bremner wrote: > > > > Pushed, finally. Thanks for both of your hard work on this. > > Hi, > > is anybody willing to review > id:"1329603149-6047-1-git-send-email-sojk...@fel.cvut.cz"? I plan to review it soon. Regards, Dmitry > I'd like to > see it merged togetger with these patches as it makes my work with > notmuch-hello much more pleasant. And others > (id:"8762f7etfo@qmul.ac.uk") would probably like this patch too. > > -Michal ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 6/6] emacs: `notmuch-show-tag-all' sans prefix arg only tags open messages
On Fri, 24 Feb 2012 00:09:14 +0100, Pieter Praet wrote: > * emacs/notmuch-show.el > > (notmuch-show-get-messages-ids): > New optional argument ONLY-OPEN. If non-nil, only return > Message-Id's for messages which are currently visible. > > (notmuch-show-tag-all): > New optional argument IGNORE-VISIBILITY, of which the inverse is > passed as ONLY-OPEN argument to `notmuch-show-get-messages-ids': > If called with a prefix arg, affect *all* messages in the current > buffer. Otherwise, only change tags of visible messages. > > (notmuch-show-archive-thread): > Update wrt changes to `notmuch-show-tag-all'. > > * test/emacs > > - Subtest "notmuch-show: change tags of open messages in current buffer" > is no longer broken. > --- > emacs/notmuch-show.el | 28 > test/emacs|1 - > 2 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 4b37c77..4499fcd 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1334,13 +1334,17 @@ (defun notmuch-show-get-message-id () >"Return the message id of the current message." >(concat "id:\"" (notmuch-show-get-prop :id) "\"")) > > -(defun notmuch-show-get-messages-ids (&optional separator) > +(defun notmuch-show-get-messages-ids (&optional only-open separator) >"Return a list of Message-Id's of all messages in the current buffer. > > +If optional argument ONLY-OPEN is non-nil, only return > +Message-Id's for messages which are currently visible. > + > If provided with optional argument SEPARATOR, return a string > instead, consisting of all Message-Id's separated by SEPARATOR." >(let ((message-ids)) > -(notmuch-show-mapc t > +(notmuch-show-mapc > + `(if only-open (notmuch-show-message-visible-p) t) How about changing ONLY-OPEN to a general optional PREDICATE argument and pass it as is to `notmuch-show-mapc'? Please make it the last argument. Regards, Dmitry > (lambda () (push (notmuch-show-get-message-id) message-ids))) > (if separator > (mapconcat 'identity message-ids separator) > @@ -1628,13 +1632,21 @@ (defun notmuch-show-tag (&optional initial-input) > initial-input (notmuch-show-get-message-id > (apply 'notmuch-show-tag-message tag-changes))) > > -(defun notmuch-show-tag-all (&rest tag-changes) > - "Change tags for all messages in the current thread. > +(defun notmuch-show-tag-all (&optional ignore-visibility &rest tag-changes) > + "Change tags of all open messages in the current buffer. > + > +If optional arg IGNORE-VISIBILITY is non-nil, change tags of > +*all* messages in the current buffer, independent of their > +visibility. > > TAG-CHANGES is a list of tag operations for `notmuch-tag'." > - (interactive (notmuch-read-tag-changes nil notmuch-show-thread-id)) > - (apply 'notmuch-tag (notmuch-show-get-messages-ids " or ") tag-changes) > - (notmuch-show-mapc t > + (interactive (cons current-prefix-arg > + (notmuch-read-tag-changes nil notmuch-show-thread-id))) > + (apply 'notmuch-tag > + (notmuch-show-get-messages-ids (not ignore-visibility) " or ") > + tag-changes) > + (notmuch-show-mapc > + `(if ignore-visibility t (notmuch-show-message-visible-p)) > (lambda () > (let* ((current-tags (notmuch-show-get-tags)) > (new-tags (notmuch-update-tags current-tags tag-changes))) > @@ -1719,7 +1731,7 @@ (defun notmuch-show-archive-thread (&optional unarchive) > buffer." >(interactive "P") >(let ((op (if unarchive "+" "-"))) > -(notmuch-show-tag-all (concat op "inbox" > +(notmuch-show-tag-all t (concat op "inbox" > > (defun notmuch-show-archive-thread-then-next () >"Archive each message in thread, then show next thread from search." > diff --git a/test/emacs b/test/emacs > index 644ef59..c286ff5 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -152,7 +152,6 @@ notmuch tag +"$del_tag" -"$add_tag" -- "$query" # revert > tag changes > test_expect_equal "$count_changed" "$count_total" # assert that CHANGED == > TOTAL > > test_begin_subtest "notmuch-show: change tags of open messages in current > buffer" > -test_subtest_known_broken > query="$os_x_darwin_thread" > filter="from:Jiang" > add_tag="notmuch-show-tag-all" > -- > 1.7.8.1 >
[PATCH 5/6] emacs: simplify `notmuch-show-get-messages-ids{, -search}'
On Fri, 24 Feb 2012 00:09:13 +0100, Pieter Praet wrote: > * emacs/notmuch-show.el > > (notmuch-show-get-messages-ids): > If provided with optional arg SEPARATOR, return a string consisting > of all Message-Id's, separated by SEPARATOR. Also improve original > docstring wrt default return value. > > (notmuch-show-get-messages-ids-search): > Removed, as its functionality is now in `notmuch-show-get-messages-ids'. > > (notmuch-show-tag-all): > Call `notmuch-show-get-messages-ids' with SEPARATOR arg instead of > `notmuch-show-get-messages-ids-search'. > --- There is another similar case in notmuch.el: `notmuch-show-get-messages-ids' and `notmuch-show-get-messages-ids-search'. There may be more. Please change them as well. Regards, Dmitry > emacs/notmuch-show.el | 18 +- > 1 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 5fc0e43..4b37c77 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1334,17 +1334,17 @@ (defun notmuch-show-get-message-id () >"Return the message id of the current message." >(concat "id:\"" (notmuch-show-get-prop :id) "\"")) > > -(defun notmuch-show-get-messages-ids () > - "Return all message ids of messages in the current thread." > +(defun notmuch-show-get-messages-ids (&optional separator) > + "Return a list of Message-Id's of all messages in the current buffer. > + > +If provided with optional argument SEPARATOR, return a string > +instead, consisting of all Message-Id's separated by SEPARATOR." >(let ((message-ids)) > (notmuch-show-mapc t > (lambda () (push (notmuch-show-get-message-id) message-ids))) > -message-ids)) > - > -(defun notmuch-show-get-messages-ids-search () > - "Return a search string for all message ids of messages in the > -current thread." > - (mapconcat 'identity (notmuch-show-get-messages-ids) " or ")) > +(if separator > + (mapconcat 'identity message-ids separator) > + message-ids))) > > ;; dme: Would it make sense to use a macro for many of these? > > @@ -1633,7 +1633,7 @@ (defun notmuch-show-tag-all (&rest tag-changes) > > TAG-CHANGES is a list of tag operations for `notmuch-tag'." >(interactive (notmuch-read-tag-changes nil notmuch-show-thread-id)) > - (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes) > + (apply 'notmuch-tag (notmuch-show-get-messages-ids " or ") tag-changes) >(notmuch-show-mapc t > (lambda () > (let* ((current-tags (notmuch-show-get-tags)) > -- > 1.7.8.1 >
[PATCH 4/6] emacs: add predicate arg to `notmuch-show-mapc'
On Fri, 24 Feb 2012 00:09:12 +0100, Pieter Praet wrote: > * emacs/notmuch-show.el > > (notmuch-show-mapc): > Only call FUNCTION if new argument PREDICATE is satisfied. > Also correct original docstring: 's/thread/buffer/'. > > (notmuch-show-get-messages-ids): > Update wrt changes to `notmuch-show-mapc'. > > (notmuch-show-tag-all): > Update wrt changes to `notmuch-show-mapc'. > --- > emacs/notmuch-show.el | 15 --- > 1 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index aa9ccee..5fc0e43 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1272,13 +1272,14 @@ (defun notmuch-show-goto-message-previous () > (notmuch-show-move-to-message-top) > t)) > > -(defun notmuch-show-mapc (function) > - "Iterate through all messages in the current thread with > -`notmuch-show-goto-message-next' and call FUNCTION for side > -effects." > +(defun notmuch-show-mapc (predicate function) Please make PREDICATE optional to avoid giving t when it is not needed. I would expect PREDICATE to be a function, but perhaps the way you implemented it is common in Emacs. Regards, Dmitry > + "Iterate through all messages in the current buffer with > +`notmuch-show-goto-message-next'. If PREDICATE is satisfied, > +call FUNCTION for side effects." >(save-excursion > (goto-char (point-min)) > -(loop do (funcall function) > +(loop do (if (eval predicate) > + (funcall function)) > while (notmuch-show-goto-message-next > > ;; Functions relating to the visibility of messages and their > @@ -1336,7 +1337,7 @@ (defun notmuch-show-get-message-id () > (defun notmuch-show-get-messages-ids () >"Return all message ids of messages in the current thread." >(let ((message-ids)) > -(notmuch-show-mapc > +(notmuch-show-mapc t > (lambda () (push (notmuch-show-get-message-id) message-ids))) > message-ids)) > > @@ -1633,7 +1634,7 @@ (defun notmuch-show-tag-all (&rest tag-changes) > TAG-CHANGES is a list of tag operations for `notmuch-tag'." >(interactive (notmuch-read-tag-changes nil notmuch-show-thread-id)) >(apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes) > - (notmuch-show-mapc > + (notmuch-show-mapc t > (lambda () > (let* ((current-tags (notmuch-show-get-tags)) > (new-tags (notmuch-update-tags current-tags tag-changes))) > -- > 1.7.8.1 >
[PATCH 1/6] test: emacs: new tests "notmuch-show: {add, remove} multiple tags {to, from} single message"
On Fri, 24 Feb 2012 00:09:09 +0100, Pieter Praet wrote:
> * test/emacs:
>
> - Rename subtests "{Add,Remove} tag from notmuch-show view" to
> "notmuch-show: {add,remove} single tag {to,from} single message"
> to be consistent with the following tests.
>
> - New subtest "notmuch-show: add multiple tags to single message":
> `notmuch-show-add-tag' ("+") can add multiple tags to a message.
>
> - New subtest "notmuch-show: remove multiple tags from single message":
> `notmuch-show-remove-tag' ("-") can remove multiple tags from a message.
> ---
Would be nice to have another patch that moves notmuch-show tests to
emacs-show file.
Regards,
Dmitry
> test/emacs | 16 ++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index b74cfa9..ec1dbb0 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -112,18 +112,30 @@ test_emacs "(notmuch-search \"$os_x_darwin_thread\")
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
>
> -test_begin_subtest "Add tag from notmuch-show view"
> +test_begin_subtest "notmuch-show: add single tag to single message"
> test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> (execute-kbd-macro \"+tag-from-show-view\")"
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> tag-from-show-view unread)"
>
> -test_begin_subtest "Remove tag from notmuch-show view"
> +test_begin_subtest "notmuch-show: remove single tag from single message"
> test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> (execute-kbd-macro \"-tag-from-show-view\")"
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
>
> +test_begin_subtest "notmuch-show: add multiple tags to single message"
> +test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> + (execute-kbd-macro \"+tag1-from-show-view +tag2-from-show-view\")"
> +output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> +test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> tag1-from-show-view tag2-from-show-view unread)"
> +
> +test_begin_subtest "notmuch-show: remove multiple tags from single message"
> +test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> + (execute-kbd-macro \"-tag1-from-show-view -tag2-from-show-view\")"
> +output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> +test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
> +
> test_begin_subtest "Message with .. in Message-Id:"
> add_message [id]=123..456 at example '[subject]="Message with .. in
> Message-Id"'
> test_emacs '(notmuch-search "id:\"123..456 at example\"")
> --
> 1.7.8.1
>
[PATCH v5 00/12] emacs: more flexible and consistent tagging operations
On Fri, 24 Feb 2012 14:30:29 +0400, Dmitry Kurochkin wrote: > On Fri, 24 Feb 2012 00:07:27 +0100, Pieter Praet wrote: > > On Wed, 08 Feb 2012 11:58:32 -0400, David Bremner > > wrote: > > > On Sun, 5 Feb 2012 11:13:41 +0400, Dmitry Kurochkin > > gmail.com> wrote: > > > > Changes: > > > > > > > > v4: > > > > > > > > * rebased on master, no conflicts so no need for another review > > > > > > > > > > I pushed this series. > > > > > > Maybe this was discussed already, but I think ideally * would apply only > > > to open messages. So consider that a feature request if someone is > > > looking for a project. > > > > > > > How about if '*' applies to all messages (as it currently does), > > but 'C-u *' only to open messages? That would make more sense IMHO. > > > > But, conforming to your original request, I've implemented the inverse. > > > > I personally do like '*' as is and do not want to change it's behavior. > Though I am not against adding a prefix argument for it. > Also can you please send new patches (and patch series) in a new separate thread? If they are related to another thread, you can add a reference. But having multiple patch series with multiple versions in a single thread is very confusing IMO. Regards, Dmitry > Regards, > Dmitry > > > Patches follow. > > > > > > > d > > > ___ > > > notmuch mailing list > > > notmuch at notmuchmail.org > > > http://notmuchmail.org/mailman/listinfo/notmuch > > > > > > Peace > > > > -- > > Pieter
[PATCH v5 00/12] emacs: more flexible and consistent tagging operations
On Fri, 24 Feb 2012 00:07:27 +0100, Pieter Praet wrote: > On Wed, 08 Feb 2012 11:58:32 -0400, David Bremner > wrote: > > On Sun, 5 Feb 2012 11:13:41 +0400, Dmitry Kurochkin > gmail.com> wrote: > > > Changes: > > > > > > v4: > > > > > > * rebased on master, no conflicts so no need for another review > > > > > > > I pushed this series. > > > > Maybe this was discussed already, but I think ideally * would apply only > > to open messages. So consider that a feature request if someone is > > looking for a project. > > > > How about if '*' applies to all messages (as it currently does), > but 'C-u *' only to open messages? That would make more sense IMHO. > > But, conforming to your original request, I've implemented the inverse. > I personally do like '*' as is and do not want to change it's behavior. Though I am not against adding a prefix argument for it. Regards, Dmitry > Patches follow. > > > > d > > ___ > > notmuch mailing list > > notmuch at notmuchmail.org > > http://notmuchmail.org/mailman/listinfo/notmuch > > > Peace > > -- > Pieter
Re: [PATCH 6/6] emacs: `notmuch-show-tag-all' sans prefix arg only tags open messages
On Fri, 24 Feb 2012 00:09:14 +0100, Pieter Praet wrote: > * emacs/notmuch-show.el > > (notmuch-show-get-messages-ids): > New optional argument ONLY-OPEN. If non-nil, only return > Message-Id's for messages which are currently visible. > > (notmuch-show-tag-all): > New optional argument IGNORE-VISIBILITY, of which the inverse is > passed as ONLY-OPEN argument to `notmuch-show-get-messages-ids': > If called with a prefix arg, affect *all* messages in the current > buffer. Otherwise, only change tags of visible messages. > > (notmuch-show-archive-thread): > Update wrt changes to `notmuch-show-tag-all'. > > * test/emacs > > - Subtest "notmuch-show: change tags of open messages in current buffer" > is no longer broken. > --- > emacs/notmuch-show.el | 28 > test/emacs|1 - > 2 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 4b37c77..4499fcd 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1334,13 +1334,17 @@ (defun notmuch-show-get-message-id () >"Return the message id of the current message." >(concat "id:\"" (notmuch-show-get-prop :id) "\"")) > > -(defun notmuch-show-get-messages-ids (&optional separator) > +(defun notmuch-show-get-messages-ids (&optional only-open separator) >"Return a list of Message-Id's of all messages in the current buffer. > > +If optional argument ONLY-OPEN is non-nil, only return > +Message-Id's for messages which are currently visible. > + > If provided with optional argument SEPARATOR, return a string > instead, consisting of all Message-Id's separated by SEPARATOR." >(let ((message-ids)) > -(notmuch-show-mapc t > +(notmuch-show-mapc > + `(if only-open (notmuch-show-message-visible-p) t) How about changing ONLY-OPEN to a general optional PREDICATE argument and pass it as is to `notmuch-show-mapc'? Please make it the last argument. Regards, Dmitry > (lambda () (push (notmuch-show-get-message-id) message-ids))) > (if separator > (mapconcat 'identity message-ids separator) > @@ -1628,13 +1632,21 @@ (defun notmuch-show-tag (&optional initial-input) > initial-input (notmuch-show-get-message-id > (apply 'notmuch-show-tag-message tag-changes))) > > -(defun notmuch-show-tag-all (&rest tag-changes) > - "Change tags for all messages in the current thread. > +(defun notmuch-show-tag-all (&optional ignore-visibility &rest tag-changes) > + "Change tags of all open messages in the current buffer. > + > +If optional arg IGNORE-VISIBILITY is non-nil, change tags of > +*all* messages in the current buffer, independent of their > +visibility. > > TAG-CHANGES is a list of tag operations for `notmuch-tag'." > - (interactive (notmuch-read-tag-changes nil notmuch-show-thread-id)) > - (apply 'notmuch-tag (notmuch-show-get-messages-ids " or ") tag-changes) > - (notmuch-show-mapc t > + (interactive (cons current-prefix-arg > + (notmuch-read-tag-changes nil notmuch-show-thread-id))) > + (apply 'notmuch-tag > + (notmuch-show-get-messages-ids (not ignore-visibility) " or ") > + tag-changes) > + (notmuch-show-mapc > + `(if ignore-visibility t (notmuch-show-message-visible-p)) > (lambda () > (let* ((current-tags (notmuch-show-get-tags)) > (new-tags (notmuch-update-tags current-tags tag-changes))) > @@ -1719,7 +1731,7 @@ (defun notmuch-show-archive-thread (&optional unarchive) > buffer." >(interactive "P") >(let ((op (if unarchive "+" "-"))) > -(notmuch-show-tag-all (concat op "inbox" > +(notmuch-show-tag-all t (concat op "inbox" > > (defun notmuch-show-archive-thread-then-next () >"Archive each message in thread, then show next thread from search." > diff --git a/test/emacs b/test/emacs > index 644ef59..c286ff5 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -152,7 +152,6 @@ notmuch tag +"$del_tag" -"$add_tag" -- "$query" # revert > tag changes > test_expect_equal "$count_changed" "$count_total" # assert that CHANGED == > TOTAL > > test_begin_subtest "notmuch-show: change tags of open messages in current > buffer" > -test_subtest_known_broken > query="$os_x_darwin_thread" > filter="from:Jiang" > add_tag="notmuch-show-tag-all" > -- > 1.7.8.1 > ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 5/6] emacs: simplify `notmuch-show-get-messages-ids{, -search}'
On Fri, 24 Feb 2012 00:09:13 +0100, Pieter Praet wrote: > * emacs/notmuch-show.el > > (notmuch-show-get-messages-ids): > If provided with optional arg SEPARATOR, return a string consisting > of all Message-Id's, separated by SEPARATOR. Also improve original > docstring wrt default return value. > > (notmuch-show-get-messages-ids-search): > Removed, as its functionality is now in `notmuch-show-get-messages-ids'. > > (notmuch-show-tag-all): > Call `notmuch-show-get-messages-ids' with SEPARATOR arg instead of > `notmuch-show-get-messages-ids-search'. > --- There is another similar case in notmuch.el: `notmuch-show-get-messages-ids' and `notmuch-show-get-messages-ids-search'. There may be more. Please change them as well. Regards, Dmitry > emacs/notmuch-show.el | 18 +- > 1 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 5fc0e43..4b37c77 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1334,17 +1334,17 @@ (defun notmuch-show-get-message-id () >"Return the message id of the current message." >(concat "id:\"" (notmuch-show-get-prop :id) "\"")) > > -(defun notmuch-show-get-messages-ids () > - "Return all message ids of messages in the current thread." > +(defun notmuch-show-get-messages-ids (&optional separator) > + "Return a list of Message-Id's of all messages in the current buffer. > + > +If provided with optional argument SEPARATOR, return a string > +instead, consisting of all Message-Id's separated by SEPARATOR." >(let ((message-ids)) > (notmuch-show-mapc t > (lambda () (push (notmuch-show-get-message-id) message-ids))) > -message-ids)) > - > -(defun notmuch-show-get-messages-ids-search () > - "Return a search string for all message ids of messages in the > -current thread." > - (mapconcat 'identity (notmuch-show-get-messages-ids) " or ")) > +(if separator > + (mapconcat 'identity message-ids separator) > + message-ids))) > > ;; dme: Would it make sense to use a macro for many of these? > > @@ -1633,7 +1633,7 @@ (defun notmuch-show-tag-all (&rest tag-changes) > > TAG-CHANGES is a list of tag operations for `notmuch-tag'." >(interactive (notmuch-read-tag-changes nil notmuch-show-thread-id)) > - (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes) > + (apply 'notmuch-tag (notmuch-show-get-messages-ids " or ") tag-changes) >(notmuch-show-mapc t > (lambda () > (let* ((current-tags (notmuch-show-get-tags)) > -- > 1.7.8.1 > ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/6] emacs: add predicate arg to `notmuch-show-mapc'
On Fri, 24 Feb 2012 00:09:12 +0100, Pieter Praet wrote: > * emacs/notmuch-show.el > > (notmuch-show-mapc): > Only call FUNCTION if new argument PREDICATE is satisfied. > Also correct original docstring: 's/thread/buffer/'. > > (notmuch-show-get-messages-ids): > Update wrt changes to `notmuch-show-mapc'. > > (notmuch-show-tag-all): > Update wrt changes to `notmuch-show-mapc'. > --- > emacs/notmuch-show.el | 15 --- > 1 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index aa9ccee..5fc0e43 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1272,13 +1272,14 @@ (defun notmuch-show-goto-message-previous () > (notmuch-show-move-to-message-top) > t)) > > -(defun notmuch-show-mapc (function) > - "Iterate through all messages in the current thread with > -`notmuch-show-goto-message-next' and call FUNCTION for side > -effects." > +(defun notmuch-show-mapc (predicate function) Please make PREDICATE optional to avoid giving t when it is not needed. I would expect PREDICATE to be a function, but perhaps the way you implemented it is common in Emacs. Regards, Dmitry > + "Iterate through all messages in the current buffer with > +`notmuch-show-goto-message-next'. If PREDICATE is satisfied, > +call FUNCTION for side effects." >(save-excursion > (goto-char (point-min)) > -(loop do (funcall function) > +(loop do (if (eval predicate) > + (funcall function)) > while (notmuch-show-goto-message-next > > ;; Functions relating to the visibility of messages and their > @@ -1336,7 +1337,7 @@ (defun notmuch-show-get-message-id () > (defun notmuch-show-get-messages-ids () >"Return all message ids of messages in the current thread." >(let ((message-ids)) > -(notmuch-show-mapc > +(notmuch-show-mapc t > (lambda () (push (notmuch-show-get-message-id) message-ids))) > message-ids)) > > @@ -1633,7 +1634,7 @@ (defun notmuch-show-tag-all (&rest tag-changes) > TAG-CHANGES is a list of tag operations for `notmuch-tag'." >(interactive (notmuch-read-tag-changes nil notmuch-show-thread-id)) >(apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes) > - (notmuch-show-mapc > + (notmuch-show-mapc t > (lambda () > (let* ((current-tags (notmuch-show-get-tags)) > (new-tags (notmuch-update-tags current-tags tag-changes))) > -- > 1.7.8.1 > ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/6] test: emacs: new tests "notmuch-show: {add, remove} multiple tags {to, from} single message"
On Fri, 24 Feb 2012 00:09:09 +0100, Pieter Praet wrote:
> * test/emacs:
>
> - Rename subtests "{Add,Remove} tag from notmuch-show view" to
> "notmuch-show: {add,remove} single tag {to,from} single message"
> to be consistent with the following tests.
>
> - New subtest "notmuch-show: add multiple tags to single message":
> `notmuch-show-add-tag' ("+") can add multiple tags to a message.
>
> - New subtest "notmuch-show: remove multiple tags from single message":
> `notmuch-show-remove-tag' ("-") can remove multiple tags from a message.
> ---
Would be nice to have another patch that moves notmuch-show tests to
emacs-show file.
Regards,
Dmitry
> test/emacs | 16 ++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index b74cfa9..ec1dbb0 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -112,18 +112,30 @@ test_emacs "(notmuch-search \"$os_x_darwin_thread\")
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
>
> -test_begin_subtest "Add tag from notmuch-show view"
> +test_begin_subtest "notmuch-show: add single tag to single message"
> test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> (execute-kbd-macro \"+tag-from-show-view\")"
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> tag-from-show-view unread)"
>
> -test_begin_subtest "Remove tag from notmuch-show view"
> +test_begin_subtest "notmuch-show: remove single tag from single message"
> test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> (execute-kbd-macro \"-tag-from-show-view\")"
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
>
> +test_begin_subtest "notmuch-show: add multiple tags to single message"
> +test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> + (execute-kbd-macro \"+tag1-from-show-view +tag2-from-show-view\")"
> +output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> +test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> tag1-from-show-view tag2-from-show-view unread)"
> +
> +test_begin_subtest "notmuch-show: remove multiple tags from single message"
> +test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> + (execute-kbd-macro \"-tag1-from-show-view -tag2-from-show-view\")"
> +output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> +test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
> +
> test_begin_subtest "Message with .. in Message-Id:"
> add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
> test_emacs '(notmuch-search "id:\"123..456@example\"")
> --
> 1.7.8.1
>
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v5 00/12] emacs: more flexible and consistent tagging operations
On Fri, 24 Feb 2012 14:30:29 +0400, Dmitry Kurochkin wrote: > On Fri, 24 Feb 2012 00:07:27 +0100, Pieter Praet wrote: > > On Wed, 08 Feb 2012 11:58:32 -0400, David Bremner wrote: > > > On Sun, 5 Feb 2012 11:13:41 +0400, Dmitry Kurochkin > > > wrote: > > > > Changes: > > > > > > > > v4: > > > > > > > > * rebased on master, no conflicts so no need for another review > > > > > > > > > > I pushed this series. > > > > > > Maybe this was discussed already, but I think ideally * would apply only > > > to open messages. So consider that a feature request if someone is > > > looking for a project. > > > > > > > How about if '*' applies to all messages (as it currently does), > > but 'C-u *' only to open messages? That would make more sense IMHO. > > > > But, conforming to your original request, I've implemented the inverse. > > > > I personally do like '*' as is and do not want to change it's behavior. > Though I am not against adding a prefix argument for it. > Also can you please send new patches (and patch series) in a new separate thread? If they are related to another thread, you can add a reference. But having multiple patch series with multiple versions in a single thread is very confusing IMO. Regards, Dmitry > Regards, > Dmitry > > > Patches follow. > > > > > > > d > > > ___ > > > notmuch mailing list > > > notmuch@notmuchmail.org > > > http://notmuchmail.org/mailman/listinfo/notmuch > > > > > > Peace > > > > -- > > Pieter ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v5 00/12] emacs: more flexible and consistent tagging operations
On Fri, 24 Feb 2012 00:07:27 +0100, Pieter Praet wrote: > On Wed, 08 Feb 2012 11:58:32 -0400, David Bremner wrote: > > On Sun, 5 Feb 2012 11:13:41 +0400, Dmitry Kurochkin > > wrote: > > > Changes: > > > > > > v4: > > > > > > * rebased on master, no conflicts so no need for another review > > > > > > > I pushed this series. > > > > Maybe this was discussed already, but I think ideally * would apply only > > to open messages. So consider that a feature request if someone is > > looking for a project. > > > > How about if '*' applies to all messages (as it currently does), > but 'C-u *' only to open messages? That would make more sense IMHO. > > But, conforming to your original request, I've implemented the inverse. > I personally do like '*' as is and do not want to change it's behavior. Though I am not against adding a prefix argument for it. Regards, Dmitry > Patches follow. > > > > d > > ___ > > notmuch mailing list > > notmuch@notmuchmail.org > > http://notmuchmail.org/mailman/listinfo/notmuch > > > Peace > > -- > Pieter ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 2/4] test: emacs: new test "notmuch-search: replying to a thread (sending)"
On Sun, 19 Feb 2012 21:38:26 +0100, Pieter Praet wrote:
> As pointed out in the previous commit, the test for replying from
> within Emacs didn't actually submit the reply. This one does.
> ---
> test/emacs | 41 +
> 1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 308d749..0f4f42b 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -273,6 +273,47 @@ On 01 Jan 2000 12:00:00 -, Notmuch Test Suite
> w
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> +test_begin_subtest "notmuch-search: replying to a thread (sending)"
> +$TEST_DIRECTORY/smtp-dummy sent_message &
> +smtp_dummy_pid=$!
> +test_emacs \
> +'(let ((message-send-mail-function '\''message-smtpmail-send-it)
> + (smtpmail-smtp-server "localhost")
> + (smtpmail-smtp-service "25025"))
> + (notmuch-search "subject:\"testing message sent via SMTP\"")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (end-of-buffer)
> + (newline)
> + (insert "Reply to a message via Emacs with fake SMTP")
> + (message-send-and-exit))' >/dev/null 2>&1
> +wait ${smtp_dummy_pid}
> +notmuch new >/dev/null
> +sed \
> +-e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX
> Emacs/XXX,' \
> +-e s',^Message-ID: <.*>$,Message-ID: ,' \
> +-e s',^In-Reply-To: <.*>$,In-Reply-To: ,' \
> +-e s',^References: <.*>$,References: ,' \
> +-e s',^Date: .*$,Date: Fri\, 29 Mar 1974 10:05:00 -,' < sent_message
> >OUTPUT
This sed command is a (partial) copy past from "Sending a message via
(fake) SMTP" test. I suggest adding notmuch_sent_message_sanitize
function which would replace common headers with XXX.
Replace Date with XXX for Date for consistency with other headers,
AFAICT a valid date value is not needed here.
Regards,
Dmitry
> +cat +From: Notmuch Test Suite
> +To: user at example.com
> +Subject: Re: Testing message sent via SMTP
> +In-Reply-To:
> +References:
> +User-Agent: Notmuch/XXX Emacs/XXX
> +Date: Fri, 29 Mar 1974 10:05:00 -
> +Message-ID:
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=us-ascii
> +
> +On 01 Jan 2000 12:00:00 -, Notmuch Test Suite notmuchmail.org> wrote:
> +> This is a test that messages are sent via SMTP
> +
> +Reply to a message via Emacs with fake SMTP
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> test_begin_subtest "Quote MML tags in reply"
> message_id='test-emacs-mml-quoting at message.id'
> add_message [id]="$message_id" \
> --
> 1.7.8.1
>
[PATCH v2 4/4] test: emacs: new test "notmuch-search: change tags of all matching messages"
On Sun, 19 Feb 2012 21:38:28 +0100, Pieter Praet wrote:
> `notmuch-search-tag-all' (bound to "*") adds and removes tags
> to/from all messages which match the query used to populate the
> current search buffer.
LGTM. But since you will need to send a new version to address Tomi's
comments anyway, below are few minor comments.
Regards,
Dmitry
> ---
> test/emacs | 32
> 1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index b0fb760..1db8540 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -124,6 +124,38 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
>
> +test_begin_subtest "notmuch-search: change tags of all matching messages"
> +old_tag="inbox"
> +new_tag="xobni"
> +filter="AND from:cworth"
> +# Get initial tag counts and prevent false positives/negatives
> +old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
> +test "${old_tag_count_1}" == "0" && old_tag_count_1="Need 1+ matches!"
Consider s/1+/>0/.
> +test "${new_tag_count_1}" == "0" || new_tag_count_1="Need 0 matches!"
> +# Change tags of all matching messages and get tag counts
> +test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
> + (notmuch-test-wait)
> + (notmuch-search-tag-all \"-${old_tag}\" \"+${new_tag}\")"
> +old_tag_count_2=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_2=$(notmuch count tag:"${new_tag}" "${filter}")
> +# Revert tag changes and get tag counts
> +test_emacs "(notmuch-search \"tag:${new_tag} ${filter}\")
> + (notmuch-test-wait)
> + (notmuch-search-tag-all \"+${old_tag}\" \"-${new_tag}\")"
> +old_tag_count_3=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_3=$(notmuch count tag:"${new_tag}" "${filter}")
> +# ... and verify the results
> +output="
> +before: old:${old_tag_count_1} new:${new_tag_count_1}
> +after:old:${old_tag_count_2} new:${new_tag_count_2}
> +restored: old:${old_tag_count_3} new:${new_tag_count_3}"
> +expected="
> +before: old:${old_tag_count_1} new:0
> +after:old:0 new:${old_tag_count_1}
> +restored: old:${old_tag_count_1} new:0"
> +test_expect_equal "$output" "$expected"
> +
I would add a newline before every commented block.
Regards,
Dmitry
> test_begin_subtest "Message with .. in Message-Id:"
> add_message [id]=123..456 at example '[subject]="Message with .. in
> Message-Id"'
> test_emacs '(notmuch-search "id:\"123..456 at example\"")
> --
> 1.7.8.1
>
[PATCH v2 3/4] test: emacs: new test "notmuch-search: when reply is sent, parent message should be tagged 'replied'"
On Sun, 19 Feb 2012 21:38:27 +0100, Pieter Praet wrote: > When a message is replied to, it should be tagged `replied'. > --- > test/emacs |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/test/emacs b/test/emacs > index 0f4f42b..b0fb760 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -314,6 +314,11 @@ Reply to a message via Emacs with fake SMTP > EOF > test_expect_equal_file OUTPUT EXPECTED > > +test_begin_subtest "notmuch-search: when reply is sent, the parent message > should be tagged 'replied'" > +# depends on subtest "notmuch-search: replying to a thread (sending)" > +output=$(notmuch search 'tag:replied' | notmuch_search_sanitize) Add subject:"testing message sent via SMTP" to the query to avoid potential collisions with other tests. Regards, Dmitry > +test_expect_equal "$output" "thread:XXX 2000-01-01 [1/2] Notmuch Test > Suite; Testing message sent via SMTP (inbox replied)" > + > test_begin_subtest "Quote MML tags in reply" > message_id='test-emacs-mml-quoting at message.id' > add_message [id]="$message_id" \ > -- > 1.7.8.1 >
[PATCH v2 1/4] test: emacs: rename subtest "Reply within emacs"
On Sun, 19 Feb 2012 21:38:25 +0100, Pieter Praet wrote: > Rename subtest "Reply within emacs" to "notmuch-search: replying to a > thread (composing)" as its title is misleading: it populates a reply > buffer but doesn't submit it. > --- LGTM Regards, Dmitry > test/emacs |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/test/emacs b/test/emacs > index b74cfa9..308d749 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -255,7 +255,7 @@ Subject: > EOF > test_expect_equal_file OUTPUT EXPECTED > > -test_begin_subtest "Reply within emacs" > +test_begin_subtest "notmuch-search: replying to a thread (composing)" > test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"") > (notmuch-test-wait) > (notmuch-search-reply-to-thread) > -- > 1.7.8.1 >
Re: [PATCH v2 2/4] test: emacs: new test "notmuch-search: replying to a thread (sending)"
On Sun, 19 Feb 2012 21:38:26 +0100, Pieter Praet wrote:
> As pointed out in the previous commit, the test for replying from
> within Emacs didn't actually submit the reply. This one does.
> ---
> test/emacs | 41 +
> 1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 308d749..0f4f42b 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -273,6 +273,47 @@ On 01 Jan 2000 12:00:00 -, Notmuch Test Suite
> w
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> +test_begin_subtest "notmuch-search: replying to a thread (sending)"
> +$TEST_DIRECTORY/smtp-dummy sent_message &
> +smtp_dummy_pid=$!
> +test_emacs \
> +'(let ((message-send-mail-function '\''message-smtpmail-send-it)
> + (smtpmail-smtp-server "localhost")
> + (smtpmail-smtp-service "25025"))
> + (notmuch-search "subject:\"testing message sent via SMTP\"")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (end-of-buffer)
> + (newline)
> + (insert "Reply to a message via Emacs with fake SMTP")
> + (message-send-and-exit))' >/dev/null 2>&1
> +wait ${smtp_dummy_pid}
> +notmuch new >/dev/null
> +sed \
> +-e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX
> Emacs/XXX,' \
> +-e s',^Message-ID: <.*>$,Message-ID: ,' \
> +-e s',^In-Reply-To: <.*>$,In-Reply-To: ,' \
> +-e s',^References: <.*>$,References: ,' \
> +-e s',^Date: .*$,Date: Fri\, 29 Mar 1974 10:05:00 -,' < sent_message
> >OUTPUT
This sed command is a (partial) copy past from "Sending a message via
(fake) SMTP" test. I suggest adding notmuch_sent_message_sanitize
function which would replace common headers with XXX.
Replace Date with XXX for Date for consistency with other headers,
AFAICT a valid date value is not needed here.
Regards,
Dmitry
> +cat +From: Notmuch Test Suite
> +To: u...@example.com
> +Subject: Re: Testing message sent via SMTP
> +In-Reply-To:
> +References:
> +User-Agent: Notmuch/XXX Emacs/XXX
> +Date: Fri, 29 Mar 1974 10:05:00 -
> +Message-ID:
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=us-ascii
> +
> +On 01 Jan 2000 12:00:00 -, Notmuch Test Suite
> wrote:
> +> This is a test that messages are sent via SMTP
> +
> +Reply to a message via Emacs with fake SMTP
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> test_begin_subtest "Quote MML tags in reply"
> message_id='test-emacs-mml-quot...@message.id'
> add_message [id]="$message_id" \
> --
> 1.7.8.1
>
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 4/4] test: emacs: new test "notmuch-search: change tags of all matching messages"
On Sun, 19 Feb 2012 21:38:28 +0100, Pieter Praet wrote:
> `notmuch-search-tag-all' (bound to "*") adds and removes tags
> to/from all messages which match the query used to populate the
> current search buffer.
LGTM. But since you will need to send a new version to address Tomi's
comments anyway, below are few minor comments.
Regards,
Dmitry
> ---
> test/emacs | 32
> 1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index b0fb760..1db8540 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -124,6 +124,38 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> test_expect_equal "$output" "thread:XXX 2009-11-18 [4/4] Jjgod Jiang,
> Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox
> unread)"
>
> +test_begin_subtest "notmuch-search: change tags of all matching messages"
> +old_tag="inbox"
> +new_tag="xobni"
> +filter="AND from:cworth"
> +# Get initial tag counts and prevent false positives/negatives
> +old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
> +test "${old_tag_count_1}" == "0" && old_tag_count_1="Need 1+ matches!"
Consider s/1+/>0/.
> +test "${new_tag_count_1}" == "0" || new_tag_count_1="Need 0 matches!"
> +# Change tags of all matching messages and get tag counts
> +test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
> + (notmuch-test-wait)
> + (notmuch-search-tag-all \"-${old_tag}\" \"+${new_tag}\")"
> +old_tag_count_2=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_2=$(notmuch count tag:"${new_tag}" "${filter}")
> +# Revert tag changes and get tag counts
> +test_emacs "(notmuch-search \"tag:${new_tag} ${filter}\")
> + (notmuch-test-wait)
> + (notmuch-search-tag-all \"+${old_tag}\" \"-${new_tag}\")"
> +old_tag_count_3=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_3=$(notmuch count tag:"${new_tag}" "${filter}")
> +# ... and verify the results
> +output="
> +before: old:${old_tag_count_1} new:${new_tag_count_1}
> +after:old:${old_tag_count_2} new:${new_tag_count_2}
> +restored: old:${old_tag_count_3} new:${new_tag_count_3}"
> +expected="
> +before: old:${old_tag_count_1} new:0
> +after:old:0 new:${old_tag_count_1}
> +restored: old:${old_tag_count_1} new:0"
> +test_expect_equal "$output" "$expected"
> +
I would add a newline before every commented block.
Regards,
Dmitry
> test_begin_subtest "Message with .. in Message-Id:"
> add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
> test_emacs '(notmuch-search "id:\"123..456@example\"")
> --
> 1.7.8.1
>
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 3/4] test: emacs: new test "notmuch-search: when reply is sent, parent message should be tagged 'replied'"
On Sun, 19 Feb 2012 21:38:27 +0100, Pieter Praet wrote: > When a message is replied to, it should be tagged `replied'. > --- > test/emacs |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/test/emacs b/test/emacs > index 0f4f42b..b0fb760 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -314,6 +314,11 @@ Reply to a message via Emacs with fake SMTP > EOF > test_expect_equal_file OUTPUT EXPECTED > > +test_begin_subtest "notmuch-search: when reply is sent, the parent message > should be tagged 'replied'" > +# depends on subtest "notmuch-search: replying to a thread (sending)" > +output=$(notmuch search 'tag:replied' | notmuch_search_sanitize) Add subject:"testing message sent via SMTP" to the query to avoid potential collisions with other tests. Regards, Dmitry > +test_expect_equal "$output" "thread:XXX 2000-01-01 [1/2] Notmuch Test > Suite; Testing message sent via SMTP (inbox replied)" > + > test_begin_subtest "Quote MML tags in reply" > message_id='test-emacs-mml-quot...@message.id' > add_message [id]="$message_id" \ > -- > 1.7.8.1 > ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 1/4] test: emacs: rename subtest "Reply within emacs"
On Sun, 19 Feb 2012 21:38:25 +0100, Pieter Praet wrote: > Rename subtest "Reply within emacs" to "notmuch-search: replying to a > thread (composing)" as its title is misleading: it populates a reply > buffer but doesn't submit it. > --- LGTM Regards, Dmitry > test/emacs |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/test/emacs b/test/emacs > index b74cfa9..308d749 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -255,7 +255,7 @@ Subject: > EOF > test_expect_equal_file OUTPUT EXPECTED > > -test_begin_subtest "Reply within emacs" > +test_begin_subtest "notmuch-search: replying to a thread (composing)" > test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"") > (notmuch-test-wait) > (notmuch-search-reply-to-thread) > -- > 1.7.8.1 > ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
