This is the first patch towards creating the llvm-mt tool for merging
Windows manifests. This is a reimplementation of mt.exe.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
llvm/tools/llvm-mt/Opts.td | ||
---|---|---|
3 ↗ | (On Diff #106335) | By convention, option names are written in lowercase. So unsupported instead of UNSUPPORTED. Apply all other options in this file. |
6 ↗ | (On Diff #106335) | Help text shouldn't end with a full stop. |
llvm/tools/llvm-mt/llvm-mt.cpp | ||
48 ↗ | (On Diff #106335) | This open parenthesis is not at the same indentation depth as the closing one. |
66 ↗ | (On Diff #106335) | It is convenient to write out "\n" here, instead of embedding "\n" as part of Msg. |
71 ↗ | (On Diff #106335) | Please follow the LLVM naming convention. |
91 ↗ | (On Diff #106335) | ; -> : |
117 ↗ | (On Diff #106335) | Remove this blank line. |
It's different. Perhaps it looks the same because you are looking at the option IDs which I left capitalized, which is actually the convention. However the actual option text that is matched and displayed in the helptext was moved to lower case. All the other changes you mentioned were also performed.
Is all uppercase really a convention? I took a quick look at a few .td files in clang, and looks like they write option ids in lowercase.
hmm it seems the convention is capital first letter, rest lower case
llvm/tools/llvm-mt/llvm-mt.cpp | ||
---|---|---|
48 ↗ | (On Diff #106335) | shoot sorry for some reason git format keeps changing this. |
66 ↗ | (On Diff #106335) | hmmm in most recent revision it should be done |
71 ↗ | (On Diff #106335) | on most recent revision this should be fine |
llvm/tools/llvm-mt/llvm-mt.cpp | ||
---|---|---|
48 ↗ | (On Diff #106744) | Align the trailing \. |
90 ↗ | (On Diff #106744) | Error messages should start with lowercase letter. |
103 ↗ | (On Diff #106744) | Ditto |
113 ↗ | (On Diff #106744) | Ditto |
66 ↗ | (On Diff #106335) | Error messages shouldn't end with a full stop. See https://clang.llvm.org/diagnostics.html for example. |
71 ↗ | (On Diff #106335) | Is it? argc and argv should be Argc and Argv. |
remove error full stops
llvm/tools/llvm-mt/llvm-mt.cpp | ||
---|---|---|
71 ↗ | (On Diff #106335) | Hmmm I'm not sure if Argc and Argv are the convention in this case. In all the other "mains" in llvm/tools there is variation between using argc, argc_ and *argv[], argv, *argv_[]. The most common combination is argc and argv. |
LGTM
llvm/tools/llvm-mt/llvm-mt.cpp | ||
---|---|---|
71 ↗ | (On Diff #106335) | If that is a convention, that is fine, but I hope you mentioned that instead of just marking this as done. |