markap14 commented on a change in pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#discussion_r534464347



##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
##########
@@ -602,8 +602,7 @@ public boolean isAllDataBufferedForEntireText() {
         @Override
         public FlowFile replace(FlowFile flowFile, final ProcessSession 
session, final ProcessContext context, final String evaluateMode, final Charset 
charset, final int maxBufferSize) {

Review comment:
       I can understand why the existence of it and the removal of it would be 
confusing. I don't think a comment in the code would be helpful though. It 
never should have been escaped. I think it was escaped as a result of 
refactoring the code, or perhaps because it is escaped when using a Regular 
Expression as the search value.... but if we added an inline comment about why 
we're not arbitrarily escaping something that doesn't need escaping, outside of 
the scope of this PR, I think it would result in more confusion than 
clarification :)




----------------------------------------------------------------
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.

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


Reply via email to