Expose a short-term flag to configure wrapping/indentation spaces.
This supports teams who insist on a "different" style than the default.
--port_declarations_indentation= {indent for 2 spaces, wrap for 4 spaces}
Initially, this only covers port declarations (modules, interfaces).
Other contexts may follow.
We do not publish these flags because they will be re-organized down the road.
issues #40
PiperOrigin-RevId: 330977558
diff --git a/common/formatting/BUILD b/common/formatting/BUILD
index d27c821..6d2972f 100644
--- a/common/formatting/BUILD
+++ b/common/formatting/BUILD
@@ -51,7 +51,21 @@
cc_library(
name = "basic_format_style",
+ srcs = ["basic_format_style.cc"],
hdrs = ["basic_format_style.h"],
+ deps = [
+ "//common/util:enum_flags",
+ "@com_google_absl//absl/strings",
+ ],
+)
+
+cc_test(
+ name = "basic_format_style_test",
+ srcs = ["basic_format_style_test.cc"],
+ deps = [
+ ":basic_format_style",
+ "@com_google_googletest//:gtest_main",
+ ],
)
cc_library(
diff --git a/common/formatting/basic_format_style.cc b/common/formatting/basic_format_style.cc
new file mode 100644
index 0000000..9ccf9da
--- /dev/null
+++ b/common/formatting/basic_format_style.cc
@@ -0,0 +1,58 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "common/formatting/basic_format_style.h"
+
+#include <initializer_list>
+#include <iostream>
+#include <map>
+#include <sstream>
+#include <string>
+#include <utility>
+
+#include "absl/strings/string_view.h"
+#include "common/util/enum_flags.h"
+
+namespace verible {
+
+namespace internal {
+// This mapping defines how this enum is displayed and parsed.
+constexpr std::initializer_list<
+ std::pair<const absl::string_view, IndentationStyle>>
+ kIndentationStyleStringMap = {
+ {"indent", IndentationStyle::kIndent},
+ {"wrap", IndentationStyle::kWrap},
+};
+} // namespace internal
+
+std::ostream& operator<<(std::ostream& stream, IndentationStyle p) {
+ static const auto* flag_map =
+ verible::MakeEnumToStringMap(internal::kIndentationStyleStringMap);
+ return stream << flag_map->find(p)->second;
+}
+
+bool AbslParseFlag(absl::string_view text, IndentationStyle* mode,
+ std::string* error) {
+ static const auto* flag_map =
+ verible::MakeStringToEnumMap(internal::kIndentationStyleStringMap);
+ return EnumMapParseFlag(*flag_map, text, mode, error);
+}
+
+std::string AbslUnparseFlag(const IndentationStyle& mode) {
+ std::ostringstream stream;
+ stream << mode;
+ return stream.str();
+}
+
+} // namespace verible
diff --git a/common/formatting/basic_format_style.h b/common/formatting/basic_format_style.h
index b93a0e1..cf4b782 100644
--- a/common/formatting/basic_format_style.h
+++ b/common/formatting/basic_format_style.h
@@ -15,6 +15,11 @@
#ifndef VERIBLE_COMMON_FORMATTING_BASIC_FORMAT_STYLE_H_
#define VERIBLE_COMMON_FORMATTING_BASIC_FORMAT_STYLE_H_
+#include <iosfwd>
+#include <string>
+
+#include "absl/strings/string_view.h"
+
namespace verible {
// Style configuration common to all languages.
@@ -36,6 +41,24 @@
int over_column_limit_penalty = 100;
};
+// Control how a section of code is indented.
+enum class IndentationStyle {
+ // One unit of indentation.
+ // This applies verible::BasicFormatStyle::indentation_spaces amount of
+ // spaces.
+ kIndent,
+ // Indentation that is applied to long, continued lines.
+ // This is also known as hang-indent.
+ // This applies verible::BasicFormatStyle::wrap_spaces amount of spaces.
+ kWrap,
+};
+
+std::ostream& operator<<(std::ostream&, IndentationStyle);
+
+bool AbslParseFlag(absl::string_view, IndentationStyle*, std::string*);
+
+std::string AbslUnparseFlag(const IndentationStyle&);
+
} // namespace verible
#endif // VERIBLE_COMMON_FORMATTING_BASIC_FORMAT_STYLE_H_
diff --git a/common/formatting/basic_format_style_test.cc b/common/formatting/basic_format_style_test.cc
new file mode 100644
index 0000000..f908553
--- /dev/null
+++ b/common/formatting/basic_format_style_test.cc
@@ -0,0 +1,44 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "common/formatting/basic_format_style.h"
+
+#include <string>
+#include <utility>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace verible {
+namespace {
+
+TEST(IndentationStyleParseFlagTest, Parse) {
+ std::string error;
+ IndentationStyle istyle;
+ // Test valid values.
+ EXPECT_TRUE(AbslParseFlag("indent", &istyle, &error));
+ EXPECT_EQ(istyle, IndentationStyle::kIndent);
+ EXPECT_TRUE(AbslParseFlag("wrap", &istyle, &error));
+ EXPECT_EQ(istyle, IndentationStyle::kWrap);
+ // Test for invalid string.
+ EXPECT_FALSE(AbslParseFlag("invalid", &istyle, &error));
+}
+
+TEST(IndentationStyleUnparseFlagTest, Unparse) {
+ EXPECT_EQ(AbslUnparseFlag(IndentationStyle::kIndent), "indent");
+ EXPECT_EQ(AbslUnparseFlag(IndentationStyle::kWrap), "wrap");
+}
+
+} // namespace
+} // namespace verible
diff --git a/verilog/formatting/BUILD b/verilog/formatting/BUILD
index 6238a5c..ec81614 100644
--- a/verilog/formatting/BUILD
+++ b/verilog/formatting/BUILD
@@ -229,26 +229,14 @@
cc_library(
name = "format_style",
- srcs = ["format_style.cc"],
hdrs = ["format_style.h"],
deps = [
"//common/formatting:align",
"//common/formatting:basic_format_style",
- "//common/util:enum_flags",
"@com_google_absl//absl/strings",
],
)
-cc_test(
- name = "format_style_test",
- srcs = ["format_style_test.cc"],
- deps = [
- ":format_style",
- "//common/util:enum_flags_test_util",
- "@com_google_googletest//:gtest_main",
- ],
-)
-
cc_library(
name = "token_annotator",
srcs = ["token_annotator.cc"],
diff --git a/verilog/formatting/format_style.cc b/verilog/formatting/format_style.cc
deleted file mode 100644
index abd3153..0000000
--- a/verilog/formatting/format_style.cc
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2017-2020 The Verible Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "verilog/formatting/format_style.h"
-
-#include <map>
-#include <sstream>
-#include <string>
-
-#include "absl/strings/string_view.h"
-#include "common/util/enum_flags.h"
-
-namespace verilog {
-namespace formatter {
-
-} // namespace formatter
-} // namespace verilog
diff --git a/verilog/formatting/format_style.h b/verilog/formatting/format_style.h
index fcc8454..885b9de 100644
--- a/verilog/formatting/format_style.h
+++ b/verilog/formatting/format_style.h
@@ -15,6 +15,10 @@
#ifndef VERIBLE_VERILOG_FORMATTING_FORMAT_STYLE_H_
#define VERIBLE_VERILOG_FORMATTING_FORMAT_STYLE_H_
+#include <iosfwd>
+#include <string>
+
+#include "absl/strings/string_view.h"
#include "common/formatting/align.h"
#include "common/formatting/basic_format_style.h"
@@ -24,11 +28,15 @@
// Style parameters that are specific to Verilog formatter
struct FormatStyle : public verible::BasicFormatStyle {
using AlignmentPolicy = verible::AlignmentPolicy;
+ using IndentationStyle = verible::IndentationStyle;
FormatStyle() : verible::BasicFormatStyle() {
over_column_limit_penalty = 10000;
}
+ // Control indentation amount for port declarations.
+ IndentationStyle port_declarations_indentation = IndentationStyle::kWrap;
+
// Control how named port_declaration (e.g. in modules, interfaces) are
// formatted. Internal tests assume these are forced to kAlign.
AlignmentPolicy port_declarations_alignment = AlignmentPolicy::kAlign;
@@ -78,6 +86,12 @@
// TODO(fangism): parameter to limit number of consecutive blank lines to
// preserve between partitions.
+ int PortDeclarationsIndentation() const {
+ return port_declarations_indentation == IndentationStyle::kWrap
+ ? wrap_spaces
+ : indentation_spaces;
+ }
+
void ApplyToAllAlignmentPolicies(AlignmentPolicy policy) {
port_declarations_alignment = policy;
named_parameter_alignment = policy;
diff --git a/verilog/formatting/format_style_test.cc b/verilog/formatting/format_style_test.cc
deleted file mode 100644
index af5ef67..0000000
--- a/verilog/formatting/format_style_test.cc
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2017-2020 The Verible Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "verilog/formatting/format_style.h"
-
-#include <utility>
-
-#include "gtest/gtest.h"
-#include "common/util/enum_flags_test_util.h"
-
-namespace verilog {
-namespace formatter {
-namespace {
-
-} // namespace
-} // namespace formatter
-} // namespace verilog
diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
index 70ec6d8..23b8a17 100644
--- a/verilog/formatting/formatter_test.cc
+++ b/verilog/formatting/formatter_test.cc
@@ -7477,6 +7477,65 @@
}
}
+TEST(FormatterEndToEndTest, ModulePortDeclarationsIndentNotWrap) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {"", ""},
+ {"\n", "\n"},
+ {"\n\n", "\n\n"},
+ {"module m ;\t\n"
+ " endmodule\n",
+ "module m;\n"
+ "endmodule\n"},
+ {"module m( ) ;\n"
+ " endmodule\n",
+ "module m ();\n" // empty ports formatted compactly
+ "endmodule\n"},
+ {// single port example
+ "module m ( input clk )\t;\n"
+ " endmodule\n",
+ "module m (\n"
+ " input clk\n" // 2 spaces
+ ");\n"
+ "endmodule\n"},
+ {// example with two ports
+ "module m (\n"
+ "input clk,\n"
+ "output bar\n"
+ ")\t;\n"
+ " endmodule\n",
+ "module m (\n"
+ " input clk,\n" // indented 2 spaces, and aligned
+ " output bar\n"
+ ");\n"
+ "endmodule\n"},
+ {// interface example
+ "interface handshake (\n"
+ "wire req,\n"
+ "wire ack\n"
+ ")\t;\n"
+ " endinterface\n",
+ "interface handshake (\n"
+ " wire req,\n" // indented 2 spaces
+ " wire ack\n"
+ ");\n"
+ "endinterface\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ // Indent 2 spaces instead of wrapping 4 spaces.
+ style.port_declarations_indentation = verible::IndentationStyle::kIndent;
+ for (const auto& test_case : kTestCases) {
+ VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status =
+ FormatVerilog(test_case.input, "<filename>", style, stream);
+ // Require these test cases to be valid.
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
struct SelectLinesTestCase {
absl::string_view input;
LineNumberSet lines; // explicit set of lines to enable formatting
diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc
index 1b1e96c..c13b775 100644
--- a/verilog/formatting/tree_unwrapper.cc
+++ b/verilog/formatting/tree_unwrapper.cc
@@ -1030,7 +1030,21 @@
break;
}
- case NodeEnum::kPortDeclarationList:
+ case NodeEnum::kPortDeclarationList: {
+ // Do not further indent preprocessor clauses.
+ const int indent =
+ suppress_indentation ? 0 : style_.PortDeclarationsIndentation();
+ if (Context().IsInside(NodeEnum::kClassHeader) ||
+ // kModuleHeader covers interfaces and programs
+ Context().IsInside(NodeEnum::kModuleHeader)) {
+ VisitIndentedSection(node, indent,
+ PartitionPolicyEnum::kTabularAlignment);
+ } else {
+ VisitIndentedSection(node, indent,
+ PartitionPolicyEnum::kFitOnLineElseExpand);
+ }
+ break;
+ }
case NodeEnum::kFormalParameterList: {
// Do not further indent preprocessor clauses.
const int indent = suppress_indentation ? 0 : style_.wrap_spaces;
diff --git a/verilog/tools/formatter/verilog_format.cc b/verilog/tools/formatter/verilog_format.cc
index f504886..a3fbb63 100644
--- a/verilog/tools/formatter/verilog_format.cc
+++ b/verilog/tools/formatter/verilog_format.cc
@@ -47,6 +47,7 @@
using absl::StatusCode;
using verible::AlignmentPolicy;
+using verible::IndentationStyle;
using verible::LineNumberSet;
using verilog::formatter::ExecutionControl;
using verilog::formatter::FormatStyle;
@@ -122,6 +123,8 @@
// These flags exist in the short term to disable formatting of some regions.
// Do not expect to be able to use these in the long term, once they find
// a better home in a configuration struct.
+ABSL_FLAG(IndentationStyle, port_declarations_indentation,
+ IndentationStyle::kWrap, "Indent port declarations: {indent,wrap}");
// For most of the following in this group, kInferUserIntent is a reasonable
// default behavior because it allows for user-control with minimal invasiveness
@@ -205,6 +208,10 @@
// formatting style flags
format_style.try_wrap_long_lines = absl::GetFlag(FLAGS_try_wrap_long_lines);
+ // various indentation control
+ format_style.port_declarations_indentation =
+ absl::GetFlag(FLAGS_port_declarations_indentation);
+
// various alignment control
format_style.port_declarations_alignment =
absl::GetFlag(FLAGS_port_declarations_alignment);
