tinkerpop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmallette <...@git.apache.org>
Subject [GitHub] tinkerpop pull request #934: Apply StringEscapeUtils in GroovyTranslator#con...
Date Fri, 21 Sep 2018 19:46:22 GMT
Github user spmallette commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/934#discussion_r219608379
  
    --- Diff: gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslator.java
---
    @@ -115,7 +116,8 @@ else if (object instanceof Bytecode)
             else if (object instanceof Traversal)
                 return convertToString(((Traversal) object).asAdmin().getBytecode());
             else if (object instanceof String) {
    -            return (((String) object).contains("\"") ? "\"\"\"" + object + "\"\"\"" :
"\"" + object + "\"").replace("$", "\\$");
    +            return (((String) object).contains("\"") ? "\"\"\"" + StringEscapeUtils.escapeJava((String)
object) + "\"\"\"" : "\"" + StringEscapeUtils.escapeJava((String) object) + "\"")
    +                    .replace("$", "\\$");
    --- End diff --
    
    since you used the "groovy" package for this, i was wondering if it still handles the
"$" properly or if we still need the final `replace("$", "\\$")`? either way, could you please
modify your tests to include a "$" character so that we can be sure that gets asserted?


---

Mime
View raw message