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

[Pavel Labath via Phabricator via lldb-commits]

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. :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65489/new/

https://reviews.llvm.org/D65489