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

[Daniel Kahn Gillmor <dkg@fifthhorseman.net>]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]