chromeos-dbus-bindings: Minor code formatting and comment fixes
Made minor code formatting changes to the generated code and fixed
comment generation from <tp:docstring> element to maintain relative
in-comment indentation as seen in peerd's Service.xml file.
So the comment tag like this:
<tp:docstring>
A list of IP addresses, each composed of:
- an array of bytes containing the octets of the IP
address (either an IPv4 or an IPv6 address)
- a 16 bit port number.
</tp:docstring>
Would become a code comment like this:
// A list of IP addresses, each composed of:
// - an array of bytes containing the octets of the IP
// address (either an IPv4 or an IPv6 address)
// - a 16 bit port number.
BUG=None
TEST=FEATURES=test emerge-link chromeos-dbus-bindings
Manually inspect the generated proxy/adapters for
existing components (buffet, peerd, privetd).
Change-Id: I794533a26a75991a9a89382f185df721a1b53793
Reviewed-on: https://chromium-review.googlesource.com/232060
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/chromeos-dbus-bindings/adaptor_generator.cc b/chromeos-dbus-bindings/adaptor_generator.cc
index 5ffdf70..8576907 100644
--- a/chromeos-dbus-bindings/adaptor_generator.cc
+++ b/chromeos-dbus-bindings/adaptor_generator.cc
@@ -37,6 +37,7 @@
text.AddLine(StringPrintf("#define %s", header_guard.c_str()));
text.AddLine("#include <memory>");
text.AddLine("#include <string>");
+ text.AddLine("#include <tuple>");
text.AddLine("#include <vector>");
text.AddBlankLine();
text.AddLine("#include <base/macros.h>");
@@ -436,11 +437,9 @@
CHECK(signature.Parse(property.type, &type));
string variable_name = GetPropertyVariableName(property.name);
- block.AddLine(StringPrintf("chromeos::dbus_utils::ExportedProperty<%s>",
- type.c_str()));
- block.PushOffset(kLineContinuationOffset);
- block.AddLine(StringPrintf("%s_;", variable_name.c_str()));
- block.PopOffset();
+ block.AddLine(
+ StringPrintf("chromeos::dbus_utils::ExportedProperty<%s> %s_;",
+ type.c_str(), variable_name.c_str()));
}
if (!interface.properties.empty())
block.AddBlankLine();
diff --git a/chromeos-dbus-bindings/adaptor_generator_unittest.cc b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
index 583b1ea..d63054f 100644
--- a/chromeos-dbus-bindings/adaptor_generator_unittest.cc
+++ b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
@@ -52,6 +52,7 @@
const char kExpectedContent[] = R"literal_string(
#include <memory>
#include <string>
+#include <tuple>
#include <vector>
#include <base/macros.h>
@@ -151,8 +152,7 @@
std::vector<dbus::ObjectPath>>;
std::weak_ptr<SignalMappingType> signal_Mapping_;
- chromeos::dbus_utils::ExportedProperty<std::string>
- character_name_;
+ chromeos::dbus_utils::ExportedProperty<std::string> character_name_;
TestInterface* interface_; // Owned by container of this adapter.
diff --git a/chromeos-dbus-bindings/indented_text.cc b/chromeos-dbus-bindings/indented_text.cc
index ea6b45a..5a8ed6f 100644
--- a/chromeos-dbus-bindings/indented_text.cc
+++ b/chromeos-dbus-bindings/indented_text.cc
@@ -9,6 +9,7 @@
#include <vector>
#include <base/logging.h>
+#include <base/strings/string_util.h>
#include <chromeos/strings/string_utils.h>
using std::string;
@@ -41,10 +42,47 @@
}
void IndentedText::AddComments(const std::string& doc_string) {
- // Split at \n, trim all whitespaces and eliminate empty strings.
- auto lines = chromeos::string_utils::Split(doc_string, '\n', true, true);
- for (const auto& line : lines) {
- AddLine("// " + line);
+ // Try to retain indentation in the comments. Find the first non-empty line
+ // of the comment and find its whitespace indentation prefix.
+ // For all subsequent lines, remove the same whitespace prefix as found
+ // at the first line of the comment but keep any additional spaces to
+ // maintain the comment layout.
+ auto lines = chromeos::string_utils::Split(doc_string, '\n', false, false);
+ vector<string> lines_out;
+ lines_out.reserve(lines.size());
+ bool first_nonempty_found = false;
+ std::string trim_prefix;
+ for (string line : lines) {
+ base::TrimWhitespaceASCII(line, base::TRIM_TRAILING, &line);
+ if (!first_nonempty_found) {
+ size_t pos = line.find_first_not_of(" \t");
+ if (pos != std::string::npos) {
+ first_nonempty_found = true;
+ trim_prefix = line.substr(0, pos);
+ lines_out.push_back(line.substr(pos));
+ }
+ } else {
+ if (StartsWithASCII(line, trim_prefix, false)) {
+ line = line.substr(trim_prefix.length());
+ } else {
+ base::TrimWhitespaceASCII(line, base::TRIM_LEADING, &line);
+ }
+ lines_out.push_back(line);
+ }
+ }
+
+ // We already eliminated all empty lines at the beginning of the comment
+ // block. Now remove the trailing empty lines.
+ while (!lines_out.empty() && lines_out.back().empty())
+ lines_out.pop_back();
+
+ for (const string& line : lines_out) {
+ const bool all_whitespace = (line.find_first_not_of(" \t") == string::npos);
+ if (all_whitespace) {
+ AddLine("//");
+ } else {
+ AddLine("// " + line);
+ }
}
}
diff --git a/chromeos-dbus-bindings/indented_text_unittest.cc b/chromeos-dbus-bindings/indented_text_unittest.cc
index ef60082..d615770 100644
--- a/chromeos-dbus-bindings/indented_text_unittest.cc
+++ b/chromeos-dbus-bindings/indented_text_unittest.cc
@@ -125,4 +125,45 @@
EXPECT_TRUE(GetHistory().empty());
}
+TEST_F(IndentedTextTest, AddComments_Empty) {
+ text_.AddComments("");
+ EXPECT_EQ("", text_.GetContents());
+}
+
+TEST_F(IndentedTextTest, AddComments_WhitespaceOnly) {
+ text_.AddComments(" \n \t \n");
+ EXPECT_EQ("", text_.GetContents());
+}
+
+TEST_F(IndentedTextTest, AddComments_EmptyLines) {
+ string comment_block = R"(
+
+ line1
+
+ line2
+
+
+ )";
+ text_.AddComments(comment_block);
+ EXPECT_EQ("// line1\n"
+ "//\n"
+ "// line2\n", text_.GetContents());
+}
+
+TEST_F(IndentedTextTest, AddComments_Indentation) {
+ string comment_block = R"(
+ line1
+ - bullet1
+ line2
+ - bullet2
+ line3
+ )";
+ text_.AddComments(comment_block);
+ EXPECT_EQ("// line1\n"
+ "// - bullet1\n"
+ "// line2\n"
+ "// - bullet2\n"
+ "// line3\n", text_.GetContents());
+}
+
} // namespace chromeos_dbus_bindings
diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index 17d299a..01f2a75 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -86,6 +86,7 @@
text->AddLine(StringPrintf("// Interface proxy for %s.",
GetFullClassName(namespaces, itf_name).c_str()));
+ text->AddComments(interface.doc_string);
text->AddLine(StringPrintf("class %s final {", proxy_name.c_str()));
text->AddLineWithOffset("public:", kScopeOffset);
text->PushOffset(kBlockOffset);
