-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(types): remove newlines from docstring signature #4735
Changes from 6 commits
c89e5b2
214b200
99cd8d0
f795b7e
9640613
293e6e2
d8e7d2d
418bdef
4521195
d47aa39
7a9382a
15196d1
5d7e555
a2d675f
035a044
19a8d51
122e15a
e6be9d5
1b5a3d7
4c844d5
9918f22
25c9657
0ea0a13
a2add0d
1a9dd7b
d53892c
9cefda8
68c5643
d3885da
b391386
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,43 @@ PYBIND11_WARNING_DISABLE_MSVC(4127) | |
|
||
PYBIND11_NAMESPACE_BEGIN(detail) | ||
|
||
inline std::string replaceNewlinesAndSquash(const char *text) { | ||
std::string result; | ||
|
||
// Replace newlines with spaces and squash consecutive spaces | ||
while (*text != '\0') { | ||
if (*text == '\n') { | ||
result += ' '; | ||
while (*(text + 1) == ' ') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to reuse the same list of whitespace characters as below? |
||
++text; | ||
} | ||
} else { | ||
result += *text; | ||
} | ||
++text; | ||
} | ||
|
||
// Strip leading spaces | ||
size_t firstNonSpace = result.find_first_not_of(" \t\n\r\f\v"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please move the string of whitespace characters to a constant and reuse? |
||
|
||
if (firstNonSpace != std::string::npos) { | ||
result.erase(0, firstNonSpace); | ||
} else { | ||
result.clear(); | ||
} | ||
|
||
// Strip trailing spaces | ||
size_t lastNonSpace = result.find_last_not_of(" \t\n\r\f\v"); | ||
|
||
if (lastNonSpace != std::string::npos) { | ||
result.erase(lastNonSpace + 1); | ||
} else { | ||
result.clear(); | ||
} | ||
|
||
return result; | ||
} | ||
|
||
// 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 +461,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::replaceNewlinesAndSquash(rec->args[arg_index].descr); | ||
} | ||
// Separator for positional-only arguments (placed after the | ||
// argument, rather than before like * | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use camel-case I think. Could you please use
the_prevailing_underscore_style
for the function name and the variable names in this function?