[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Austin Clements]

On Mon, Oct 31, 2011 at 6:07 PM, Jani Nikula  wrote:
> On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins  finestructure.net> wrote:
>> In order to push forward with this, though, I think we really need to
>> have a complete unit test for this new functionality. ?We usually like
>> to see units tests that describe and then test for the new functionality
>> you wish to add, followed by the patches that provide the new
>> functionality. ?Lots of good tests for new functionality being proposed
>> here shouldn't be too difficult to work out ahead of time.
>
> Right. I'd just like to make sure the approach I've taken (particularly
> patch 1 in the set as it touches the lib) is acceptable before spending
> time on testing and documentation etc. Indeed patches 1 and 2 changed
> fundamentally between v1 and v2 after some chats on IRC. If the comments
> now are favourable, I'll write the tests and documentation. (Though I
> guess I have to admit the tests would've been beneficial to me already
> now...)

The library interface looks perfectly reasonable and consistent to me.
 My only concern would be that there's no way to return errors from
notmuch_query_count_threads, but notmuch_query_count_messages has
exactly the same problem.

Other than that, you missed a few spaces before parentheses.

Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Austin Clements]

On Mon, Oct 31, 2011 at 6:07 PM, Jani Nikula  wrote:
> On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins 
>  wrote:
>> In order to push forward with this, though, I think we really need to
>> have a complete unit test for this new functionality.  We usually like
>> to see units tests that describe and then test for the new functionality
>> you wish to add, followed by the patches that provide the new
>> functionality.  Lots of good tests for new functionality being proposed
>> here shouldn't be too difficult to work out ahead of time.
>
> Right. I'd just like to make sure the approach I've taken (particularly
> patch 1 in the set as it touches the lib) is acceptable before spending
> time on testing and documentation etc. Indeed patches 1 and 2 changed
> fundamentally between v1 and v2 after some chats on IRC. If the comments
> now are favourable, I'll write the tests and documentation. (Though I
> guess I have to admit the tests would've been beneficial to me already
> now...)

The library interface looks perfectly reasonable and consistent to me.
 My only concern would be that there's no way to return errors from
notmuch_query_count_threads, but notmuch_query_count_messages has
exactly the same problem.

Other than that, you missed a few spaces before parentheses.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jani Nikula]

On Tue, 01 Nov 2011 09:30:56 -0300, David Bremner  wrote:
> Is just because it add a function to the library that you think this
> might be problematic?  I don't think we are super-dogmatic about the
> library never growing.  When notmuch started, there were no bindings, so
> in retrospect maybe more functionality went into the CLI than might
> happen if we started from scratch. If I remember Carl's statement
> correctly, one rule is that stuff in the library should not require
> configuration.

Hi, thanks, that's encouraging. It's mostly that, based on past
experience, I'm hesitant about extending a library interface. Once you
get users, you have to live with it. So you want to get it right.

Having said that, I think in this v2 of the set, the library interface
for notmuch_query_count_threads() is sane and complimentary to the
existing notmuch_query_count_messages(). (Also quoting IRC: "
j4ni: Yes, the interface seems totally reasonable.") Later on, someone
might come up with a better implementation, but for now it's already
much better than having to iterate and construct the threads.

I'll prepare a v3 with some tests and polish.

BR,
Jani.

Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jani Nikula]

On Tue, 01 Nov 2011 09:30:56 -0300, David Bremner  wrote:
> Is just because it add a function to the library that you think this
> might be problematic?  I don't think we are super-dogmatic about the
> library never growing.  When notmuch started, there were no bindings, so
> in retrospect maybe more functionality went into the CLI than might
> happen if we started from scratch. If I remember Carl's statement
> correctly, one rule is that stuff in the library should not require
> configuration.

Hi, thanks, that's encouraging. It's mostly that, based on past
experience, I'm hesitant about extending a library interface. Once you
get users, you have to live with it. So you want to get it right.

Having said that, I think in this v2 of the set, the library interface
for notmuch_query_count_threads() is sane and complimentary to the
existing notmuch_query_count_messages(). (Also quoting IRC: "
j4ni: Yes, the interface seems totally reasonable.") Later on, someone
might come up with a better implementation, but for now it's already
much better than having to iterate and construct the threads.

I'll prepare a v3 with some tests and polish.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[David Bremner]

On Mon, 31 Oct 2011 23:18:07 +0200, Jani Nikula  wrote:
> 
> I'm still marking it as RFC. It works for me, but patch 1 might be deemed
> unacceptable.
> 

Hi Jani;

Is just because it add a function to the library that you think this
might be problematic?  I don't think we are super-dogmatic about the
library never growing.  When notmuch started, there were no bindings, so
in retrospect maybe more functionality went into the CLI than might
happen if we started from scratch. If I remember Carl's statement
correctly, one rule is that stuff in the library should not require
configuration.

Just my personal view,

David

Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[David Bremner]

On Mon, 31 Oct 2011 23:18:07 +0200, Jani Nikula  wrote:
> 
> I'm still marking it as RFC. It works for me, but patch 1 might be deemed
> unacceptable.
> 

Hi Jani;

Is just because it add a function to the library that you think this
might be problematic?  I don't think we are super-dogmatic about the
library never growing.  When notmuch started, there were no bindings, so
in retrospect maybe more functionality went into the CLI than might
happen if we started from scratch. If I remember Carl's statement
correctly, one rule is that stuff in the library should not require
configuration.

Just my personal view,

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

[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jani Nikula]

On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins  wrote:
> Hi, Jani.  Thanks for working on this.  This should also be valuable for
> vim users.

Thanks for your interest! :)

