Re: [PATCH] Add Emacs' imenu support in notmuch-show and notmuch-search

[David Bremner]

Damien Cassou  writes:

> David Bremner  writes: 
>> In show mode the chosen imenu index function seems less useful 
>> to me; it treats indentation level as significant, so in threads 
>> with many levels of reply, one has to type many periods. It's 
>> also not clear to me that the information on the header line is 
>> very helpful for navigation, even without indentation. 
>  
> I guess you are using the default imenu frontend which is both 
> ugly and inefficient. I also guess few people care about adding 
> imenu support to packages for this exact reason.   The counsel's 
> version is quite good though (see attached screenshot). The 
> indentation is respected making the imenu content a visual 
> overview of the thread (and you don't have to type any periods).

I am indeed using the default. I think you forgot the screen shot.

> I can still get rid of indentation if you confirm you don't want 
> it.

I think so, although to be honest I never tried imenu before testing
your patches, perhaps we should wait for other opinions.

> You also say the information on 
> the header line is not very helpful. Do you have any other 
> suggestion? I'm open to trying several different alternatives and 
> see what people prefer. 

I guess I don't really understand how the matching is supposed to
work. If the user has to type from the beginning of the index string,
then I guess putting either the date or the subject first might be
helpful.  It's probably true that there is no one good order for all
threads. Some threads change subject often, others author, and one I'm
looking at right now has 25 messages with two authors and one subject.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 11/11] emacs: notmuch-show: add filesize to headerline

[David Bremner]

Ioan-Adrian Ratiu  writes:

>   date
>   ") ("
> + (file-size-human-readable filesize)
> + ") ("
>   (notmuch-tag-format-tags tags tags)


I think this should probably be optional.  I don't know that everyone
will want to give up the screen space for this. I'm not sure how hard it
would be to have a fully customizable format like notmuch-search-result-format

Another thing I noticed is that your emacs code crashes when used
against an older version of notmuch.  You can either make the emacs code
more robust, or use the format-version machinery to ensure that the
emacs binary version is new enough.  The former is probably nicer since
you most likely want to gracefully handle databases without the filesize
value slot.

Finally, with the current patch series, there are over 200 failing
tests. That's obviously something to improve ;).

d


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 09/11] notmuch-show: export message filesize

[David Bremner]

Ioan-Adrian Ratiu  writes:

> Signed-off-by: Ioan-Adrian Ratiu 
> ---
>  notmuch-show.c | 5 +
>  1 file changed, 5 insertions(+)

This should update schemata and several tests.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 10/11] emacs: notmuch-search: add display thread sizes capability

[David Bremner]

Ioan-Adrian Ratiu  writes:

> By default this is off because it's tiresome to look at all those
> numbers in every search view. It's much more pleasant to have it
> enabled by default in notmuch-show even if you apply searches and sort
> results based on file size.
>
> Signed-off-by: Ioan-Adrian Ratiu 
> ---
>  emacs/notmuch.el | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 5b9c1d07..dbcd67eb 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -73,7 +73,7 @@
>  ("subject" . "%s ")
>  ("tags" . "(%s)"))
>"Search result formatting. Supported fields are:
> - date, count, authors, subject, tags
> + date, count, total_filesize, authors, subject, tags
>  For example:

>  
>  (defun notmuch-search-insert-field (field format-string result)
>(cond
> +   ((string-equal field "total-filesize")
> +(insert (propertize (format format-string (file-size-human-readable 
> (plist-get result :total_filesize)))
> + 'face 'notmuch-search-thread-total-filesize)))

there seems to be some confusion here about total-filesize and
total_filesize; I guess the docstring for the variable should match the
string-equal test.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 10/11] emacs: notmuch-search: add display thread sizes capability

[David Bremner]

Ioan-Adrian Ratiu  writes:

> By default this is off because it's tiresome to look at all those
> numbers in every search view. It's much more pleasant to have it
> enabled by default in notmuch-show even if you apply searches and sort
> results based on file size.

this comment is a bit confusing, since the commit doesn't actually do
anything with notmuch-show. Maybe you can clarify that.

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 08/11] notmuch-search: output total_filesize thread result

[David Bremner]

Ioan-Adrian Ratiu  writes:

> This works for all the search output formats (sexp, json, text).
>
>   if (format->is_text_printer) {
>  /* Special case for the text formatter */
> - printf ("thread:%s %12s [%d/%d] %s; %s (",
> + printf ("thread:%s %lu %12s [%d/%d] %s; %s (",

I'm not convinced about this change. I don't think that everyone finds it
as important as you to know the total filesize in a thread. It also
breaks many tests. One thing I noticed when running the tests is that in
quite a few cases, it prints 0. So I suspect there is some bugs left.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 07/11] lib: thread: add thread total size function

[David Bremner]

Ioan-Adrian Ratiu  writes:

> Given a thread object this function computes the total disk space used
> by all messages contained in the thread.
>

Can you take the approach of id:20170604123235.24466-9-da...@tethera.net
and compute the total filesize as the thread is built?

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 05/11] emacs: notmuch-search: add filesize sorting

[David Bremner]

Ioan-Adrian Ratiu  writes:

> Besides the previous date-based result orderings (oldest-first and
> newest-first) add two more filesize-based orderings: biggest-first
> smallest-first.
>
> The orderings are interchangeable, you can specify any one as the
> default via notmuch-search-default-sort-order or as the preffered
spelling ^
> ordering for a saved search (via the :sort-order property).
>
> Signed-off-by: Ioan-Adrian Ratiu 
> ---
>  emacs/notmuch-hello.el |  9 ++---
>  emacs/notmuch-lib.el   |  4 +++-
>  emacs/notmuch.el   | 12 ++--
>  3 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index 3ba2a16b..51117577 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -95,7 +95,9 @@ searches so they still work in customize."
>   (choice :tag " Sort Order"
>   (const :tag "Default" nil)
>   (const :tag "Oldest-first" oldest-first)
> - (const :tag "Newest-first" newest-first)))
> + (const :tag "Newest-first" newest-first)
> + (const :tag "Biggest-first" biggest-first)
> + (const :tag "Smallest-first" 
> smallest-first)))

