[GitHub] [nifi] markap14 commented on a change in pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org