> In order to push forward with this, though, I think we really need to
> have a complete unit test for this new functionality.  We usually like
> to see units tests that describe and then test for the new functionality
> you wish to add, followed by the patches that provide the new
> functionality.  Lots of good tests for new functionality being proposed
> here shouldn't be too difficult to work out ahead of time.

Right. I'd just like to make sure the approach I've taken (particularly
patch 1 in the set as it touches the lib) is acceptable before spending
time on testing and documentation etc. Indeed patches 1 and 2 changed
fundamentally between v1 and v2 after some chats on IRC. If the comments
now are favourable, I'll write the tests and documentation. (Though I
guess I have to admit the tests would've been beneficial to me already
now...)

BR,
Jani.

[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jani Nikula]

Hi, this is an iteration of id:"cover.1319833617.git.jani at nikula.org" 
addressing
comments on the list and IRC. Main changes:

* Results are now limited based on threads (not messages) for thread and summary
  output. This is accomplished with a new lib function to count the number of
  threads in matching messages.

* cli part is now inspired by James Vasile's patch
  id:"8739gyw0zh.fsf at opensourcematters.org", with the additional ability to
  limit from the end of result set.

* Bugs reported by Daniel Schoepe fixed.

* Don't show buttons if everything is visible already.

I'm still marking it as RFC. It works for me, but patch 1 might be deemed
unacceptable.


BR,
Jani.


Jani Nikula (3):
  lib: add function to get the number of threads matching a search
  cli: add options --first and --maxitems to notmuch search
  emacs: support limiting the number of results shown in search results

 emacs/notmuch-hello.el |   17 ++--
 emacs/notmuch.el   |   53 ++---
 lib/notmuch.h  |   14 ++
 lib/query.cc   |   40 
 notmuch-search.c   |   67 +++
 5 files changed, 171 insertions(+), 20 deletions(-)

-- 
1.7.5.4

Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jameson Graef Rollins]

On Tue, 01 Nov 2011 00:07:59 +0200, Jani Nikula  wrote:
> On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins 
>  wrote:
> Right. I'd just like to make sure the approach I've taken (particularly
> patch 1 in the set as it touches the lib) is acceptable before spending
> time on testing and documentation etc. Indeed patches 1 and 2 changed
> fundamentally between v1 and v2 after some chats on IRC. If the comments
> now are favourable, I'll write the tests and documentation. (Though I
> guess I have to admit the tests would've been beneficial to me already
> now...)

I would say that since this is functionality that we do eventually want,
a known_broken test suite that tests for the functionality we want to
see would be very useful right from the start, even before any patches
to supply the functionality are made.  With the tests in hand we can
iterate over exactly what we want the ui to provide, and then worry
about the specific implementation.

The documentation can come later with the actual feature patches.

jamie.


pgpvHaBzQPqjo.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jameson Graef Rollins]

On Tue, 01 Nov 2011 00:07:59 +0200, Jani Nikula  wrote:
> On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins  finestructure.net> wrote:
> Right. I'd just like to make sure the approach I've taken (particularly
> patch 1 in the set as it touches the lib) is acceptable before spending
> time on testing and documentation etc. Indeed patches 1 and 2 changed
> fundamentally between v1 and v2 after some chats on IRC. If the comments
> now are favourable, I'll write the tests and documentation. (Though I
> guess I have to admit the tests would've been beneficial to me already
> now...)

I would say that since this is functionality that we do eventually want,
a known_broken test suite that tests for the functionality we want to
see would be very useful right from the start, even before any patches
to supply the functionality are made.  With the tests in hand we can
iterate over exactly what we want the ui to provide, and then worry
about the specific implementation.

The documentation can come later with the actual feature patches.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jani Nikula]

On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins 
 wrote:
> Hi, Jani.  Thanks for working on this.  This should also be valuable for
> vim users.

Thanks for your interest! :)

> In order to push forward with this, though, I think we really need to
> have a complete unit test for this new functionality.  We usually like
> to see units tests that describe and then test for the new functionality
> you wish to add, followed by the patches that provide the new
> functionality.  Lots of good tests for new functionality being proposed
> here shouldn't be too difficult to work out ahead of time.

