[
https://issues.apache.org/jira/browse/TINKERPOP-3248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18118652#comment-18118652
]
ASF GitHub Bot commented on TINKERPOP-3248:
-------------------------------------------
kenhuuu commented on code in PR #3673:
URL: https://github.com/apache/tinkerpop/pull/3673#discussion_r4089784454
##########
gremlin-go/driver/gremlinlang_test.go:
##########
@@ -717,6 +717,20 @@ func Test_GremlinLang(t *testing.T) {
},
equals: `g.inject(Binary("AA=="))`,
},
+ {
+ name: "g_Inject_Char",
+ assert: func(g *GraphTraversalSource) *GraphTraversal {
+ return g.Inject(Char('a'))
+ },
+ equals: `g.inject("a"c)`,
+ },
+ {
+ name: "g_Inject_Char_EscapedQuote",
+ assert: func(g *GraphTraversalSource) *GraphTraversal {
+ return g.Inject(Char('"'))
+ },
+ equals: `g.inject("\""c)`,
+ },
Review Comment:
I think we should probably add some more cases here like we did with
gremlin-python:
```
Char('\\') // g.inject("\\"c)
Char('\'') // g.inject("'"c)
Char('\n') // g.inject("\n"c)
rune('a') // g.inject(97), confirming rune remains Int
```
##########
gremlin-js/gremlin-javascript/test/unit/translator/gremlin-translator-test.js:
##########
@@ -285,6 +285,8 @@ describe('GoTranslateVisitor', function () {
['g.inject(Duration(9000,0))', 'g.Inject(time.Duration(9000000000000))'],
// Binary literal
['g.inject(Binary("AQID"))', 'g.Inject(gremlingo.ByteBuffer{Data:
[]byte{1,2,3}})'],
+ // Character literal
+ ['g.inject("a"c)', "g.Inject(gremlingo.Char('a'))"],
Review Comment:
I think we should have more tests here too. same as the ones I said with
gremlinlang
```
"a"c
"\""c
"\\"c
"'"c
'\''c
```
that last one is particularly important because I think it might be a bug.
```
// Current, invalid Go
gremlingo.Char(''')
// Required
gremlingo.Char('\'')
```
also, Gremlin supports one- and two-digit octal escapes
```
"\7"c
"\07"c
```
> Character support in gremlin-go
> -------------------------------
>
> Key: TINKERPOP-3248
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3248
> Project: TinkerPop
> Issue Type: Improvement
> Components: go
> Affects Versions: 3.8.1
> Reporter: Ken Hu
> Priority: Major
>
> gremlin-go doesn't support the Character right now but it should be
> representable with Rune. Add this to both GremlinLang and GraphBinary in
> gremlin-go.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
