Page MenuHome
Differential D7323

util: add option to format only changed files to make format
AbandonedPublic
Actions

Authored by Ray Molenkamp (LazyDodo) on Apr 3 2020, 5:07 PM.

Details

Reviewers
Sybren A. Stüvel (sybren)
Julian Eisel (Severin)
Summary

make format is great, but it's not fast, and sometimes when doing patches you pick up stray formats cause someone else committed badly formatted code you ended up merging.

This patch adds a --changed-only option that will only format changed files keeping your patches clean and brings the time make format needs down from about 25 to 1.5 seconds on my system.

Tested on windows, may need some love on linux/mac

Diff Detail

Event Timeline

Ray Molenkamp (LazyDodo) created this revision.Apr 3 2020, 5:07 PM
Julian Eisel (Severin) accepted this revision.EditedApr 3 2020, 5:57 PM

Doing this was my proposal (so far I've been using an alias to do the same thing), so obviously I'm biased in favor of this. It makes formatting even less of a hassle, which is important so people have no reason to avoid dealing with it.
Not sure if --changed-only is the best name, but it seems fine.

utils_maintenance/clang_format_paths.py
68

Whitespace.

153

Sounds a bit messed up? :)

185

Whitespace.

This revision is now accepted and ready to land.Apr 3 2020, 5:57 PM
Brecht Van Lommel (brecht) removed a reviewer: Brecht Van Lommel (brecht).Apr 3 2020, 7:31 PM

Fine with me, will leave it to others to review and test.

Ray Molenkamp (LazyDodo) updated this revision to Diff 23413.Apr 3 2020, 8:00 PM

update with feedback

Ray Molenkamp (LazyDodo) marked 2 inline comments as done.Apr 3 2020, 8:05 PM
Julian Eisel (Severin) added a comment.Apr 6 2020, 5:29 PM

Guess this can be committed then?

utils_maintenance/clang_format_paths.py
68

,*paths is the whitespace culprit ;)

Ray Molenkamp (LazyDodo) added a comment.Apr 6 2020, 5:34 PM

Dunno, have you tested it? i tried to run it on WSL (linux on windows, it's kinda trippy) and it did not seem to have a happy time

Julian Eisel (Severin) added a comment.Apr 6 2020, 5:41 PM

I tried it via the Python file directly and that went just fine (although the "Operating on: ..." output is misleading). You indeed can't just do make format --changed-only but that's a separate thing I guess.

Ray Molenkamp (LazyDodo) abandoned this revision.Jul 3 2020, 10:27 PM
Sybren A. Stüvel (sybren) added a comment.Aug 5 2020, 10:46 AM

@Ray Molenkamp (LazyDodo) why did you abandon this patch? It looks like useful functionality to me.

Sybren A. Stüvel (sybren) added inline comments.Aug 5 2020, 10:56 AM
utils_maintenance/clang_format_paths.py
192

This is getting a bit too complex for my taste now, so I would write this as:

git_paths = (".",) if use_default_paths else paths
files_retab = [
    f for f in source_files_from_git(git_paths, args.changed_only)
    if f.endswith(extensions_only_retab)
    if f not in ignore_files
]
Ray Molenkamp (LazyDodo) added a comment.Aug 5 2020, 2:34 PM

I scaled back the hours i spend on blender a while ago, and abandoned all patches i had open where i didn't have to time/motivation to get a review/test/land them, patch is yours if you want to put the time in.

Sybren A. Stüvel (sybren) added a comment.Aug 31 2020, 12:31 PM

As a reminder for future updates: make format actually changes $PATH so that it runs clang-format from the SVN libs. Directly running this Python file does not, which means that it could potentially find a different version of clang-format and thus produce different output (I've been bitten by this before).

Ankit Meel (ankitm) added a subscriber: Ankit Meel (ankitm).Oct 6 2020, 2:46 PM

D8991: make format: add option to format only edited files.
rBDTb56014f295a2: make format: add option to format only edited files

Revision Contents
Changeset List

PathSize
utils_maintenance/
20 lines

Diff 23413

View Options

utils_maintenance/clang_format_paths.py

Loading...