fgerlits commented on a change in pull request #1170:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1170#discussion_r716619048



##########
File path: libminifi/include/utils/StringUtils.h
##########
@@ -75,7 +75,12 @@ class StringUtils {
 
   static std::string toLower(std::string_view str);
 
-  // Trim String utils
+  /**
+   * Strips the line ending (\n or \r\n) from the end of the input line.
+   * @param input_line
+   * @return (stripped line, line ending) pair
+   */
+  static std::pair<std::string, std::string> chomp(const std::string& 
input_line);

Review comment:
       That would require modifying `endsWith()` (and, for consistency, 
`endsWithIgnoreCase()` and `startsWith()`), which would introduce more 
conflicts with #1168.  If #1168 is merged before this, and it includes the 
change to `endsWith()`, then I will change `chomp()` similarly.
   
   EDIT: I'm not sure we want to do this.  As you pointed out elsewhere, we'd 
still need to keep the version taking a `std::string` argument to deal with 
temporary strings.  So we'll end up with 3 overloads (to remove the ambiguity 
when called with a `const char*`), and in most cases I'll need to convert the 
result to a string after calling `chomp()` anyway.  So this would mean quite a 
bit of extra code, for not much benefit.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to