Re: Inconsistencies in handling command flags: `--flag=value` different than `--flag value`

2020-04-27 Thread Ciprian Dorin Craciun
On Mon, Apr 27, 2020 at 9:21 PM Tomi Ollila  wrote:
> > On Mon 2020-04-27 14:53:07 -0300, David Bremner wrote:
> >> Quoting notmuch(1)
> >>
> >>OPTION SYNTAX
> >>All options accepting an argument can be used with '='
> >>or ':' as a separator. For the cases where it's not ambiguous
> >>(in particular excluding boolean options), a space can also be
> >>used.


I definitively skipped over that warning, mainly because I was reading
the man-page for the specific command (i.e. `notmuch-search`, etc.)
that don't feature that warning.

Please note that I understand "why" I get this behavior, and
definitively I agree that it's my fault.  However my initial report
was intended to find a way that new users don't shoot themselves in
the foot, especially since many will use `notmuch` from a script, and
sometimes they don't thoroughly check the arguments passed by the
user.



> > Alternately, we could deprecate using whitespace for all options,
> > produce explicit warnings to stderr when whitespace appears on the next
>
> was it so, that originally we did not support whitespace, but David
> added that in some commit...


>From a "correctness" point of view, this would be the best approach.
However I think it could be too late to introduce it, and it would
break too many integrations.



> > release, remove the suggestion to use a whitespace separator from the
> > documentation, and eventually phase it out entirely in some future
> > release.
>
> Alternatively we could check that next arg is (case-insensitively)
> (subset of) 'true', 'false', 'yes', 'no', '0', '1', 't', 'nil'
> (but not tpyoes of these ;) and in that case have that as an option
> value...


This would be perhaps the best approach.  However I don't think it
would solve the issues for integrators that would not see these
warnings in the logs, until it is too late.



> ... would that work better for human user who just wants to be
> fluent on command line -- frontends can then always use = and option
> values...


Perhaps there could be an additional option (either on the command
line or in the configuration) that would apply "strict" checking, and
not letting any other form except `--argument=value`, including the
boolean flags, and failing loudly.

I think this third option would enable much safer integrations.

(BTW, this "strict" option could also apply to the parsing of the
search terms, which most of the time are under the control of the end
user.)

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


Re: Inconsistencies in handling command flags: `--flag=value` different than `--flag value`

2020-04-27 Thread Tomi Ollila
On Mon, Apr 27 2020, Daniel Kahn Gillmor wrote:

> On Mon 2020-04-27 14:53:07 -0300, David Bremner wrote:
>> Quoting notmuch(1)
>>
>>OPTION SYNTAX
>>All options accepting an argument can be used with '='
>>or ':' as a separator. For the cases where it's not ambiguous
>>(in particular excluding boolean options), a space can also be
>>used.
>
> This is a pretty twisty way to say what we mean.  Are there other cases
> besides boolean options?  If there are, perhaps it'd be clearer to say
> something like this for the last sentence:
>
> Except for boolean options and other potential ambiguous cases, a
> space can also be used as a separator.
>
> If there aren't, we could say:
>
> Except for boolean options (which would be ambiguous), a space can
> also be used as a separator.
>
> Alternately, we could deprecate using whitespace for all options,
> produce explicit warnings to stderr when whitespace appears on the next

was it so, that originally we did not support whitespace, but David
added that in some commit...

> release, remove the suggestion to use a whitespace separator from the
> documentation, and eventually phase it out entirely in some future
> release.

Alternatively we could check that next arg is (case-insensitively)
(subset of) 'true', 'false', 'yes', 'no', '0', '1', 't', 'nil'
(but not tpyoes of these ;) and in that case have that as an option
value...

... would that work better for human user who just wants to be
fluent on command line -- frontends can then always use = and option
values...

> --dkg

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


Re: [PATCH] util/zlib-extra: de-inline gzerror_str

