klimek added a comment.

In D68554#1702090 <https://reviews.llvm.org/D68554#1702090>, @MyDeveloperDay 
wrote:

> As this behaviour is not something clang-format should do because there are 
> external tool that can do this for us, I thought I'd set myself a challenge 
> of trying to write this with external tools, to prove to myself that it was 
> indeed easy, what I want to achieve is being able to go to a directory and 
> determine if there are any clang-format changes needed in the source files of 
> the levels below with a single command.
>
> The idea would be that this could be something that could fit on the command 
> line, or made into a bash alias/function (more likely), I really don't want 
> to turn to python as if I'm coding this in another language I might as well 
> be coding it in C++ inside clang-format and it opens me up to the whole can't 
> find the correct python headache. (but I also think the clang diagnostic 
> classes give doing it inside C++ the edge)
>
> I want to take into account at least the concepts from D29039: [clang-format] 
> Proposal for clang-format -r option <https://reviews.llvm.org/D29039> about 
> being able to look deeply into a directory.
>
> There are a couple of requirements: (some nice to have's)
>
> 1. look deeply into a directory for all files supported by clang-format 
> (C++,C# etc)
> 2. file extensions should be case insensitive i.e. .cpp or .CPP
> 3. be able to clearly see the name of  the file that need clang-formatting
> 4. don't actually do the formatting (tell me what needs doing but don't 
> necessarily make the change)
> 5. ideally show a snippet of the file where the change is needed (I guess 
> this isn't essential)
> 6. This should be cross platform windows, linux, (I guess other *nix's) it 
> should work in at least the common shells bash,cygwin,powershell
>
>   The cross platform is the real killer, this forces you down the python 
> road, (or the C++) if you want a common solution otherwise you are 
> implementing different solutions on different plaforms.
>
>   But lets ignore that and lets start with a Linux or Cygwin bash shell. 
> Surely the best way to find all source files that can be converted is via a 
> find. In the past I've only ever used find like this to find multiple 
> extensions
>
>   ```find .\( -name "*.h" -o name "*.cpp" \) -print ```
>
>   But this is really unworkable for so many output formats that clang-format 
> supports, and even if you could get that to work it would be case sensitive 
> unless you uses -ilname which would make the command line even longer
>
>   So lets try using -iregex, this is a much shorter case insensitive command 
> but still, it needs a certain amount of escaping in the regular expression, 
> (lets hope we never do "clang-format all the things") as this could get 
> longer.
>
>   ```find . -iregex 
> '.*\.\(cpp\|cc\|c\+\+\|cxx\|c\|cl\|h\|hh\|hpp\|m\|mm\|inc\|js\|ts\|protor\|protodevel\|java\|cs\)'
>  -print```
>
>   if I want to put that into an alias then I need switch the ' and " around
>
>   ```alias find_all_code='find . -iregex 
> ".*\.\(cpp\|cc\|c\+\+\|cxx\|c\|cl\|h\|hh\|hpp\|m\|mm\|inc\|js\|ts\|protor\|protodevel\|java\|cs\)"
>  -print'```
>
>   Is the same thing as easy in powershell? Ok not really .. this is as far as 
> I got, and I couldn't work out how to handle multiple extensions let alone 
> case sensitivity.. (it feels very slow too!)
>
>   ```PowerShell.exe -Command "dir -Recurse -ea 0 | ? FullName -like '*.cpp'```
>
>   So at least my brief initial conclusion is... if we are not using C++, its 
> Python or nothing if you want cross-platform or maybe find if you only care 
> about *nix
>
>   But even the find... that only covers 1) and 2) of my requirements...how do 
> I determine even with find which files need formatting..
>
>   This command would show me the contents of everything that needs changing.
>
>   ```find . -iregex 
> '.*\.\(cpp\|cc\|c\+\+\|cxx\|c\|cl\|h\|hh\|hpp\|m\|mm\|inc\|js\|ts\|protor\|protodevel\|java\|cs\)'
>  -exec clang-format {} \;```
>
>   This would give me the list of changes, but then I lose the name of the 
> file.
>
>   ```find . -iregex 
> '.*\.\(cpp\|cc\|c\+\+\|cxx\|c\|cl\|h\|hh\|hpp\|m\|mm\|inc\|js\|ts\|protor\|protodevel\|java\|cs\)'
>  -exec clang-format --output-replacements-xml {} \;```
>
>   So using external tools is easy how? the dumping of the output to stdout or 
> the replacement xml to stdout makes this much harder as I lose the filename 
> that was passed to clang-format, (there is probably an xargs trick here I'm 
> missing)
>
>   This proposed solution would make all this possible just with:
>
>   ```find . -iregex 
> '.*\.\(cpp\|cc\|c\+\+\|cxx\|c\|cl\|h\|hh\|hpp\|m\|mm\|inc\|js\|ts\|protor\|protodevel\|java\|cs\)'
>  -exec clang-format -i -n {} \;```
>
>   and more to the point the list of file types supported by clang-format is 
> something clang-format knows... why not combine this with D29039: 
> [clang-format] Proposal for clang-format -r option 
> <https://reviews.llvm.org/D29039> and just do
>
>   clang-format -r -i -n .
>
>   This provides in my view the only cross platform, shell independent way of 
> doing this in what is a relatively minimal code change, and I'm struggling to 
> see this as easy to do with an external tools? but if someone wants to show 
> me how, I'd be interested to learn.


I think it's easy enough to do as a kind of driver, either in python or C++, 
but then again, at that point putting it into ClangFormat seems fine. One thing 
that I find on the one side kinda nice, but on the other side also weird is the 
names of the flags. Making them more similar to clang flags is nice from the 
point of people used to clang being able to quickly identify them, but the 
structure also makes it look like clang-format would support a wider range of 
flags, which it doesn't. Not sure which side I'm on, and I'm fine with the 
patch as is (minus pulling out a function), but wanted to bring it up if folks 
have ideas.



================
Comment at: clang/tools/clang-format/ClangFormat.cpp:371
+      if (!Replaces.empty()) {
+        IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
+            new DiagnosticOptions();
----------------
I'd pull this block out into a function.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68554/new/

https://reviews.llvm.org/D68554



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to