[PATCH] D95017: [clang-format] Add case aware include sorting.

PragmaNull added a comment. In D95017#2595230 , @HazardyKnusperkeks wrote: > > Do you make a change? Otherwise I will do, but that will take some time, > because I'm rather busy. Sorry, I have not; unfortunately I've been rather busy myself and

[PATCH] D95017: [clang-format] Add case aware include sorting.

HazardyKnusperkeks added a comment. In D95017#2572578 , @PragmaNull wrote: > In D95017#2572238 , @curdeius wrote: > >> Do you have an idea for better names? >> I see that e.g. MS documentation uses ascending order

[PATCH] D95017: [clang-format] Add case aware include sorting.

HazardyKnusperkeks added a comment. In D95017#2572578 , @PragmaNull wrote: > In D95017#2572238 , @curdeius wrote: > >> Do you have an idea for better names? >> I see that e.g. MS documentation uses ascending order

[PATCH] D95017: [clang-format] Add case aware include sorting.

PragmaNull added a comment. In D95017#2572238 , @curdeius wrote: > Do you have an idea for better names? > I see that e.g. MS documentation uses ascending order and case-sensitive > order. I'm okay with the names, it just seems to me that they are

[PATCH] D95017: [clang-format] Add case aware include sorting.

curdeius added a comment. Do you have an idea for better names? I see that e.g. MS documentation uses ascending order and case-sensitive order. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017

[PATCH] D95017: [clang-format] Add case aware include sorting.

PragmaNull added a comment. I find the naming of the case sensitive options confusing here. When I read "CaseSensitive" I think of the behavior of strcmp() which sorts according to "ASCIIbetical" order. But here "CaseSensitive" throws away case by comparing the result of "Filename.lower()"

[PATCH] D95017: [clang-format] Add case aware include sorting.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa8105b3766e4: [clang-format] Add case aware include sorting. (authored by kentsommer, committed by curdeius). Changed prior to commit:

[PATCH] D95017: [clang-format] Add case aware include sorting.

kentsommer updated this revision to Diff 320007. kentsommer added a comment. NFC rebase to fix CI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D95017: [clang-format] Add case aware include sorting.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I like this much better LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 ___

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 319819. kentsommer added a comment. NFC Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 319806. kentsommer added a comment. Fixed release notes typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 319804. kentsommer added a comment. Added release notes, updated commit message and summary Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files:

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added a comment. I was thinking of https://github.com/llvm/llvm-project/blob/main/clang/docs/ReleaseNotes.rst#clang-format. Look at the history to see how it was written before. Also, you may change the revision title if you want (it will become the commit message). Repository: rG

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer added a comment. In D95017#2527512 , @curdeius wrote: > Concerning the `--sort-includes` command-line flag. I'd rather keep it as is > and, if need be, work on accepting an **optional** string argument. > Please update release notes.

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. kentsommer wrote: > HazardyKnusperkeks wrote:

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added a comment. Concerning the `--sort-includes` command-line flag. I'd rather keep it as is and, if need be, work on accepting an **optional** string argument. Please update release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:399 +IO.enumCase(Value, "false", FormatStyle::SI_Never); +IO.enumCase(Value, "", FormatStyle::SI_CaseInsensitive); +IO.enumCase(Value, "true", FormatStyle::SI_CaseInsensitive);

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:399 +IO.enumCase(Value, "false", FormatStyle::SI_Never); +IO.enumCase(Value, "", FormatStyle::SI_CaseInsensitive); +IO.enumCase(Value, "true", FormatStyle::SI_CaseInsensitive);

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 319786. kentsommer added a comment. Remove improper enum mapping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer marked an inline comment as done. kentsommer added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion.

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 319784. kentsommer added a comment. Turn SortIncludes into an enum Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D95017: [clang-format] add case aware include sorting

HazardyKnusperkeks added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. MyDeveloperDay wrote: > curdeius

[PATCH] D95017: [clang-format] add case aware include sorting

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. curdeius wrote: > kentsommer wrote: > >

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Requesting changes so that it appears correctly in the review queue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. kentsommer wrote: > MyDeveloperDay wrote: > >

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added a comment. Ok, reverted. If there's a doubt, then I guess it's the best solution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 ___ cfe-commits

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer added a comment. In D95017#2522063 , @curdeius wrote: > Should we then revert ASAP and rework it later? @mydeveloperday Reverting would also be fine with me. I am happy to drive towards a feature (and flags) that everyone is comfortable with

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added a comment. Should we then revert ASAP and rework it later? @mydeveloperday Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 ___ cfe-commits mailing

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. MyDeveloperDay wrote: > Are you sure

[PATCH] D95017: [clang-format] add case aware include sorting

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. Are you sure

[PATCH] D95017: [clang-format] add case aware include sorting

This revision was automatically updated to reflect the committed changes. Closed by commit rG3395a336b025: [clang-format] add case aware include sorting (authored by kentsommer, committed by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius accepted this revision. curdeius added a comment. LGTM. Thanks for being prompt at addressing the comments. I'll merge it today for you if @MyDeveloperDay has no more remarks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 31. kentsommer added a comment. Undo some accidental comment formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files:

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2039 +**IncludeSortCaseAware** (``bool``) + Specify if sorting should be done in a case aware fashion. MyDeveloperDay wrote: > curdeius wrote: > > The name is somewhat

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 318886. kentsommer marked 5 inline comments as done. kentsommer added a comment. Addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files:

[PATCH] D95017: [clang-format] add case aware include sorting

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2039 +**IncludeSortCaseAware** (``bool``) + Specify if sorting should be done in a case aware fashion. curdeius wrote: > The name is somewhat awkward IMO (in the sense it

[PATCH] D95017: [clang-format] add case aware include sorting

curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2039 +**IncludeSortCaseAware** (``bool``) + Specify if sorting should be done in a case aware fashion. The name is somewhat awkward IMO (in the sense it doesn't read nicely),

[PATCH] D95017: [clang-format] add case aware include sorting

HazardyKnusperkeks added a comment. I can push that, but will wait a bit longer so the others have time to object. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer added a comment. I do not have commit access. Full Name: Kent Sommer Email: w...@kentsommer.com Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 ___

[PATCH] D95017: [clang-format] add case aware include sorting

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer added a comment. @HazardyKnusperkeks let me know if the additions to the unit test are along the lines you were hoping for or not, I think this captures what you had in mind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 318111. kentsommer added a comment. Expanded unit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D95017: [clang-format] add case aware include sorting

HazardyKnusperkeks added a comment. Please expand the test case to different options like grouping. Otherwise looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer updated this revision to Diff 317790. kentsommer added a comment. Fix unittests Fixes both failing unit tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 Files:

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer added a comment. I'll work on fixing the unit tests. Thought they ran with check-cpang-format but I was obviously wrong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017

[PATCH] D95017: [clang-format] add case aware include sorting

kentsommer created this revision. kentsommer requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Adds an option to [clang-format] which sorts headers in an alphabetical manner using case only for tie-breakers. The options is off by default