Page MenuHomePhabricator

*_OPTIONS="help=1" should print to stdout and fit 80 chars width

Authored by marxin on Feb 24 2016, 2:54 AM.



I would like to modify printing of help options in order to fix:

I basically split all * options with new lines. Eventually, these
string are printed to limit the output to 80 characters:

Diff Detail

Event Timeline

marxin updated this revision to Diff 48902.Feb 24 2016, 2:54 AM
marxin retitled this revision from to *_OPTIONS="help=1" should print to stdout and fit 80 chars width.
marxin updated this object.
marxin added a subscriber: llvm-commits.
beanz added a subscriber: beanz.Feb 24 2016, 12:58 PM

I don't really agree with the premise or direction of this proposal.

LLVM does not have a convention for limiting output to 80 columns, and I don't believe we should be conforming to another project's conventions when it would introduce inconsistency across our projects.

I would feel differently about this if we made a decision that all LLVM projects should conform in this way (although I prefer that we don't).

kcc edited edge metadata.Feb 25 2016, 1:09 PM

Agree about 80 char limit -- we have no good reason to enforce it here.
I also don't want to change the default output from stderr to stdout.
the sanitizers print to stderr by default for a set of reasons, and I don't see why help=1 should use a different stream.

marxin updated this revision to Diff 57158.May 13 2016, 4:04 AM
marxin edited edge metadata.

I understand that you do not want to change the current behavior. Would it be possible to make
the dumping code more portable so that providing a compiler macro (_GNU_HELP_OUTPUT)
would produce output in a way the GCC would like to? It would just require a minimal change
to GCC's build system.

I verified that the change does not affect output for LLVM compiler.


kcc added a comment.May 13 2016, 9:11 AM

I'd like to avoid having one extra ifdef in the code.
Especially, if it will cause difference in behavior between the LLVM and GCC versions.

Frankly, I don't understand why this bothers you that much.

beanz requested changes to this revision.May 13 2016, 9:39 AM
beanz added a reviewer: beanz.

@marxin The patch relies on the maintainers of compiler-rt to format our text in 80-col limits, then it optionally converts from 80-col to the current formatting.

This patch also only addresses the help output. I'm not very familiar with the GNU conventions, do they apply to the tool output as well?

This imposes burden on our community to conform to another community's coding standards. I believe that is fundamentally unacceptable. It is certainly outside the realms of a discussion for a patch review.

The larger question of whether or not our community should conform to GNU conventions in Compiler-RT (and maintain that conformance) is probably something that would be more appropriate in an LLVM-Dev discussion. That said, I'm violently opposed to enforcing those conventions on our code.

If you want to go down the approach of a compile-time or runtime configuration to conform to GNU conventions that doesn't warrant a larger discussion you'll need to construct the patch in a way that doesn't impose additional burden on the project maintainers.

This revision now requires changes to proceed.May 13 2016, 9:39 AM
marxin abandoned this revision.Mar 14 2018, 1:49 AM