coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770247583



##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& 
regexName,
 
        std::string tmp;
 
+       auto sedSanitizer = [] (const std::string& in, const std::string& 
sedSeparator = "Q")
+       {
+               std::string ret = in;
+               std::string replaceTo = "\\" + sedSeparator;
+               size_t pos = 0;
+
+               while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+               {
+                       ret.replace(pos, sedSeparator.length(), replaceTo);
+                       pos += replaceTo.length();
+               }
+
+               return ret;
+       };
+
        for (log4cxx::Filter::PatternList::const_iterator iter = 
patterns.begin();
                iter != patterns.end();
                iter++)
        {
                tmp = "sQ";

Review comment:
       Yes, this PR is only focusing on the delimiter only -- to fix the test 
issues I saw when packaging... 
   
   Regarding your sed: Did you try the switch `-E` instead? `--regexp-extend` 
is not POSIX, but `-E` would be, so maybe it understands only that switch (TIL: 
the feature is indeed POSIX; before, I thought this was not a portable feature)
   
   Of course, if wanted, I can extend sedSanitizer  to escape more characters, 
so that it `-E` (aka `--rexep-extended` is not needed), or look into 
boost::regexp to avoid the invocation to sed alltogether. Though this feels for 
me as overkill just for the tests (having to have boost I mean) 




-- 
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: notifications-unsubscr...@logging.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to