Right. I'd just like to make sure the approach I've taken (particularly
patch 1 in the set as it touches the lib) is acceptable before spending
time on testing and documentation etc. Indeed patches 1 and 2 changed
fundamentally between v1 and v2 after some chats on IRC. If the comments
now are favourable, I'll write the tests and documentation. (Though I
guess I have to admit the tests would've been beneficial to me already
now...)

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jameson Graef Rollins]

On Mon, 31 Oct 2011 23:18:07 +0200, Jani Nikula  wrote:
> Hi, this is an iteration of id:"cover.1319833617.git.j...@nikula.org" 
> addressing
> comments on the list and IRC. Main changes:
> 
> * Results are now limited based on threads (not messages) for thread and 
> summary
>   output. This is accomplished with a new lib function to count the number of
>   threads in matching messages.
> 
> * cli part is now inspired by James Vasile's patch
>   id:"8739gyw0zh@opensourcematters.org", with the additional ability to
>   limit from the end of result set.
> 
> * Bugs reported by Daniel Schoepe fixed.
> 
> * Don't show buttons if everything is visible already.
> 
> I'm still marking it as RFC. It works for me, but patch 1 might be deemed
> unacceptable.

Hi, Jani.  Thanks for working on this.  This should also be valuable for
vim users.

In order to push forward with this, though, I think we really need to
have a complete unit test for this new functionality.  We usually like
to see units tests that describe and then test for the new functionality
you wish to add, followed by the patches that provide the new
functionality.  Lots of good tests for new functionality being proposed
here shouldn't be too difficult to work out ahead of time.

For instance, here's an example of a test that I would like to see:

test_begin_subtest "maxitems does the right thing"
notmuch search tag:foo | head -n 20 >expected
notmuch search --maxitems=20 tag:foo >output
test_expect_equal_file expected output

test_begin_subtest "concatenation of limited searches does the right thing"
notmuch search tag:foo | head -n 20 >expected
notmuch search --maxitems=10 tag:foo >output
notmuch search --maxitems=10 --first=10 tag:foo >>output
test_expect_equal_file expected output


jamie.


pgpGy6jvj8t0K.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jameson Graef Rollins]

On Mon, 31 Oct 2011 23:18:07 +0200, Jani Nikula  wrote:
> Hi, this is an iteration of id:"cover.1319833617.git.jani at nikula.org" 
> addressing
> comments on the list and IRC. Main changes:
> 
> * Results are now limited based on threads (not messages) for thread and 
> summary
>   output. This is accomplished with a new lib function to count the number of
>   threads in matching messages.
> 
> * cli part is now inspired by James Vasile's patch
>   id:"8739gyw0zh.fsf at opensourcematters.org", with the additional ability to
>   limit from the end of result set.
> 
> * Bugs reported by Daniel Schoepe fixed.
> 
> * Don't show buttons if everything is visible already.
> 
> I'm still marking it as RFC. It works for me, but patch 1 might be deemed
> unacceptable.

Hi, Jani.  Thanks for working on this.  This should also be valuable for
vim users.

In order to push forward with this, though, I think we really need to
have a complete unit test for this new functionality.  We usually like
to see units tests that describe and then test for the new functionality
you wish to add, followed by the patches that provide the new
functionality.  Lots of good tests for new functionality being proposed
here shouldn't be too difficult to work out ahead of time.

For instance, here's an example of a test that I would like to see:

test_begin_subtest "maxitems does the right thing"
notmuch search tag:foo | head -n 20 >expected
notmuch search --maxitems=20 tag:foo >output
test_expect_equal_file expected output

test_begin_subtest "concatenation of limited searches does the right thing"
notmuch search tag:foo | head -n 20 >expected
notmuch search --maxitems=10 tag:foo >output
notmuch search --maxitems=10 --first=10 tag:foo >>output
test_expect_equal_file expected output


jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

[RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results

[Jani Nikula]

Hi, this is an iteration of id:"cover.1319833617.git.j...@nikula.org" addressing
comments on the list and IRC. Main changes:

* Results are now limited based on threads (not messages) for thread and summary
  output. This is accomplished with a new lib function to count the number of
  threads in matching messages.

* cli part is now inspired by James Vasile's patch
  id:"8739gyw0zh@opensourcematters.org", with the additional ability to
  limit from the end of result set.

* Bugs reported by Daniel Schoepe fixed.

* Don't show buttons if everything is visible already.

I'm still marking it as RFC. It works for me, but patch 1 might be deemed
unacceptable.


BR,
Jani.


Jani Nikula (3):
  lib: add function to get the number of threads matching a search
  cli: add options --first and --maxitems to notmuch search
  emacs: support limiting the number of results shown in search results

 emacs/notmuch-hello.el |   17 ++--
 emacs/notmuch.el   |   53 ++---
 lib/notmuch.h  |   14 ++
 lib/query.cc   |   40 
 notmuch-search.c   |   67 +++
 5 files changed, 171 insertions(+), 20 deletions(-)

-- 
1.7.5.4

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