Page MenuHomePhabricator

Manikishan (Manikishan Ghantasala)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 9 2019, 10:44 AM (6 w, 8 h)

Recent Activity

Thu, Jul 18

Manikishan added inline comments to D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Thu, Jul 18, 9:20 AM · Restricted Project
Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

Is there sufficient test coverage as to what happens if SortPriority is not set?

If SortPriority is not set, the Includes will be grouped without sorting,

Let me rephrase - for the exiting .clang-formats, that don't currently specify SortPriority,
this introduction of SortPriority should not change the header handling.
Is that the case, and if so is there sufficient test coverage for that?

I got your idea now.
No, there is no test coverage for that case, and with the current patch they have to add SortPriority.
To avoid this shall I set SortPriority as Priority as default if it is not defined? I think that will fix the issue.

Thu, Jul 18, 1:40 AM · Restricted Project

Wed, Jul 17

Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Wed, Jul 17, 10:09 AM · Restricted Project
Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

Is there sufficient test coverage as to what happens if SortPriority is not set?

Wed, Jul 17, 10:04 AM · Restricted Project
Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Wed, Jul 17, 9:49 AM · Restricted Project
Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

Thanks! Can you update the documentation too?

Wed, Jul 17, 7:55 AM · Restricted Project

Tue, Jul 16

Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

In the following patch I have modified Include categories by adding a new field "sortInlcudes" which defines the priority of the sort. And priority field will now only be used for grouping.

Tue, Jul 16, 8:24 PM · Restricted Project
Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Tue, Jul 16, 4:37 PM · Restricted Project

Mon, Jul 15

Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

I appreciate what you've done to make this patch less duplicated code..but now you've almost got to the point where being able to control it all via configuration.

I'm with @rdwampler I think you should be able to extend the include categories...and then this becomes a very powerful capability. There are many people who use clang-format for a "non" standard style source trees, and they might like to use such a capability themselves without having to inherit from NetBSD type.

I agree it will be complex, but that is a good justification for adding hard coded a NetBSD style.

Mon, Jul 15, 12:43 PM · Restricted Project
Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Mon, Jul 15, 12:26 PM · Restricted Project
Manikishan added inline comments to D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Mon, Jul 15, 12:23 PM · Restricted Project
Manikishan added inline comments to D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Mon, Jul 15, 12:23 PM · Restricted Project
Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

I am not quite sure why this change is required to sort the headers for NetBSD, you can set the priorities via IncludeStyle.IncludeCategories. Is that not sufficient?

It can be done by setting priorities in IncludeCategories, but here we have nearly 40+ cases and categories to hardcode due to complex interdependencies between their headers. So, I have added this style reducing the cases using regex. And if this is fully parameterised any OS related project can add their own header priorities.

Note that IncludeCategories is already a regex - https://clang.llvm.org/docs/ClangFormatStyleOptions.html

Mon, Jul 15, 9:31 AM · Restricted Project
Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

I am not quite sure why this change is required to sort the headers for NetBSD, you can set the priorities via IncludeStyle.IncludeCategories. Is that not sufficient?

Mon, Jul 15, 8:57 AM · Restricted Project
Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

Is this change that was meant?

Mon, Jul 15, 8:45 AM · Restricted Project
Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Mon, Jul 15, 5:48 AM · Restricted Project
Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

I guess my comment was that you patche introduces a lot of code duplication, could the small amount of code you added be say put into a lambda and past in based on style?

Mon, Jul 15, 2:48 AM · Restricted Project
Manikishan added inline comments to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Mon, Jul 15, 1:51 AM · Restricted Project
Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Mon, Jul 15, 1:51 AM · Restricted Project

Sun, Jul 14

Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

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?

Sun, Jul 14, 10:07 AM · Restricted Project
Manikishan added a comment to D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

Do you think there is anything about this algorithm that could be parameterized so that other projects could utilize it?

I think we can take input from users the header files which should be grouped together and pass them to regex searches which then sets up priority.
But I dob the feasibility of this approach. Will come up with one.

I guess its not completely clear how this differs from just using the IncludeCategories?

Sun, Jul 14, 4:33 AM · Restricted Project

Sat, Jul 13

Manikishan updated the summary of D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Sat, Jul 13, 5:38 AM · Restricted Project
Manikishan updated the diff for D64695: [clang-format] Added new style rule: SortNetBSDIncludes.

This is the squashed commit of both Adding NetBSD Style and adding sortNetBSDIncludes

Sat, Jul 13, 5:36 AM · Restricted Project
Manikishan updated subscribers of D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Sat, Jul 13, 5:32 AM · Restricted Project
Manikishan created D64695: [clang-format] Added new style rule: SortNetBSDIncludes.
Sat, Jul 13, 5:32 AM · Restricted Project

Wed, Jun 26

Manikishan updated the summary of D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Wed, Jun 26, 5:44 AM · Restricted Project

Jun 12 2019

Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 12 2019, 12:26 PM · Restricted Project

Jun 11 2019

Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 11 2019, 7:36 AM · Restricted Project
Manikishan retitled D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine from [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines to [clang-format] Added New Style Rule: OnePerLineBitFieldDecl.
Jun 11 2019, 6:19 AM · Restricted Project
Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 11 2019, 5:30 AM · Restricted Project
Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.

Changed Style name to OnePerLineBitFieldDecl

Jun 11 2019, 5:24 AM · Restricted Project

Jun 10 2019

Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.

Updated unittest

Jun 10 2019, 10:47 AM · Restricted Project
Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.

Made some missing style modifications in the last revision

Jun 10 2019, 9:17 AM · Restricted Project
Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.

Added unittests and made the changes suggested by @mgorny

Jun 10 2019, 9:03 AM · Restricted Project
Manikishan updated the diff for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 10 2019, 8:35 AM · Restricted Project

Jun 9 2019

Manikishan updated the summary of D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 9 2019, 12:38 PM · Restricted Project
Manikishan updated the summary of D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 9 2019, 12:38 PM · Restricted Project
Manikishan updated the summary of D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 9 2019, 12:35 PM · Restricted Project
Manikishan updated the summary of D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 9 2019, 12:35 PM · Restricted Project
Manikishan updated the summary of D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 9 2019, 12:35 PM · Restricted Project
Manikishan updated the summary of D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 9 2019, 12:35 PM · Restricted Project
Manikishan added a reviewer for D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine: djasper.
Jun 9 2019, 12:33 PM · Restricted Project
Manikishan created D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine.
Jun 9 2019, 12:21 PM · Restricted Project