Page MenuHomePhabricator

[clang-format] NFC - Move functionality into functions to help code structure
AbandonedPublic

Authored by MyDeveloperDay on Oct 10 2019, 12:58 AM.

Details

Summary

I don't think its the main()'s responsibility to be working out how to dump the config, or the format()'s responsibility to determine what is and isn't a valid BOM

move those functions out to their own functions, this gives clear roles and responsibilities and it makes it easier to add new additional capabilities.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2019, 12:58 AM
owenpan added inline comments.Oct 11 2019, 1:16 AM
clang/tools/clang-format/ClangFormat.cpp
245

May I suggest getInvalidBOM(const StringRef) for the function name and parameter type?

245–266

This code was copied from clang/lib/Basic/SourceManager.cpp. I suggest that you move this function to there and export it through clang/include/clang/Basic/SourceManager.h.

381

AssumeFileName is a file-scope global, so the file-scope function doesn't need to make it a parameter? Also, a bool or int may be a better return type than unsigned.

MyDeveloperDay marked 6 inline comments as done.Oct 12 2019, 7:54 AM
clang/tools/clang-format/ClangFormat.cpp
245
245–266

That sounds good, do you mind if I do that separately? (as this review is a little entwined with
D68554: [clang-format] Proposal for clang-format to give compiler style warnings, which is where I'll actually update that revision with your suggestions.

381

I think it should be int as it ends up being the return from main, I'll do that.

MyDeveloperDay abandoned this revision.Oct 12 2019, 11:56 AM
MyDeveloperDay marked 3 inline comments as done.

Review comments regarding movign getInvalidBOM are addressed in D68914: [clang-format] Remove duplciate code from Invalid BOM detection