correlates the option with reponse file concept.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/docs/ClangFormat.rst | ||
---|---|---|
29 | if response files are so common place why not say it here clang-format [options] [@file] I personally don't believe we need to document how a shell works | |
72–73 | if you google "response files" you don't get anything that leads to understand what they are. I understand what it means but I'm not convinced others who have not come across it before will. I don't personally think this change adds anything. |
The option handling in clang-format itself would need to be updated as well.
clang/docs/ClangFormat.rst | ||
---|---|---|
29 | Pretty sure it's not the shell doing the expansion here? Certainly Clang has explicit code for deciding between gnu-style and windows-style handling. |
Its Is a minor point, but when using --files, the shell will allow me to autocomplete..
clang-format -verbose -n -files ./cla<TAB> clang-format -verbose -n -files ./clang/d<TAB> etc... all the way to the file clang-format -verbose -n -files ./clang/docs/tools/clang-formatted-files.txt
with @ I have to be pixel perfect in my path.. so I'm more likely to go wrong
clang-format -verbose -n @./cla<TAB> does nothing.
that feels like a positive for the --files.
clang/docs/ClangFormat.rst | ||
---|---|---|
72–74 | I was thinking something like this. In particular, call the argument a "filename" and then say what it should look like. The old description with a "string" argument reads like it might want to be |
The help text in ClangFormat.cpp for the --files option needs to be updated the same way.
Note using cl::value_desc("filename") is what you need to change the meta-variable in the help output.
It's too long ago for me. Does the --files option take multiple file names on the command line, or a file containing the file names? If the latter the patch generally looks good.
Except for the @<file> part. (Which also isn't handles by clang-format, or is it?)
It is not. It is handled by the LLVM commandline library. https://llvm.org/docs/CommandLine.html#response-files
It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an NFC or refactoring, adding documentation etc..)
Add your unit tests in clang/unittests/Format and you can build with ninja FormatTests. We recommend using the verifyFormat(xxx) format of unit tests rather than EXPECT_EQ as this will ensure you change is tolerant to random whitespace changes (see FormatTest.cpp as an example)
For situations where your change is altering the TokenAnnotator.cpp which can happen if you are trying to improve the annotation phase to ensure we are correctly identifying the type of a token, please add a token annotator test in TokenAnnotatorTest.cpp
clang/docs/ClangFormat.rst | ||
---|---|---|
72–74 | Fixed in c47c480b1845. |
if response files are so common place why not say it here
clang-format [options] [@file]
I personally don't believe we need to document how a shell works