Page MenuHomePhabricator

[Support] Indent multi-line descr of enum cli options.
ClosedPublic

Authored by fodinabor on Dec 17 2020, 2:58 PM.

Details

Summary

As noted in https://reviews.llvm.org/D93459, the formatting of
multi-line descriptions of clEnumValN and the likes is unfavorable.
Thus this patch adds support for correctly indenting these.

Diff Detail

Event Timeline

fodinabor created this revision.Dec 17 2020, 2:58 PM
fodinabor requested review of this revision.Dec 17 2020, 2:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2020, 2:58 PM
fodinabor updated this revision to Diff 312752.Dec 18 2020, 4:23 AM

Fix naming to make clang-tidy happy.

Happy new year's ping :)

Would be good to have a test entry for that new pretty printer . That would also make the review easier ;-)

fodinabor updated this revision to Diff 319247.Jan 26 2021, 1:58 AM

Add print option info test for multiline value descriptions.

Given the awful formatting in clang-format is merged in release/12.x, would be great to get this in, before 12.0.0 final.. :)

serge-sans-paille accepted this revision.Feb 3 2021, 3:18 PM

LGTM, thanks for providing a test case!

This revision is now accepted and ready to land.Feb 3 2021, 3:18 PM
This revision was automatically updated to reflect the committed changes.
chapuni added a subscriber: chapuni.May 1 2021, 6:17 AM

It might causes assertion failure when a name in enum is long.
Reproducible with clang-scan-deps -help

llvm/lib/Support/CommandLine.cpp
1731

This is not reflected to MaxWidth.

1948

Could we adjust it with ValHelpPrefix?

mhh... I'd just say the assert was wrong.
The BaseIndent is the maximum width of an option. e.g. --compilation-database=<string>.
printEnumValHelpStr does not deal with that part at all, only with the help string that comes after that (e.g. This is the first enum value\nwhich has a really long description\nthus it is multi-line.).
For nice formatting in the first line - and are prepended for the enum values in the first line and later BaseIndent + are used for the indent.
What the assert should have verified is only that BaseIndent >= FirstLineIndentedBy so that we can later have the indent: BaseIndent - FirstLineIndentedBy and be sure it's >=0.
(and that really should always be true..)

So I will just adapt the assert..