2020-04-27 Thread Tomi Ollila
On Mon, Apr 27 2020, David Bremner wrote:

> It turns out the behaviour of inline functions in C header files is
> not a good idea, and can cause linking problems if the compiler
> decides not to inline them.  In principle this is solvable by using a
> "static inline" declaration, but this potentially makes a copy in
> every compilation unit. Since we don't actually care about the
> performance of this function, just use a non-inline function.

LGTM.

Tomi

> ---
>  util/zlib-extra.c | 7 +++
>  util/zlib-extra.h | 4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/util/zlib-extra.c b/util/zlib-extra.c
> index 2d2d2414..3a75e504 100644
> --- a/util/zlib-extra.c
> +++ b/util/zlib-extra.c
> @@ -85,3 +85,10 @@ gz_error_string (util_status_t status, gzFile file)
>  else
>   return util_error_string (status);
>  }
> +
> +const char *
> +gzerror_str(gzFile file)
> +{
> +int dummy;
> +return gzerror (file, );
> +}
> diff --git a/util/zlib-extra.h b/util/zlib-extra.h
> index 296dc914..e9925c98 100644
> --- a/util/zlib-extra.h
> +++ b/util/zlib-extra.h
> @@ -29,8 +29,8 @@ gz_error_string (util_status_t status, gzFile stream);
>  
>  /* Call gzerror with a dummy errno argument, the docs don't promise to
>   * support the NULL case */
> -inline const char *
> -gzerror_str(gzFile file) { int dummy; return gzerror (file, ); }
> +const char *
> +gzerror_str(gzFile file);
>  
>  #ifdef __cplusplus
>  }
> -- 
> 2.26.2
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Inconsistencies in handling command flags: `--flag=value` different than `--flag value`

2020-04-27 Thread Daniel Kahn Gillmor
On Mon 2020-04-27 14:53:07 -0300, David Bremner wrote:
> Quoting notmuch(1)
>
>OPTION SYNTAX
>All options accepting an argument can be used with '='
>or ':' as a separator. For the cases where it's not ambiguous
>(in particular excluding boolean options), a space can also be
>used.

This is a pretty twisty way to say what we mean.  Are there other cases
besides boolean options?  If there are, perhaps it'd be clearer to say
something like this for the last sentence:

Except for boolean options and other potential ambiguous cases, a
space can also be used as a separator.

If there aren't, we could say:

Except for boolean options (which would be ambiguous), a space can
also be used as a separator.

Alternately, we could deprecate using whitespace for all options,
produce explicit warnings to stderr when whitespace appears on the next
release, remove the suggestion to use a whitespace separator from the
documentation, and eventually phase it out entirely in some future
release.


--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] util/zlib-extra: de-inline gzerror_str

2020-04-27 Thread Daniel Kahn Gillmor
On Mon 2020-04-27 09:28:08 -0300, David Bremner wrote:
> It turns out the behaviour of inline functions in C header files is
> not a good idea, and can cause linking problems if the compiler
> decides not to inline them.  In principle this is solvable by using a
> "static inline" declaration, but this potentially makes a copy in
> every compilation unit. Since we don't actually care about the
> performance of this function, just use a non-inline function.

LGTM.  No need for premature optimization in the error case anyway.

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Inconsistencies in handling command flags: `--flag=value` different than `--flag value`

2020-04-27 Thread David Bremner
Ciprian Dorin Craciun  writes:

