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

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

Date: Sun, 16 Jun 2019 13:09:47 -0400

To: David Bremner, notmuch@notmuchmail.org

Cc:

From: Daniel Kahn Gillmor


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
signature.asc (application/pgp-signature)
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: