[PATCH] D29039: [clang-format] Proposal for clang-format -r option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D29039#1704166 , @zturner wrote:

> In Windows you just write a Python script to do this, and this works 
> everywhere, not just on one platform or the other, so bash isn't even 
> necessary and Python is easy to write so I wouldn't really say it's "even 
> harder".  If you google for `run-clang-format.py` you'll find a script that 
> actually branches out and does this in parallel.  There's a lot of logic and 
> smarts you could bake into an external tool which can then be used for many 
> different programs, not just clang-format.


@zturner thanks for the feedback, I took a look and run-clang-format.py at 
(https://github.com/Sarcasm/run-clang-format), and whilst I have a reasonable 
knowledge of python, reading the python code just makes me ask why isn't all of 
this just in clang-format in the first place?

To me I think these are unaddressed clang-format requirements that other 
third-party developers are providing, without having to conform to some agreed 
upstream set of rules or gaining consensus. The author has coded it the way 
they want, how they want and can add to it to do what they want (good on them). 
But that doesn't mean that's the only approach or best choice of technology, or 
that someone should follow the same model or develop something similar. Its one 
option, a good option, but not the only option.

I've personally been burnt on Windows using a combination of python in both the 
python windows distribution and cygwin together, the windows version doesn't 
understand Cygwin paths (as expected). I know I do my own development in clang 
with 1 Visual Studio Command Prompt setup to pick up VS2019 and one bash prompt 
to ensure it picks up python correctly,  (I don't use CMake to make visual 
studio projects but use the Code Blocks CMake options as that works nicely with 
cygwin, allowing me to do a make -jN which is essential, the visual studio 
projects are pretty unworkable)

I have to build in the windows shell so it finds the correct compiler cl.exe 
and not the cygwin's gcc, but I can't run `git clang-format` or `git llvm push` 
in those windows shells as the python breaks. and that all comes down to the 
interaction of different pythons. (and I just cannot justify to myself 
proliferating that pain to others by using python as a solution when we have a 
perfectly good solution in C++) . Ask yourself how many people have trouble 
getting the lit tests to work in LLVM on windows I know I do. Which I'm sure is 
resolved by some configuration magic but I haven't found it yet.

But I am grateful for the evidence that needing this capability is obviously 
required (otherwise someone wouldn't have written this script)

if we take a look at the script what is it doing... the script is ~ 350 lines

1. A certain section is covering recursive running (about 20-40 lines)
2. A reasonable amount  is simply handling the command line arguments (30-50 
lines)
3. About 40 lines is colourizing a diff
4. And about 70-100 lines is handling running in parallel

The command-line options it supports just:

--clang-format-executable (to tell it where to find the executable)
-extensions to tell it what to file types to handle (which actually their list 
is out of date as it only covers some C++ code and not js,C#,protobuf etc)
-r for recursive
-q for quiet 
-j for the number of parallel jobs
-color for showing a colored diff
--exclude for paths to exclude.

Of that which parts are useful? the colored diff? maybe.. the parallelizm is 
nice but that's not essential in my view on a fast machine and there are other 
ways of doing that especially in makefiles which are already running -j N maybe 
you could even use gnu parallel for that?

So basically we are down to --extensions, -r, --exclude ... as perhaps the real 
benefit, I think its this functionality that this revision is/should be adding. 
I think its relatively simple and is already partially covered in about 30+ 
lines above.

But the existence of this script just means I'm even more sold on the fact that 
we should do it..so I really appreciate you bringing that to my attention.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D29039



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

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In Windows you just write a Python script to do this, and this works 
everywhere, not just on one platform or the other, so bash isn't even necessary 
and Python is easy to write so I wouldn't really say it's "even harder".  If 
you google for `run-clang-format.py` you'll find a script that actually 
branches out and does this in parallel.  There's a lot of logic and smarts you 
could bake into an external tool which can then be used for many different 
programs, not just clang-format.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D29039



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

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.






Comment at: tools/clang-format/ClangFormat.cpp:345
+
+  Regex regex(FileNames[i]);
+

I could imagine the path you supplying being just a directory . or  
../../include/Format as much as a wild card, once in that directory I might 
want to you clang-format all the file types clang-format supports or just a 
few? how could we do that?

it would be nice if we could supply multiple directories on the command line

```clang-format -r -i  . .. ../include/Format```

I wonder if the wild card should act more like --gtest_filter   

```--gtest_filter=POSTIVE_PATTERNS[-NEGATIVE_PATTERNS]``` 

or find's -iregex?




Repository:
  rL LLVM

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

https://reviews.llvm.org/D29039



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

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a subscriber: djasper.
mitchell-stellar added a comment.

I agree with @djasper that this is outside the scope of clang-format. git for 
Windows gives you a full set of bash utilities to utilize, so doing stuff like 
this on Windows is much easier now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D29039



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