[PATCH] D67750: Allow additional file suffixes/extensions considered as source in main include grouping

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

I think fundamentally this seems like a reasonable idea, for those that don't need they don't need to use it.



================
Comment at: clang/docs/ClangFormat.rst:36
-                                -style=file) and to determine the language.
+    -assume-filename=<string> - Override filename used to determine the language.
+                                When reading from stdin, clang-format assumes this
+                                filename to determine the language.
     -cursor=<uint>            - The position of the cursor when invoking
                                 clang-format from an editor integration
----------------
I agree this seems wrong, it's making it sound like -assume-filename is a file that will replace .clang-format .. I don't think thats true as you correctly point out


================
Comment at: clang/lib/Format/Format.cpp:456
+    IO.mapOptional("IncludeIsMainSourceRegex",
+                   Style.IncludeStyle.IncludeIsMainSourceRegex);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
----------------
I think we are missing a change to the operator== in Format.h (I notice IncludeIsMainRegex is missing from there too)


================
Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:183
     CategoryRegexs.emplace_back(Category.Regex, llvm::Regex::IgnoreCase);
+  llvm::Regex MainFileRegex(Style.IncludeIsMainSourceRegex);
   IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
----------------
whats the default value of `IncludeIsMainSourceRegex?`, are we making a regex even when its empty?

could we not or the regex after if its defined

```
IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
               FileName.endswith(".cpp") || FileName.endswith(".c++") ||
               FileName.endswith(".cxx") || FileName.endswith(".m") ||
               FileName.endswith(".mm");
if (!Style.IncludeIsMainSourceRegex.empty()){
    llvm::Regex MainFileRegex(Style.IncludeIsMainSourceRegex);
    IsMainFile |= MainFileRegex.match(FileName);    
}
```


================
Comment at: clang/tools/clang-format/ClangFormat.cpp:79
+             "When reading from stdin, clang-format assumes this\n"
+             "filename to determine the language."),
+    cl::init("<stdin>"), cl::cat(ClangFormatCategory));
----------------
This may be a little out of date and needs rebasing as I believe this reformat has already been done, but your new wording is correct


Repository:
  rC Clang

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

https://reviews.llvm.org/D67750