This is an archive of the discontinued LLVM Phabricator instance.

[llvm-ar] Uncapitalize and delete full stop from error messages
ClosedPublic

Authored by MaskRay on Sep 13 2019, 9:05 AM.

Details

Summary

Most GNU binutils don't append full stops in error messages. This
convention has been adopted by a bunch of LLVM binary utilities. Make
llvm-ar follow the convention as well.

Diff Detail

Event Timeline

MaskRay created this revision.Sep 13 2019, 9:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2019, 9:05 AM
grimar added inline comments.Sep 13 2019, 9:10 AM
tools/llvm-ar/llvm-ar.cpp
1139

Should we remove ! too?

grimar added inline comments.Sep 13 2019, 9:15 AM
tools/llvm-ar/llvm-ar.cpp
386

btw, below modifiers are sometimes wrapped into ''. May be worth to make this consistent in messages while you are here too?

MaskRay updated this revision to Diff 220122.Sep 13 2019, 9:17 AM
MaskRay marked an inline comment as done.

Delete !

MaskRay updated this revision to Diff 220125.Sep 13 2019, 9:24 AM
MaskRay marked an inline comment as done and an inline comment as not done.

Wrap operation names in ''

tools/llvm-ar/llvm-ar.cpp
386

Wrapped in '' for consistency.

It seems many error messages here are no tested. This is something we should improve in the future.

grimar accepted this revision.Sep 13 2019, 9:31 AM

LGTM with a nits.

tools/llvm-ar/llvm-ar.cpp
212

nit: needs wrapping too it seems.

221

The same.

386

Yeah. I also found one more inconsistency:

"cannot mix -M and other options"

Seems in other places we use the word "operation", but here the word "option" with a dash.
It is anyways out of the area of this patch, just had to mention :)

This revision is now accepted and ready to land.Sep 13 2019, 9:31 AM
MaskRay updated this revision to Diff 220129.Sep 13 2019, 9:44 AM
MaskRay marked 5 inline comments as done.

Wrap two other error messages

This revision was automatically updated to reflect the committed changes.