[GitHub] [jmeter] woonsan commented on a change in pull request #660: bz65108: escape/unescape jmeter variable references

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

woonsan commented on a change in pull request #660:
URL: https://github.com/apache/jmeter/pull/660#discussion_r742735571



##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -56,6 +55,12 @@
 
     private static final Pattern WHITESPACES_PATTERN = Pattern.compile("\\p{Space}+");
 
+    private static final Pattern JMETER_VARIABLE_REFERENCE_PATTERN = Pattern
+            .compile("(:\\s*)\\$\\{([^\\$\\{\\}]+)\\}([\\s,\\}])");

Review comment:
       Hi @FSchumacher ,
   
   I'm very sorry for late response.
   My intent there is to find a string, (a) starting with "${", followed by (b) any char(s) excluding '$', '{' and '}', followed by (c) "}".  The negative part is only for (b): var name shouldn't include the special chars. I believe it should work even if it looks a bit complex.
   
   If we want to create Pattern objects every time instead of having just one generic Pattern, I'm not so sure how it could improve the readability because the varname should probably be dynamically concatenated at runtime to build a regex pattern, which might be less readable in the end, IMHO.
   
   Regarding the complex setup example with function, is it possible to use that complex variable expression in JMeter? (If so, perhaps we might need to replace the regex by commons-text's StrSubstitutor instead.
   
   Regards,
   
   Woonsan




-- 
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: dev-unsubscribe@jmeter.apache.org

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