[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 1 00:31:27 PDT 2019

labath added a comment.

In D65489#1609001 <https://reviews.llvm.org/D65489#1609001>, @JDevlieghere wrote:

> In D65489#1608936 <https://reviews.llvm.org/D65489#1608936>, @labath wrote:
> > Yeah, the C arrays aren't the prettiest sight, but OTOH your tablegen files don't respect the column limit, and so if you view them with a line-wrapping editor (such as this phabricator page), they don't look particularly nice either.
> True, but at least you can grep without having to guess where the ling break might have ended up :-)

Yeah, the grepping thing isn't nice, but I think working around that by putting the strings into tablegen is cheating  -- you're exploiting the fact that we don't have a code formatting tool nor a formal style guide for tablegen files. A lot of the official style guide cannot be directly applied to the tablegen files, but the column limit is one of the few things that can. :D

It looks like llvm tablegen files are not strictly adhering to that, but they are written with that limit in mind -- ascii art is 80 chars wide, and only 2.5% of lines are 81 chars or over.

>> I'm not sure the result is better, but it is possible to control the way clang-format lays out arrays like this via trailing commas. If you include a trailing comma, it will put each entry on a separate line, which may be better for those long strings (though that's highly subjective). E.g.:
>>   static constexpr OptionEnumValueElement s_stop_show_column_values[] = {
>>       {
>>           eStopShowColumnAnsiOrCaret,
>>           "ansi-or-caret",
>>           "Highlight the stop column with ANSI terminal codes when color/ANSI "
>>           "mode is enabled; otherwise, fall back to using a text-only caret (^) "
>>           "as if \"caret-only\" mode was selected.",
>>       },
>>       {
>>           eStopShowColumnAnsi,
>>           "ansi",
>>           "Highlight the stop column with ANSI terminal codes when running LLDB "
>>           "with color/ANSI enabled.",
>>       },
>>       {
>>           eStopShowColumnCaret,
>>           "caret",
>>           "Highlight the stop column with a caret character (^) underneath the "
>>           "stop column. This method introduces a new line in source listings "
>>           "that display thread stop locations.",
>>       },
>>       {
>>           eStopShowColumnNone,
>>           "none",
>>           "Do not highlight the stop column.",
>>       },
>>   };
> This looks much, much better imho. I can live with reformatting them if we don't want to go the tablegen route.

Great, then I propose we go for that. :)



More information about the lldb-commits mailing list