[PATCH 5/7] CLI: add --leak-report top level option

[Tomi Ollila]

On Sun, Jan 20 2013, Jameson Graef Rollins <jrollins at finestructure.net> wrote:

> On Sat, Jan 19 2013, david at tethera.net wrote:
>> This roughly mimics the samba4 argument. The presence of the command
>> line argument overrides any value of NOTMUCH_TALLOC_REPORT in the
>> environment.
>> ---
>>  man/man1/notmuch.1 |    8 ++++++++
>>  notmuch.c          |   18 +++++++-----------
>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
>> index 6bf9b2e..5c58c41 100644
>> --- a/man/man1/notmuch.1
>> +++ b/man/man1/notmuch.1
>> @@ -70,6 +70,14 @@ Print a synopsis of available commands and exit.
>>  Print the installed version of notmuch, and exit.
>>  .RE
>>  
>> +.RS 4
>> +.TP 4
>> +.BI \-\-leak-report= path
>> +
>> +Write a detailed report of all memory allocated via talloc to
>> +.I path
>> +.RE
>
> Do we really need a command line option for this?  Why isn't the env var
> sufficient?  This just seems to me like it clutters the interface, for
> an option that is purely for debugging and should rarely if ever be used
> by most users.

Jameson does have a point. Now that we already have that environment
variable and it can be used in shipped notmuch 0.15 it is perhaps
simplest just to stick with that.

My thoughts after brief first visit to the patche series has so far
being either make the command line usage 1:1 compatible with samba
or use option like --leak-report-output=..., --leak-report-file=... or
--leak-report-to=... (and attempt to deprecate the env var...)

That said, I withdraw my previous suggestion of the command line option...

The other changes in this patch series looks initially good -- and changes
that drop deprecated features should be get in as early after last release
as possible.

> jamie.


Tomi