Remove newlines from docstring signature (#4735)

JeanElsner · pre-commit-ci[bot] · web-flow · commit b9359ceadbf4 · 2023-08-15T07:48:59.000-07:00
* Remove newlines from docstring signature * Jean/dev (#1) Replace newlines in arg values with spaces * style: pre-commit fixes * Don't use std::find_if for C++ 11 compatibility * Avoid implicit char to bool conversion * Test default arguments for line breaks * style: pre-commit fixes * Separate Eigen tests * style: pre-commit fixes * Fix merge * Try importing numpy * Avoid unreferenced variable in catch block * style: pre-commit fixes * Update squash function * Reduce try block * Additional test cases * style: pre-commit fixes * Put statement inside braces * Move string into function body * Rename repr for better readability. Make constr explicit. * Add multiline string default argument test case * style: pre-commit fixes * Add std namespace, do not modify string repr * Test for all space chars, test str repr not modified --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -52,6 +52,45 @@ PYBIND11_WARNING_DISABLE_MSVC(4127)
 
 PYBIND11_NAMESPACE_BEGIN(detail)
 
+inline std::string replace_newlines_and_squash(const char *text) {
+    const char *whitespaces = " \t\n\r\f\v";
+    std::string result(text);
+    bool previous_is_whitespace = false;
+
+    // Do not modify string representations
+    char first_char = result[0];
+    char last_char = result[result.size() - 1];
+    if (first_char == last_char && first_char == '\'') {
+        return result;
+    }
+    result.clear();
+
+    // Replace characters in whitespaces array with spaces and squash consecutive spaces
+    while (*text != '\0') {
+        if (std::strchr(whitespaces, *text)) {
+            if (!previous_is_whitespace) {
+                result += ' ';
+                previous_is_whitespace = true;
+            }
+        } else {
+            result += *text;
+            previous_is_whitespace = false;
+        }
+        ++text;
+    }
+
+    // Strip leading and trailing whitespaces
+    const size_t str_begin = result.find_first_not_of(whitespaces);
+    if (str_begin == std::string::npos) {
+        return "";
+    }
+
+    const size_t str_end = result.find_last_not_of(whitespaces);
+    const size_t str_range = str_end - str_begin + 1;
+
+    return result.substr(str_begin, str_range);
+}
+
 // Apply all the extensions translators from a list
 // Return true if one of the translators completed without raising an exception
 // itself. Return of false indicates that if there are other translators
@@ -424,7 +463,7 @@ class cpp_function : public function {
                 // Write default value if available.
                 if (!is_starred && arg_index < rec->args.size() && rec->args[arg_index].descr) {
                     signature += " = ";
-                    signature += rec->args[arg_index].descr;
+                    signature += detail::replace_newlines_and_squash(rec->args[arg_index].descr);
                 }
                 // Separator for positional-only arguments (placed after the
                 // argument, rather than before like *
diff --git a/tests/test_eigen_matrix.cpp b/tests/test_eigen_matrix.cpp
@@ -330,6 +330,23 @@ TEST_SUBMODULE(eigen_matrix, m) {
     m.def("dense_c", [mat]() -> DenseMatrixC { return DenseMatrixC(mat); });
     m.def("dense_copy_r", [](const DenseMatrixR &m) -> DenseMatrixR { return m; });
     m.def("dense_copy_c", [](const DenseMatrixC &m) -> DenseMatrixC { return m; });
+    // test_defaults
+    bool have_numpy = true;
+    try {
+        py::module_::import("numpy");
+    } catch (const py::error_already_set &) {
+        have_numpy = false;
+    }
+    if (have_numpy) {
+        py::module_::import("numpy");
+        Eigen::Matrix<double, 3, 3> defaultMatrix = Eigen::Matrix3d::Identity();
+        m.def(
+            "defaults_mat", [](const Eigen::Matrix3d &) {}, py::arg("mat") = defaultMatrix);
+
+        Eigen::VectorXd defaultVector = Eigen::VectorXd::Ones(32);
+        m.def(
+            "defaults_vec", [](const Eigen::VectorXd &) {}, py::arg("vec") = defaultMatrix);
+    }
     // test_sparse, test_sparse_signature
     m.def("sparse_r", [mat]() -> SparseMatrixR {
         // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.UndefReturn)
diff --git a/tests/test_eigen_matrix.py b/tests/test_eigen_matrix.py
@@ -716,6 +716,11 @@ def test_dense_signature(doc):
     )
 
 
+def test_defaults(doc):
+    assert "\n" not in str(doc(m.defaults_mat))
+    assert "\n" not in str(doc(m.defaults_vec))
+
+
 def test_named_arguments():
     a = np.array([[1.0, 2], [3, 4], [5, 6]])
     b = np.ones((2, 1))
diff --git a/tests/test_kwargs_and_defaults.cpp b/tests/test_kwargs_and_defaults.cpp
@@ -42,6 +42,50 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
     m.def("kw_func_udl", kw_func, "x"_a, "y"_a = 300);
     m.def("kw_func_udl_z", kw_func, "x"_a, "y"_a = 0);
 
+    // test line breaks in default argument representation
+    struct CustomRepr {
+        std::string repr_string;
+
+        explicit CustomRepr(const std::string &repr) : repr_string(repr) {}
+
+        std::string __repr__() const { return repr_string; }
+    };
+
+    py::class_<CustomRepr>(m, "CustomRepr")
+        .def(py::init<const std::string &>())
+        .def("__repr__", &CustomRepr::__repr__);
+
+    m.def(
+        "kw_lb_func0",
+        [](const CustomRepr &) {},
+        py::arg("custom") = CustomRepr("  array([[A, B], [C, D]])  "));
+    m.def(
+        "kw_lb_func1",
+        [](const CustomRepr &) {},
+        py::arg("custom") = CustomRepr("  array([[A, B],\n[C, D]])  "));
+    m.def(
+        "kw_lb_func2",
+        [](const CustomRepr &) {},
+        py::arg("custom") = CustomRepr("\v\n   array([[A, B], [C, D]])"));
+    m.def(
+        "kw_lb_func3",
+        [](const CustomRepr &) {},
+        py::arg("custom") = CustomRepr("array([[A, B], [C, D]])   \f\n"));
+    m.def(
+        "kw_lb_func4",
+        [](const CustomRepr &) {},
+        py::arg("custom") = CustomRepr("array([[A, B],\n\f\n[C, D]])"));
+    m.def(
+        "kw_lb_func5",
+        [](const CustomRepr &) {},
+        py::arg("custom") = CustomRepr("array([[A, B],\r  [C, D]])"));
+    m.def(
+        "kw_lb_func6", [](const CustomRepr &) {}, py::arg("custom") = CustomRepr(" \v\t "));
+    m.def(
+        "kw_lb_func7",
+        [](const std::string &) {},
+        py::arg("str_arg") = "First line.\n  Second line.");
+
     // test_args_and_kwargs
     m.def("args_function", [](py::args args) -> py::tuple {
         PYBIND11_WARNING_PUSH
diff --git a/tests/test_kwargs_and_defaults.py b/tests/test_kwargs_and_defaults.py
@@ -23,6 +23,38 @@ def test_function_signatures(doc):
         doc(m.KWClass.foo1)
         == "foo1(self: m.kwargs_and_defaults.KWClass, x: int, y: float) -> None"
     )
+    assert (
+        doc(m.kw_lb_func0)
+        == "kw_lb_func0(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
+    )
+    assert (
+        doc(m.kw_lb_func1)
+        == "kw_lb_func1(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
+    )
+    assert (
+        doc(m.kw_lb_func2)
+        == "kw_lb_func2(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
+    )
+    assert (
+        doc(m.kw_lb_func3)
+        == "kw_lb_func3(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
+    )
+    assert (
+        doc(m.kw_lb_func4)
+        == "kw_lb_func4(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
+    )
+    assert (
+        doc(m.kw_lb_func5)
+        == "kw_lb_func5(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
+    )
+    assert (
+        doc(m.kw_lb_func6)
+        == "kw_lb_func6(custom: m.kwargs_and_defaults.CustomRepr = ) -> None"
+    )
+    assert (
+        doc(m.kw_lb_func7)
+        == "kw_lb_func7(str_arg: str = 'First line.\\n  Second line.') -> None"
+    )
 
 
 def test_named_arguments():
