Eugene.Zelenko added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:96
+ start.append('--use-color')
+ elif use_color is not None:
+ start.append('--use-color=false')
----------------
kesyog wrote:
> Eugene.Zelenko wrote:
> > Shouldn't it be just `else:`?
> There are three cases:
>
> | Argument | `use_color` | Behavior |
> | `-use-color` | `True` | `--use-color` is passed to clang-tidy, force
> enabling color |
> | `-no-use-color` | `False` | `--use-color=false` is passed to clang-tidy,
> force disabling color |
> | (none provided) | `None` | Nothing passed to clang-tidy. clang-tidy follows
> its default coloring behavior |
>
> The case on the highlighted line is the second row of the table, and we have
> to check that `use_color` is not `None` to exclude the case of the third row.
>
> I was trying to avoid the extra nesting of something like below, but maybe
> the intent would be clearer?
> ```
> if use_color is not None:
> if use_color:
> start.append('--use-color')
> else:
> start.append('--use-color=false')
> ```
I think you implementation is incorrect. You should check for not `None` first
and than set proper value for `--use-color`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119562/new/
https://reviews.llvm.org/D119562
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
