kesyog 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')
----------------
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')
```


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

Reply via email to