> [Again sorry for double reporting.  BTW, where should I search for
> previous bugs?  I've currently tried the mailing list archive.]

Another option is

https://nmbug.notmuchmail.org/status

This is just pointers into the mailing archive, but triaged.

>
> Trying to play with `notmuch` from a wrapper, I've stumbled upon the
> following command line flags handling bug:
>
> 
> notmuch show --format json --entire-thread true --body false --
> 'cipr...@volution.ro'
> notmuch show --format json --entire-thread true --body=false --
> 'cipr...@volution.ro'

Quoting notmuch(1)

   OPTION SYNTAX
   All options accepting an argument can be used with '='
   or ':' as a separator. For the cases where it's not ambiguous
   (in particular excluding boolean options), a space can also be
   used.

So what you are seeing is a side effect of "--entire-thread true" not
being supported syntax.

You can see what's happening with

 NOTMUCH_DEBUG_QUERY=t notmuch show --format json --entire-thread true 
djkldsfjkl

the "true" is considered the first search term.

In your example you could just delete the "true" to switch on the
option.

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


Re: performance problems with notmuch new

2020-04-27 Thread David Bremner
Don Zickus  writes:

> On Fri, Apr 24, 2020 at 08:07:27PM -0300, David Bremner wrote:
>> Don Zickus  writes:
>> 
>> >
>> > Of course I am using this in a strange context.  I am deleting emails
>> > locally and then running 'notmuch new' to clean up the database.
>> >
>> [snip]
>> > So maybe my use case is unusual and the slowness is expected.
>> 
>> I'd say it's a known performance bug in notmuch that it doesn't deal
>> well with large numbers of deletions. At some point it is faster to
>> reindex the whole database from scratch.
>
> Does that mean 'notmuch dump' and 'notmuch restore'?  Willing to try that.
>

Exactly.

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


Re: performance problems with notmuch new

2020-04-27 Thread Don Zickus
On Fri, Apr 24, 2020 at 08:07:27PM -0300, David Bremner wrote:
> Don Zickus  writes:
> 
> >
> > Of course I am using this in a strange context.  I am deleting emails
> > locally and then running 'notmuch new' to clean up the database.
> >
> [snip]
> > So maybe my use case is unusual and the slowness is expected.
> 
> I'd say it's a known performance bug in notmuch that it doesn't deal
> well with large numbers of deletions. At some point it is faster to
> reindex the whole database from scratch.

Does that mean 'notmuch dump' and 'notmuch restore'?  Willing to try that.

Cheers,
Don

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


[PATCH] util/zlib-extra: de-inline gzerror_str

2020-04-27 Thread David Bremner
It turns out the behaviour of inline functions in C header files is
not a good idea, and can cause linking problems if the compiler
decides not to inline them.  In principle this is solvable by using a
"static inline" declaration, but this potentially makes a copy in
every compilation unit. Since we don't actually care about the
performance of this function, just use a non-inline function.
---
 util/zlib-extra.c | 7 +++
 util/zlib-extra.h | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/util/zlib-extra.c b/util/zlib-extra.c
index 2d2d2414..3a75e504 100644
--- a/util/zlib-extra.c
+++ b/util/zlib-extra.c
@@ -85,3 +85,10 @@ gz_error_string (util_status_t status, gzFile file)
 else
return util_error_string (status);
 }
+
+const char *
+gzerror_str(gzFile file)
+{
+int dummy;
+return gzerror (file, );
+}
diff --git a/util/zlib-extra.h b/util/zlib-extra.h
index 296dc914..e9925c98 100644
--- a/util/zlib-extra.h
+++ b/util/zlib-extra.h
@@ -29,8 +29,8 @@ gz_error_string (util_status_t status, gzFile stream);
 
 /* Call gzerror with a dummy errno argument, the docs don't promise to
  * support the NULL case */
-inline const char *
-gzerror_str(gzFile file) { int dummy; return gzerror (file, ); }
+const char *
+gzerror_str(gzFile file);
 
 #ifdef __cplusplus
 }
-- 
2.26.2

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


Re: [PATCH] emacs: Use `cl-lib' instead of deprecated `cl'

2020-04-27 Thread David Bremner
Jonas Bernoulli  writes:

> I have fixed the remaining issues and added two small commits.
> See v3.  
>
Great, thanks. I've pushed the main one, and tagged the other two for
review.

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