This is an archive of the discontinued LLVM Phabricator instance.

[NFC][Py Reformat] Reformat python files in libcxx/libcxxabi
ClosedPublic

Authored by thieta on May 17 2023, 2:10 AM.

Details

Reviewers
jhenderson
MatzeB
JDevlieghere
ldionne
kwk
Mordante
Group Reviewers
Restricted Project
Restricted Project
Restricted Project
Commits
rG7bfaa0f09d05: [NFC][Py Reformat] Reformat python files in libcxx/libcxxabi
Summary

This is an ongoing series of commits that are reformatting our
Python code.

Reformatting is done with black.

If you end up having problems merging this commit because you
have made changes to a python file, the best way to handle that
is to run git checkout --ours <yourfile> and then reformat it
with black.

If you run into any problems, post to discourse about it and
we will try to help.

RFC Thread below:

https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style

Diff Detail

Event Timeline

thieta created this revision.May 17 2023, 2:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2023, 2:10 AM
thieta requested review of this revision.May 17 2023, 2:10 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 17 2023, 2:10 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
kwk accepted this revision.May 17 2023, 2:45 AM
kwk added a subscriber: kwk.

LGTM

thieta updated this revision to Diff 523676.May 19 2023, 12:33 AM

Included all .py files as well.

Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2023, 12:33 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

my initial revision didn't contain all the files. but it should be corrected now. can you have a look @ldionne @Mordante @philnik ?

Another question is it intended to validate the formatting? For libc++ we validate the formatting of our C++ code in the pre-commit CI. I really think it would be great to do that for these files too. If wanted I can do that too.

If you invoked black with command-line arguments could you add them to the commit message?

libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
7

This looks odd. Is black set to a line length of 80 or 88 characters? IMO this header part should be the same number as characters as black. I assume it now is 81 characters.

libcxx/utils/generate_feature_test_macro_components.py
9–30

I really would appreciate it when this file can remain unformatted for now. This will cause a lot of conflicts with D150795. The other Python files in that commit are smaller. I would be happy to run the formatter afterwards myself.

thieta added inline comments.May 22 2023, 11:38 PM
libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
7

Black uses 88. I have another of these headers in clang to fix as well. So I'll see if I can script it after this has landed.

libcxx/utils/generate_feature_test_macro_components.py
9–30

I just fixed a bunch of conflicts and it was really easy to just do git checkout <mybranch> <file>; black <file> - so I wouldn't be to worried about conflicts - but if you feel strongly I can exclude it.

Mordante accepted this revision.May 24 2023, 10:29 AM

LGTM!

libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
7

Sounds good to me.

libcxx/utils/generate_feature_test_macro_components.py
9–30

Note D150795 has landed.

This revision was not accepted when it landed; it landed in state Needs Review.May 25 2023, 2:15 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
ldionne added inline comments.Jun 13 2023, 10:41 AM
libcxx/utils/libcxx/test/params.py
204–235

This was laid out manually in a very intentional manner, and the code is really hard to understand after the reformatting.