[PATCH 3/8] CLI: replace some constructs with more uncrustify friendly ones

Daniel Kahn Gillmor dkg at fifthhorseman.net
Sun Jun 16 10:09:47 PDT 2019


On Thu 2019-06-13 08:08:32 -0300, David Bremner wrote:
>    - add parens in some ternery operators

itym "ternary"

> @@ -120,13 +120,13 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *
>  static int _opt_set_count (const notmuch_opt_desc_t *opt_desc)
>  {
>      return
> -	!!opt_desc->opt_inherit +
> -	!!opt_desc->opt_bool +
> -	!!opt_desc->opt_int +
> -	!!opt_desc->opt_keyword +
> -	!!opt_desc->opt_flags +
> -	!!opt_desc->opt_string +
> -	!!opt_desc->opt_position;
> +	(bool) opt_desc->opt_inherit +
> +	(bool) opt_desc->opt_bool +
> +	(bool) opt_desc->opt_int +
> +	(bool) opt_desc->opt_keyword +
> +	(bool) opt_desc->opt_flags +
> +	(bool) opt_desc->opt_string +
> +	(bool) opt_desc->opt_position;
>  }

i find this is deeply weird.  It looks like it is coercing various types
into bools, and then summing a list of bools.

While the spec might well say that the sum of two bools should be an int
(i haven't checked), it's not at all obvious to me that the infix +
operator should assume that type.  (float + float is a float, not an
int, for example)

in some sense, the !! operator works better here because i know that its
output is likely to be an int, so summing makes sense.

I can live with this if we need it for making uncrustify nicer, but i
just wanted to register that it looks to me like a regression in terms
of readability.

   --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20190616/02c3dd54/attachment.sig>


More information about the notmuch mailing list