szaszm commented on a change in pull request #1267:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1267#discussion_r811957323
##########
File path: libminifi/include/utils/StringUtils.h
##########
@@ -493,6 +493,8 @@ class StringUtils {
*/
static std::smatch getLastRegexMatch(const std::string& str, const
std::regex& pattern);
+ static std::string escapeAscii(gsl::span<const std::byte> data);
Review comment:
This name doesn't describe the kind of escaping done. I would rename it
to `backslackEscapeUnprintableBytes` or something along these lines, unless
there is a widely used shorter alternative name to this functionality.
##########
File path: libminifi/include/core/logging/Logger.h
##########
@@ -53,28 +54,72 @@ class LoggerControl {
std::atomic<bool> is_enabled_;
};
-inline char const* conditional_conversion(std::string const& str) {
- return str.c_str();
-}
+template<typename Arg>
+using has_const_c_str_method = decltype(std::declval<const Arg&>().c_str());
+
+template<typename Arg>
+using has_str_method = decltype(std::declval<Arg>().str());
+
+template<typename Arg, typename = void>
+struct StringConverter {
+ decltype(auto) operator()(Arg&& val) const {
+ return std::forward<Arg>(val);
+ }
+};
+
+template<typename Arg>
+struct StringConverter<Arg, std::enable_if_t<
+ !utils::meta::is_detected_v<has_const_c_str_method, Arg> &&
+ utils::meta::is_detected_v<has_str_method, Arg>>> {
+ decltype(auto) operator()(Arg&& val) const {
+ return std::forward<Arg>(val).str();
+ }
+};
+
+template<typename Arg>
+struct StringConverter<Arg, std::enable_if_t<
+ !utils::meta::is_detected_v<has_const_c_str_method, Arg> &&
+ !utils::meta::is_detected_v<has_str_method, Arg> &&
+ std::is_invocable_v<Arg>>> {
+ decltype(auto) operator()(Arg&& val) const {
+ return std::forward<Arg>(val)();
+ }
+};
+
+template<typename Arg, typename = void>
+struct CStringConverter;
+
+template<typename Arg>
+struct CStringConverter<Arg, std::enable_if_t<
+ std::is_same_v<decltype(std::declval<const Arg&>().c_str()), const
char*>>> {
+ const char* operator()(const Arg& val) {
+ return val.c_str();
+ }
+};
+
+template<typename Arg>
+struct CStringConverter<Arg, std::enable_if_t<
+ std::is_scalar_v<std::decay_t<Arg>>>> {
+ const Arg& operator()(const Arg& val) {
+ return val;
+ }
+};
-template<size_t N>
-inline char const* conditional_conversion(const utils::SmallString<N>& arr) {
- return arr.c_str();
+template<typename Arg>
+inline decltype(auto) conditional_stringify(Arg&& arg) {
+ return StringConverter<Arg>{}(std::forward<Arg>(arg));
Review comment:
There is no need to complicate things with partial specialization. Just
use `if constexpr` in this function. The same applies to `CStringConverter`.
But I'd rather have the classes opt into this kind of special treatment
instead of just accepting anything that has a `.str()` or `.c_str()` members.
Like having something like a `std::formatter` specialization. Having to write
`.str()` or `.c_str()` on these log lines is fine, too.
My similar changes used an ADL `to_string` function and just called it
whenever necessary. ProcessSession::readBuffer is a recent example, but there
is also MessageStatus in PublishKafka.cpp.
--
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]
