Page MenuHomePhabricator

Proposal for clang-format -r option
Needs ReviewPublic

Authored by stryku on Jan 23 2017, 9:50 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Hello Llvm Community,

I would like to present you a small clang-format proposal.

I want to propose a '-r' option, with which, clang-format will search for files, which will match regex[s] (passed as a command line arguments, like normal files), down in the directories tree.

What motivates such changes is more user-friendly clang-format. Now when user want to do format all files in the current location and subdirectories,, he need to write some scripts/use bash magic or rely on that, that shell itself will expand regex into file names. On Windows, it's even harder. Sample clang-format call, for c++ header and source files, would look e.g. like this:
clang-format -i -r .\.cpp$ .\.hpp$

This patch provides strict regex matching, so such call won't work:
'clang-format -i -r *.cpp'

Next thing to implement could be some kind of 'easy regexs' or so, which would accept '*.cpp', but IMHO this isn't in the scope of this patch, so I didn't implement it.

I didn't write any test or so. Before, I wanted to write this mail and get feedback if -r option is even a thing and you will like it. After your acceptance (if it'll be an acceptance), I'll write tests, update documentation etc.

If you have questions/comments/accepts/rejects feel free (:

P.S. I don't know why there is so many changes in lines that I didn't even touched. I've just ran clang-format -i -style=file to format this file.

Diff Detail

Repository
rL LLVM

Event Timeline

stryku created this revision.Jan 23 2017, 9:50 AM
rsmith edited reviewers, added: djasper; removed: rsmith.Jan 23 2017, 11:17 AM
rsmith added a subscriber: cfe-commits.
djasper edited edge metadata.Jan 23 2017, 3:16 PM

I am happy to let other people in the community weigh in, but I would not move forward with this patch. Listing directories is not a task that clang-format should do. It does not seem useful to me to add this functionality to basically every single tool that you might want to pipe files through. That would mean that you'd have to repeat the same code many times. Also, I would not call this "bash magic", these are more like "bash basics". I don't know how much harder this is on windows, but still solving that doesn't sound like something clang-format should fix.

The other point I'd like to make is that it rarely makes sense to format an entire directory structure with clang-format. It is actually much more useful if integrated into editors (plugins available) or the version control system (e.g. through git-clang-format). That's also why you see many diffs here, we don't reformat all of clang-format on every commit, just the changed lines. The rest might go sightly out of sync with clang-format's formatting, but that's ok, no harm done. Often this is preferable over lots of spurious diffs as present here.

@djasper You're probably right. Thanks anyway for your time (:

No problem :)

djasper resigned from this revision.Jan 31 2017, 3:57 AM