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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
