I wonder if Phabricator has an ability to highlight trailing whitespaces, because I often find trailing whitespaces after they are submitted. If they were highlighted, they would have been removed before submitting. Does anyone know about this?
Hi,
clang-format removes them.
If you are (like me) likely to forget to run "git clang-format HEAD~” before pushing, adding a pre-commit (or pre-push) hook is helpful.
When I create a patch, I run clang-format-diff, so that’s fine. What I want to do is to find trailing whitespaces in others patches.
I see! That’d be nice to have indeed.
Ideally I’d even really like to have a both checking for revision on phab, clang-formatting them, and post a comment when there is a mismatch 
... as bot comments. Yeah, that would be very useful.
I'd like that!
I’m not sure how easy it is to get clang-format into Phabricator since it is mostly developed by (phab) upstream. I’ll file feature request regarding trailing whitespaces and clang-format to upstream to see what upstream forks think.
But a bot checking revision sounds great. It might also be useful to have the bot run clang-tidy and post findings/FixHints as comments on the patch.
So, I forwarded the request for highlighting trailing whitespaces to phabricator upstream (https://secure.phabricator.com/T11879), and upstream folks suggest we enable the Lint feature in Arcanist (https://secure.phabricator.com/book/phabricator/article/arcanist_lint/). This will enforce the check when arc diff is run (reviewers wouldn’t see the warnings though).
There are two linters we might be interested in enabling:
- cpplint (code style checker based on cpplint.py)
- cppcheck (C++ linter based on cppcheck)
Note that cpplint assumes google code style, but I think it can potentially be replaced it with clang-format with configurable code styles.
We have a clang format based arcanist linter (and some others) in the
Polly repository. When arcanist is used to create a review, the linter
result is shown online. We also have an arcanist add-on to run the lit
tests and show their result in the review as well.
The problem is basically that not many ppl use arcanist...
Why isn’t it in the LLVM repo?
I assume those arc linters run on the client side? And people need to have the various tools installed before all of that works?
Just to throw out another idea: Running a linter on the server side and let it comment on phab reviews would be the perfect project to get some experience with pre-commit testing/hooks for llvm
- Matthias
Hey Mehdi, Hey Matthias,
I assume those arc linters run on the client side? And people need to have the various tools installed before all of that works?
Yes they run client side but there are no "tools" needed except
arcanist (we talk about arcanist linters after all).
Here is the .arclint file in the Polly repo:
{
"linters": {
"format": {
"include": "(include/polly/.+\\.h$|lib/.+\\.cpp$)",
"exclude": "(lib/JSON/.*)",
"type": "script-and-regex",
"script-and-regex.script": "sh -c './utils/check_format.sh \"$0\" 2> /dev/null || true'",
"script-and-regex.regex": "/^(OK:(?P<ignore>.+)|Error:) (?P<message>.+)$/m"
},
"chmod": {
"type": "chmod"
},
"filename": {
"exclude": "(www/experiments/.+|.*\\.jscop.*)",
"type": "filename"
},
"merge-conflict": {
"type": "merge-conflict"
},
"spelling": {
"exclude": "(configure|autoconf/.*)",
"type": "spelling"
}
}
}
We, or better me at some point, used various linters provided by arcanist and
one custom "script-and-regex" linter based on the check_format.sh script that
wraps clang-format. While it is apparently not present in the repository
anymore [-.-], we used it at some point in the Polly tests ("check-polly") and
the buildbots to verify the source formatting.
Just to throw out another idea: Running a linter on the server side and let it comment on phab reviews would be the perfect project to get some experience with pre-commit testing/hooks for llvm
To be honest I would prefer a server side solution or a bot as the clients
will not converge on one way of submitting patches (for review) anyway.
I also like the bot solution better.
Phabricator do provide API for adding comments to a revision: https://reviews.llvm.org/conduit/method/differential.createcomment/
I’m not familiar with the bot setup though… wondering who’s the right person to contact for this?