GumpacG commented on code in PR #3401:
URL: https://github.com/apache/tinkerpop/pull/3401#discussion_r3163948854


##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/translator/PythonTranslateVisitor.java:
##########
@@ -289,6 +289,42 @@ public Void visitUuidLiteral(final 
GremlinParser.UuidLiteralContext ctx) {
         return null;
     }
 
+    @Override
+    public Void visitCharacterLiteral(final 
GremlinParser.CharacterLiteralContext ctx) {
+        final String text = ctx.getText();
+        final String withoutSuffix = text.substring(0, text.length() - 1);
+        final String inner = removeFirstAndLastCharacters(withoutSuffix);
+        sb.append("SingleChar('").append(inner).append("')");

Review Comment:
   Does this work well with repr()? I'm wondering if repr() could potentially 
mess this up as repr() switches the wrapping quotes to `"` if the string is a 
`'`



##########
gremlin-language/src/main/antlr4/Gremlin.g4:
##########
@@ -2500,6 +2520,26 @@ SignedInfLiteral
 
 // String Literals
 
+// Character literal is a single character string with a 'c' suffix, modeled 
after printf's %c.
+// Must appear before NonEmptyStringLiteral for longest-match priority.
+CharacterLiteral
+    : '"' DoubleQuotedStringCharacter '"' 'c'

Review Comment:
   Any thoughts about switching the grammar to `CHAR("a")` instead of `"a"c`? 
We have `datetime()`, `uuid()`, etc. to specify types. Would it be worth 
following this format?



##########
gremlin-language/src/main/antlr4/Gremlin.g4:
##########
@@ -2500,6 +2520,26 @@ SignedInfLiteral
 
 // String Literals
 
+// Character literal is a single character string with a 'c' suffix, modeled 
after printf's %c.
+// Must appear before NonEmptyStringLiteral for longest-match priority.
+CharacterLiteral
+    : '"' DoubleQuotedStringCharacter '"' 'c'
+    | '\'' SingleQuotedStringCharacter '\'' 'c'
+    ;
+
+// String literal with explicit 's' suffix. The suffix is optional (no suffix 
defaults to string),
+// but can be used to force string interpretation for single-character values.
+// Must appear before NonEmptyStringLiteral for longest-match priority.
+StringSuffixLiteral
+    : '"' DoubleQuotedStringCharacters '"' 's'

Review Comment:
   Should this be for `DoubleQuotedStringCharacter` instead of 
`DoubleQuotedStringCharacters` as it's only for a single character?



##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/GeneralLiteralVisitorTest.java:
##########
@@ -238,6 +241,11 @@ public static Iterable<Object[]> generateTestParameters() {
                     {"'abc\\u2300def'", "abc\u2300def"},
                     {"'\u2300'", "\u2300"},
                     {"'abc\u2300def'", "abc\u2300def"},
+                    // explicit 's' suffix for string literals
+                    {"\"hello\"s", "hello"},
+                    {"'hello's", "hello"},
+                    {"\"\"s", "Empty"},

Review Comment:
   Shouldn't `"\"\"s"` be `""`?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to