I agree with Tomi that Largest sounds better than Biggest here.

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 04/11] emacs: make notmuch-search-oldest-first generic

[David Bremner]

Ioan-Adrian Ratiu  writes:

> The current search result order logic assumes results are always
> sorted by date and thus uses a boolean switch for oldest/newest
> ordering specifications.
>
> This is problematic if I want to introduce other result orderings,
> like for example based on the mail-file size with smallest/biggest
> ordering specifications.
It's a minor point, but I would write "if we want" here
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] Add Emacs' imenu support in notmuch-show and notmuch-search

[Damien Cassou]

David Bremner  writes: 
In show mode the chosen imenu index function seems less useful 
to me; it treats indentation level as significant, so in threads 
with many levels of reply, one has to type many periods. It's 
also not clear to me that the information on the header line is 
very helpful for navigation, even without indentation. 


I guess you are using the default imenu frontend which is both 
ugly and inefficient. I also guess few people care about adding 
imenu support to packages for this exact reason.   The counsel's 
version is quite good though (see attached screenshot). The 
indentation is respected making the imenu content a visual 
overview of the thread (and you don't have to type any periods). 
I can still get rid of indentation if you confirm you don't want 
it. For example, I understand that it would be nice to have better 
integration with the original imenu frontend (even though the 
default frontend is so bad I doubt anyone not having a better 
version ever uses imenu at all). You also say the information on 
the header line is not very helpful. Do you have any other 
suggestion? I'm open to trying several different alternatives and 
see what people prefer. 

Bye 


--
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
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] fixup! lib: index message files with duplicate message-ids

[David Bremner]

---

This seems to do the trick for preserving the subject as additional files are 
indexed.

 lib/add-message.cc | 3 ++-
 test/T670-duplicate-mid.sh | 7 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/add-message.cc b/lib/add-message.cc
index 26405742..711ed9fa 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -536,7 +536,8 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
if (ret)
goto DONE;
 
-   _notmuch_message_set_header_values (message, date, from, subject);
+   if (is_new || is_ghost)
+   _notmuch_message_set_header_values (message, date, from, subject);
 
ret = _notmuch_message_index_file (message, message_file);
if (ret)
diff --git a/test/T670-duplicate-mid.sh b/test/T670-duplicate-mid.sh
index da5ce5d4..ea5e1d6a 100755
--- a/test/T670-duplicate-mid.sh
+++ b/test/T670-duplicate-mid.sh
@@ -5,6 +5,13 @@ test_description="duplicate message ids"
 add_message '[id]="duplicate"' '[subject]="message 1" [filename]=copy1'
 add_message '[id]="duplicate"' '[subject]="message 2" [filename]=copy2'
 
+test_begin_subtest 'First subject preserved'
+cat < EXPECTED
+thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; message 1 (inbox unread)
+EOF
+notmuch search id:duplicate | notmuch_search_sanitize > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest 'Search for second subject'
 cat 

Re: [PATCH 2/2] python: add convenience function to get named queries

[David Bremner]

l-...@web.de writes:

see above re: commit messages.

> ---
>  bindings/python/notmuch/database.py | 7 +++
>  1 file changed, 7 insertions(+)
>
> +
> +def get_all_named_queries(self):
> +"""Returns a dict of all named queries mapped to their search 
> queries.
> +
> +This function is a python extension and not in the underlying C API.
> +"""
> +return {k[6:]: v for k, v in self.get_config_list('query.')}

I have somewhat mixed feelings about this. I don't really like the
python bindings diverging from the C library.  It's also not clear it's
worth supporting a new API entry (since e.g. if this goes in it also
needs a test) to save the python client one line of code. On the
positive side I can see there is arguably a missing abstraction on the
library side, as those particular config items are special.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] python: add bindings for notmuch_database_get_config{, _list}

[David Bremner]

l-...@web.de writes:


Thanks for writing these bindings, it will be good to have the bindings
(almost) catch up to the library again.


We generally expect more than just a subject line in the commit message

   https://notmuchmail.org/contributing/#index5h2

> +def get_config(self, key):
> +"""Return the value of the given config key.

I guess we will eventually want set_config as well, even if it's not
needed for your immediate application. It might save future confusion to
add them both at the same time (unless there's something complicated
about adding set_config).

It would be good to add a couple tests. test/T590-libconfig.sh has some
C tests. I think the first one, labelled
"notmuch_database_{set,get}_config" could just be translated into python
(maybe even replace the C test with the python one, depending what
others think).

> +def get_config_list(self, prefix):

I don't object to the simplified interface, but I would like to know
what we can do if it becomes a performance bottleneck.  Would it be
possible to replace building the list with a generator (yield statement)
without changing client code? or should we take the leap now?
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch