[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 6 inline comments as done.
MyDeveloperDay added a comment.

I'll likely abandon this review when D68554: [clang-format] Proposal for clang-format to give compiler style warnings <https://reviews.llvm.org/D68554> lands



================
Comment at: clang/tools/clang-format/ClangFormat.cpp:245
+// Returns an invalid BOM
+static const char *hasInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
----------------
owenpan wrote:
> owenpan wrote:
> > 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`.
> May I suggest `getInvalidBOM(const StringRef)` for the function name and parameter type?
@klimek  already got me on that one over in {D68554} 


================
Comment at: clang/tools/clang-format/ClangFormat.cpp:245-266
+static const char *hasInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  const char *InvalidBOM =
+      llvm::StringSwitch<const char *>(BufStr)
----------------
MyDeveloperDay wrote:
> owenpan wrote:
> > owenpan wrote:
> > > 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`.
> > May I suggest `getInvalidBOM(const StringRef)` for the function name and parameter type?
> @klimek  already got me on that one over in {D68554} 
That sounds good, do you mind if I do that separately? (as this review is a little entwined with 
{D68554}, which is where I'll actually update that revision with your suggestions.


================
Comment at: clang/tools/clang-format/ClangFormat.cpp:381
+// Dump the configuration.
+static unsigned dumpConfig(StringRef AssumeFileName) {
+  StringRef FileName;
----------------
owenpan wrote:
> `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`.
I think it should be `int` as it ends up being the return from main, I'll do that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68767