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