seranu wrote:
I created a new PR for separating the includes at
https://github.com/llvm/llvm-project/pull/78957 and will continue to work on
getting the top-level comment code fixed as per review comments. Closing this
PR.
https://github.com/llvm/llvm-project/pull/77918
__
https://github.com/seranu closed https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
HazardyKnusperkeks wrote:
> > I think splitting the two features would be better.
>
> I'm going to close this MR and create two new ones.
>
> > Also please add a note to the changelog.
>
> Where can I find the changelog?
`clang/docs/ReleaseNotes.rst`
https://github.com/llvm/llvm-project/pull
seranu wrote:
> I think splitting the two features would be better.
I'm going to close this MR and create two new ones.
> Also please add a note to the changelog.
Where can I find the changelog?
https://github.com/llvm/llvm-project/pull/77918
___
@@ -2459,6 +2459,52 @@ struct FormatStyle {
/// \version 12
EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
+ /// \brief Number of lines after includes.
+ /// If set, determines the number of lines to insert after includes.
+ /// Limited by MaxEmptyLin
@@ -26846,6 +26846,26 @@ TEST_F(FormatTest, BreakAdjacentStringLiterals) {
Style.BreakAdjacentStringLiterals = false;
verifyFormat(Code, Style);
}
+
+TEST_F(FormatTest, EmptyLinesAfterInclude) {
+ auto Style = getLLVMStyle();
+ Style.EmptyLinesAfterIncludes = 2;
+ Style.
@@ -4827,6 +4873,8 @@ struct FormatStyle {
DerivePointerAlignment == R.DerivePointerAlignment &&
DisableFormat == R.DisableFormat &&
EmptyLineAfterAccessModifier == R.EmptyLineAfterAccessModifier &&
+ EmptyLinesAfterIncludes == R.Empty
@@ -0,0 +1,70 @@
+//===--- TopLevelCommentSeparator.cpp ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,112 @@
+//===- unittest/Format/TopLevelCommentSeparatorTest.cpp - Formatting unit tests
+//-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Id
@@ -26846,6 +26846,26 @@ TEST_F(FormatTest, BreakAdjacentStringLiterals) {
Style.BreakAdjacentStringLiterals = false;
verifyFormat(Code, Style);
}
+
+TEST_F(FormatTest, EmptyLinesAfterInclude) {
+ auto Style = getLLVMStyle();
+ Style.EmptyLinesAfterIncludes = 2;
+ Style.
@@ -2459,6 +2459,52 @@ struct FormatStyle {
/// \version 12
EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
+ /// \brief Number of lines after includes.
+ /// If set, determines the number of lines to insert after includes.
+ /// Limited by MaxEmptyLin
@@ -1035,6 +1039,8 @@ template <> struct MappingTraits {
IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
+IO.mapOption
@@ -0,0 +1,70 @@
+//===--- TopLevelCommentSeparator.cpp ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -2459,6 +2459,52 @@ struct FormatStyle {
/// \version 12
EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
+ /// \brief Number of lines after includes.
+ /// If set, determines the number of lines to insert after includes.
+ /// Limited by MaxEmptyLin
@@ -2459,6 +2459,52 @@ struct FormatStyle {
/// \version 12
EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
+ /// \brief Number of lines after includes.
+ /// If set, determines the number of lines to insert after includes.
+ /// Limited by MaxEmptyLin
https://github.com/HazardyKnusperkeks edited
https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/HazardyKnusperkeks requested changes to this pull request.
I think splitting the two features would be better.
Also please add a note to the changelog.
https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe
https://github.com/seranu updated
https://github.com/llvm/llvm-project/pull/77918
>From af4bd7c1a75b1bb17cf4facde2f3caf86f01511e Mon Sep 17 00:00:00 2001
From: Serban Ungureanu
Date: Tue, 16 Jan 2024 22:19:52 +0200
Subject: [PATCH] [clang-format] Add formatting options
EmptyLinesAfterTopLevelC
https://github.com/seranu updated
https://github.com/llvm/llvm-project/pull/77918
>From d98120c41139cc674088258599c2c3cc9f2921dd Mon Sep 17 00:00:00 2001
From: Serban Ungureanu
Date: Tue, 16 Jan 2024 22:19:52 +0200
Subject: [PATCH] [clang-format] Add formatting options
EmptyLinesAfterTopLevelC
https://github.com/seranu updated
https://github.com/llvm/llvm-project/pull/77918
>From ae7d3b51663d60dd5365bbcb47001f964ba54456 Mon Sep 17 00:00:00 2001
From: Serban Ungureanu
Date: Tue, 16 Jan 2024 22:19:52 +0200
Subject: [PATCH] [clang-format] Add formatting options
EmptyLinesAfterTopLevelC
seranu wrote:
I updated the implementation and tried to include all the feedback I received.
Regarding "NewLines between IncludeGroups" I think this is a separate option
that, in my opinion, would be best implemented together with
"IncludeBlocksStyle::IBS_Regroup" option.
@mydeveloperday I'm
https://github.com/seranu updated
https://github.com/llvm/llvm-project/pull/77918
>From 8cc2aca8ab3d7b5d244dc7f1dfd42242ee888b15 Mon Sep 17 00:00:00 2001
From: Serban Ungureanu
Date: Tue, 16 Jan 2024 22:19:52 +0200
Subject: [PATCH] [clang-format] Add formatting options
EmptyLinesAfterTopLevelC
seranu wrote:
I am going to rework the change as per the review comments. I created another
PR to add the missing SeparateDefinitionBlocks config parse tests at
https://github.com/llvm/llvm-project/pull/78256.
For the rework, shall I create a new PR or can I still use this one?
https://github
https://github.com/seranu edited https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -3853,46 +3853,51 @@ struct FormatStyle {
/// Leave definition blocks as they are.
SDS_Leave,
/// Insert an empty line between definition blocks.
-SDS_Always,
+SDS_One,
+/// Insert two empty lines between definition blocks.
+SDS_Two,
/// Remo
@@ -171,6 +187,31 @@ void DefinitionBlockSeparator::separateBlocks(
return false;
};
+// Separate License text.
seranu wrote:
Yes, that was my working assumption
https://github.com/llvm/llvm-project/pull/77918
__
https://github.com/mydeveloperday edited
https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -610,6 +626,113 @@ TEST_F(DefinitionBlockSeparatorTest, JavaScript) {
"}",
Style);
}
+
+TEST_P(LicenseTest, SeparateLicenseFromBlock) {
+ constexpr StringRef LicenseSingleLineCommentStyle = {"// start license\n"
+
@@ -610,6 +626,113 @@ TEST_F(DefinitionBlockSeparatorTest, JavaScript) {
"}",
Style);
}
+
+TEST_P(LicenseTest, SeparateLicenseFromBlock) {
+ constexpr StringRef LicenseSingleLineCommentStyle = {"// start license\n"
+
https://github.com/mydeveloperday requested changes to this pull request.
Just keep the tests simple.
verifyFormat(this);
verifyFormat(that);
done!
https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
@@ -3853,46 +3853,51 @@ struct FormatStyle {
/// Leave definition blocks as they are.
SDS_Leave,
/// Insert an empty line between definition blocks.
-SDS_Always,
+SDS_One,
mydeveloperday wrote:
1) You cannot change Format.h without regenera
@@ -3853,46 +3853,51 @@ struct FormatStyle {
/// Leave definition blocks as they are.
SDS_Leave,
/// Insert an empty line between definition blocks.
-SDS_Always,
+SDS_One,
+/// Insert two empty lines between definition blocks.
+SDS_Two,
/// Remo
@@ -171,6 +187,31 @@ void DefinitionBlockSeparator::separateBlocks(
return false;
};
+// Separate License text.
mydeveloperday wrote:
Are we assuming any comment at the top of the file is a license comment?
https://github.com/llvm/llvm-project
https://github.com/mydeveloperday edited
https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mydeveloperday requested changes to this pull request.
Yeah I'm kind of a not aligned on this approach of hijacking what feels like
the wrong setting, it feels complicated after the change which probably means
we are not thinking about it enough about what people might reques
seranu wrote:
> You'd have to create multiple pull requests. And if they build on each other
> there is no nice way of doing this on github. You can of course use this one
> for a change. You can also not split the work, but chances are it will take
> longer for the changes to land.
I created
@@ -17,80 +17,97 @@
namespace clang {
namespace format {
namespace {
+std::string
+separateDefinitionBlocks(llvm::StringRef Code,
+ const std::vector &Ranges,
+ const FormatStyle &Style = getLLVMStyle()) {
+ LLVM_DEBUG(llvm::errs
@@ -586,7 +586,8 @@ template <>
struct ScalarEnumerationTraits {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value)
{
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
-IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+IO.enumCa
@@ -586,7 +586,8 @@ template <>
struct ScalarEnumerationTraits {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value)
{
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
-IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+IO.enumCa
@@ -17,80 +17,97 @@
namespace clang {
namespace format {
namespace {
+std::string
+separateDefinitionBlocks(llvm::StringRef Code,
+ const std::vector &Ranges,
+ const FormatStyle &Style = getLLVMStyle()) {
+ LLVM_DEBUG(llvm::errs
HazardyKnusperkeks wrote:
> > Maybe first only handle #42112 for what I will be really grateful.
>
> #42112 requests separating both license text and include directives blocks.
>
My bad, I only looked at the title, not the text.
> > And then the stuff with the _license_, because I see discuss
https://github.com/seranu updated
https://github.com/llvm/llvm-project/pull/77918
>From 60a5851b40f03fb71b2a3d30972d51ba40244d68 Mon Sep 17 00:00:00 2001
From: Serban Ungureanu
Date: Fri, 12 Jan 2024 14:33:32 +0200
Subject: [PATCH 1/3] [clang-format] Extend DefinitionBlockSeparatorStyle to
sep
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 2798b72ae7e5caad793169b77cbac47fe2362d0f
291c05994202393a858de1aafa8eeaf958223964 --
https://github.com/seranu updated
https://github.com/llvm/llvm-project/pull/77918
>From 60a5851b40f03fb71b2a3d30972d51ba40244d68 Mon Sep 17 00:00:00 2001
From: Serban Ungureanu
Date: Fri, 12 Jan 2024 14:33:32 +0200
Subject: [PATCH 1/2] [clang-format] Extend DefinitionBlockSeparatorStyle to
sep
@@ -171,6 +187,31 @@ void DefinitionBlockSeparator::separateBlocks(
return false;
};
+// Separate License text.
+const bool isComment = Lines[I]->isComment();
seranu wrote:
changed
https://github.com/llvm/llvm-project/pull/77918
___
@@ -65,18 +81,18 @@ void DefinitionBlockSeparator::separateBlocks(
}
return false;
};
- unsigned NewlineCount =
- (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always ? 1 : 0) + 1;
+ unsigned NewlineCount = getNewlineCount(Style.SeparateDefinitionBlocks);
https://github.com/seranu edited https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -586,7 +586,8 @@ template <>
struct ScalarEnumerationTraits {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value)
{
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
-IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+IO.enumCa
@@ -586,7 +586,8 @@ template <>
struct ScalarEnumerationTraits {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value)
{
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
-IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+IO.enumCa
seranu wrote:
> Maybe first only handle #42112 for what I will be really grateful.
#42112 requests separating both license text and include directives blocks.
> And then the stuff with the _license_, because I see discussion there. As far
> as I can tell you just declare all comments on top o
@@ -17,80 +17,97 @@
namespace clang {
namespace format {
namespace {
+std::string
+separateDefinitionBlocks(llvm::StringRef Code,
+ const std::vector &Ranges,
+ const FormatStyle &Style = getLLVMStyle()) {
+ LLVM_DEBUG(llvm::errs
@@ -586,7 +586,8 @@ template <>
struct ScalarEnumerationTraits {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value)
{
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
-IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+IO.enumCa
@@ -17,80 +17,97 @@
namespace clang {
namespace format {
namespace {
+std::string
+separateDefinitionBlocks(llvm::StringRef Code,
+ const std::vector &Ranges,
+ const FormatStyle &Style = getLLVMStyle()) {
+ LLVM_DEBUG(llvm::errs
@@ -65,18 +81,18 @@ void DefinitionBlockSeparator::separateBlocks(
}
return false;
};
- unsigned NewlineCount =
- (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always ? 1 : 0) + 1;
+ unsigned NewlineCount = getNewlineCount(Style.SeparateDefinitionBlocks);
@@ -65,18 +81,18 @@ void DefinitionBlockSeparator::separateBlocks(
}
return false;
};
- unsigned NewlineCount =
- (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always ? 1 : 0) + 1;
+ unsigned NewlineCount = getNewlineCount(Style.SeparateDefinitionBlocks);
@@ -586,7 +586,8 @@ template <>
struct ScalarEnumerationTraits {
static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value)
{
IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
-IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+IO.enumCa
@@ -171,6 +187,31 @@ void DefinitionBlockSeparator::separateBlocks(
return false;
};
+// Separate License text.
+const bool isComment = Lines[I]->isComment();
HazardyKnusperkeks wrote:
```suggestion
const bool IsComment = Lines[I]->isCom
https://github.com/HazardyKnusperkeks edited
https://github.com/llvm/llvm-project/pull/77918
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/HazardyKnusperkeks requested changes to this pull request.
I think you want too much in one change.
Maybe first only handle #42112 for what I will be really grateful. And then the
stuff with the _license_, because I see discussion there. As far as I can tell
you just declare
llvmbot wrote:
@llvm/pr-subscribers-clang-format
Author: serbanu (seranu)
Changes
Extend SeparateDefinitionStyle to support spacing license text, include blocks
and to also support two empty lines between blocks.
Fixes #42112 .
---
Patch is 24.66 KiB, truncated to 20.00 KiB below, full
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be
notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you, it i
https://github.com/seranu created
https://github.com/llvm/llvm-project/pull/77918
Extend SeparateDefinitionStyle to support spacing license text, include blocks
and to also support two empty lines between blocks.
Fixes #42112 .
>From 60a5851b40f03fb71b2a3d30972d51ba40244d68 Mon Sep 17 00:00:0
62 matches
Mail list logo
