[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan marked an inline comment as done.
Manikishan added a comment.

In D64695#1584706 <https://reviews.llvm.org/D64695#1584706>, @MyDeveloperDay wrote:

> There also seems like alot of duplication between the existing sortCppIncludes
>
> I think the only difference here is really just
>
>     
>
>   SmallVector<unsigned, 16> Indices;
>     SmallVector<unsigned, 16> Includes_p;
>     for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
>       unsigned pl = getNetBSDIncludePriority(Includes[i].Filename);
>       Includes_p.push_back(pl);
>       Indices.push_back(i);
>     }
>
>
> vs the original
>
>   SmallVector<unsigned, 16> Indices;
>   for (unsigned i = 0, e = Includes.size(); i != e; ++i)
>     Indices.push_back(i);
>   
>
> plus way the sorting is performed, are we sure we couldn't have just made the original sorting more powerful based on style settings?


Does it mean that adding the priority to sort based on style? 
like this:

  if (Style== NetBSD)
     // set priority to netbsd's priority

I didn't want to mess up the original sorting and made up this patch, if we have parameterise this solution, I will go for it.



================
Comment at: lib/Format/Format.cpp:1878
+  SmallVector<unsigned, 16> Indices;
+  SmallVector<unsigned, 16> Includes_p;
+  for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
----------------
MyDeveloperDay wrote:
> _p? I don't understand what it stands for?
> 
> IncludesPriority?
Yes , it means priority


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695