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

Jonas Devlieghere via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 31 13:12:00 PDT 2019


JDevlieghere added a comment.

In D65489#1608936 <https://reviews.llvm.org/D65489#1608936>, @labath wrote:

> In D65489#1608904 <https://reviews.llvm.org/D65489#1608904>, @JDevlieghere wrote:
>
> > I don't have any cleanups planned for now. My motivation is purely aesthetical: I don't like the `// clang-format off` markers and think the C arrays look messy with the multiline oddly broken up strings. Anyway, I just mention it for context. I don't want to push this through if the consensus is that this is overkill.
>
>
> 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 :-)

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


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

https://reviews.llvm.org/D65489





More information about the lldb-commits mailing list