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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
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:
> 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?


================
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)
----------------
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`.


================
Comment at: clang/tools/clang-format/ClangFormat.cpp:381
+// Dump the configuration.
+static unsigned dumpConfig(StringRef AssumeFileName) {
+  StringRef FileName;
----------------
`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`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68767