You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "kirill-stepanishin (via GitHub)" <gi...@apache.org> on 2026/02/06 17:44:25 UTC

[PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

kirill-stepanishin opened a new pull request, #3304:
URL: https://github.com/apache/tinkerpop/pull/3304

   ## Description:
   
   As part of the TP4 migration, the server no longer accepts Bytecode and only processes Gremlin queries in script form. This PR introduces the GremlinLang class, which builds Gremlin script strings incrementally.
   
   This covers the core class and unit tests only. It does not yet integrate with GraphTraversal, GraphTraversalSource, or the driver layer.
   
   GremlinLang supports:
   - addStep() / addSource() for building traversal scripts
   - Argument serialization: primitives, strings, arrays, objects/maps, P/TextP predicates, EnumValue, TraversalStrategy with configuration
   - OptionsStrategy extraction
   - Clone support for the immutable GraphTraversalSource pattern
   - getGremlin(prefix) — defaults to "g", supports "__" for anonymous traversals
   
   Follow-up PR will wire GremlinLang into GraphTraversal/GraphTraversalSource, update the driver to submit string queries, add nested traversal support, and delete Bytecode/Translator.


-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2775919669


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/gremlin-lang-test.js:
##########
@@ -0,0 +1,254 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import assert from 'assert';
+import GremlinLang from '../../lib/process/gremlin-lang.js';
+import { P, TextP, order, t } from '../../lib/process/traversal.js';
+import { OptionsStrategy, ReadOnlyStrategy } from '../../lib/process/traversal-strategy.js';
+
+describe('GremlinLang', function () {
+  describe('#getGremlin()', function () {
+    it('should return "g" for empty traversal with default prefix', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin(), 'g');
+    });
+
+    it('should return custom prefix for empty traversal', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin('__'), '__');
+    });
+  });
+
+  describe('#addStep()', function () {
+    it('should add single step', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').getGremlin(), 'g.V()');
+    });
+
+    it('should chain multiple steps', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').addStep('count').getGremlin(), 'g.V().count()');
+    });
+
+    it('should handle number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [1]).getGremlin(), 'g.V(1)');
+    });
+
+    it('should handle boolean arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['active', true]).getGremlin(), "g.has('active', true)");
+    });
+
+    it('should handle null arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('property', ['x', null]).getGremlin(), "g.property('x', null)");
+    });
+
+    it('should handle multiple number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['count', 42]).getGremlin(), "g.has('count', 42)");
+    });
+
+    it('should handle string arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['name', 'josh']).getGremlin(), "g.has('name', 'josh')");
+    });
+
+    it('should escape single quotes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', "it's"]).getGremlin(), "g.has('x', 'it\\'s')");
+    });
+
+    it('should escape backslashes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', 'a\\b']).getGremlin(), "g.has('x', 'a\\\\b')");
+    });
+
+    it('should handle array arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[1, 2, 3]]).getGremlin(), 'g.V([1, 2, 3])');
+    });
+
+    it('should handle empty array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[]]).getGremlin(), 'g.V([])');
+    });
+
+    it('should handle nested arrays', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [[[1, 2], [3, 4]]]).getGremlin(), 'g.inject([[1, 2], [3, 4]])');
+    });
+
+    it('should handle array with mixed types', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [['a', 1, true, null]]).getGremlin(), "g.inject(['a', 1, true, null])");
+    });
+  });
+
+  describe('clone support', function () {
+    it('should create independent copy', function () {
+      const original = new GremlinLang();
+      original.addStep('V').addStep('count');
+      const clone = new GremlinLang(original);
+      assert.strictEqual(clone.getGremlin(), original.getGremlin());
+    });
+
+    it('should not affect original when modifying clone', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      clone.addStep('count');
+      assert.strictEqual(original.getGremlin(), 'g.V()');
+      assert.strictEqual(clone.getGremlin(), 'g.V().count()');
+    });
+
+    it('should not affect clone when modifying original', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      original.addStep('count');
+      assert.strictEqual(clone.getGremlin(), 'g.V()');
+      assert.strictEqual(original.getGremlin(), 'g.V().count()');
+    });
+  });
+
+  describe('P and TextP predicates', function () {
+    it('should handle P.gt predicate', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5)]).getGremlin(), "g.has('age', gt(5))");

Review Comment:
   Nit: Some of these test cases don't represent valid gremlin, in this case it should likely be `g.V().has('age', gt(5))` as `has()` is not a start step. I don't expect `GremlinLang` to do this sort of validation as this is already covered by `GraphTraversal` and the grammar, however it is preferred to use valid examples here.



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2775932797


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/gremlin-lang-test.js:
##########
@@ -0,0 +1,254 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import assert from 'assert';
+import GremlinLang from '../../lib/process/gremlin-lang.js';
+import { P, TextP, order, t } from '../../lib/process/traversal.js';
+import { OptionsStrategy, ReadOnlyStrategy } from '../../lib/process/traversal-strategy.js';
+
+describe('GremlinLang', function () {
+  describe('#getGremlin()', function () {
+    it('should return "g" for empty traversal with default prefix', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin(), 'g');
+    });
+
+    it('should return custom prefix for empty traversal', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin('__'), '__');
+    });
+  });
+
+  describe('#addStep()', function () {
+    it('should add single step', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').getGremlin(), 'g.V()');
+    });
+
+    it('should chain multiple steps', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').addStep('count').getGremlin(), 'g.V().count()');
+    });
+
+    it('should handle number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [1]).getGremlin(), 'g.V(1)');
+    });
+
+    it('should handle boolean arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['active', true]).getGremlin(), "g.has('active', true)");
+    });
+
+    it('should handle null arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('property', ['x', null]).getGremlin(), "g.property('x', null)");
+    });
+
+    it('should handle multiple number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['count', 42]).getGremlin(), "g.has('count', 42)");
+    });
+
+    it('should handle string arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['name', 'josh']).getGremlin(), "g.has('name', 'josh')");
+    });
+
+    it('should escape single quotes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', "it's"]).getGremlin(), "g.has('x', 'it\\'s')");
+    });
+
+    it('should escape backslashes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', 'a\\b']).getGremlin(), "g.has('x', 'a\\\\b')");
+    });
+
+    it('should handle array arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[1, 2, 3]]).getGremlin(), 'g.V([1, 2, 3])');
+    });
+
+    it('should handle empty array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[]]).getGremlin(), 'g.V([])');
+    });
+
+    it('should handle nested arrays', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [[[1, 2], [3, 4]]]).getGremlin(), 'g.inject([[1, 2], [3, 4]])');
+    });
+
+    it('should handle array with mixed types', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [['a', 1, true, null]]).getGremlin(), "g.inject(['a', 1, true, null])");
+    });
+  });
+
+  describe('clone support', function () {
+    it('should create independent copy', function () {
+      const original = new GremlinLang();
+      original.addStep('V').addStep('count');
+      const clone = new GremlinLang(original);
+      assert.strictEqual(clone.getGremlin(), original.getGremlin());
+    });
+
+    it('should not affect original when modifying clone', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      clone.addStep('count');
+      assert.strictEqual(original.getGremlin(), 'g.V()');
+      assert.strictEqual(clone.getGremlin(), 'g.V().count()');
+    });
+
+    it('should not affect clone when modifying original', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      original.addStep('count');
+      assert.strictEqual(clone.getGremlin(), 'g.V()');
+      assert.strictEqual(original.getGremlin(), 'g.V().count()');
+    });
+  });
+
+  describe('P and TextP predicates', function () {
+    it('should handle P.gt predicate', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5)]).getGremlin(), "g.has('age', gt(5))");
+    });
+
+    it('should handle P.within with array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [P.within([1, 2, 3])]).getGremlin(), "g.V(within([1, 2, 3]))");

Review Comment:
   Similar to above, this gremlin isn't quite right, something like this would be preferred.
   ```suggestion
         assert.strictEqual(gremlinLang.addStep('V').addStep('has', [T.id, P.within([1, 2, 3])]).getGremlin(), "g.V().has(T.id,P.within([1,2,3]))");
   ```



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "kirill-stepanishin (via GitHub)" <gi...@apache.org>.
kirill-stepanishin closed pull request #3304: Add GremlinLang class for JavaScript GLV
URL: https://github.com/apache/tinkerpop/pull/3304


-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2776157031


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/gremlin-lang-test.js:
##########
@@ -0,0 +1,254 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import assert from 'assert';
+import GremlinLang from '../../lib/process/gremlin-lang.js';
+import { P, TextP, order, t } from '../../lib/process/traversal.js';
+import { OptionsStrategy, ReadOnlyStrategy } from '../../lib/process/traversal-strategy.js';
+
+describe('GremlinLang', function () {
+  describe('#getGremlin()', function () {
+    it('should return "g" for empty traversal with default prefix', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin(), 'g');
+    });
+
+    it('should return custom prefix for empty traversal', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin('__'), '__');
+    });
+  });
+
+  describe('#addStep()', function () {
+    it('should add single step', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').getGremlin(), 'g.V()');
+    });
+
+    it('should chain multiple steps', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').addStep('count').getGremlin(), 'g.V().count()');
+    });
+
+    it('should handle number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [1]).getGremlin(), 'g.V(1)');
+    });
+
+    it('should handle boolean arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['active', true]).getGremlin(), "g.has('active', true)");
+    });
+
+    it('should handle null arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('property', ['x', null]).getGremlin(), "g.property('x', null)");
+    });
+
+    it('should handle multiple number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['count', 42]).getGremlin(), "g.has('count', 42)");
+    });
+
+    it('should handle string arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['name', 'josh']).getGremlin(), "g.has('name', 'josh')");
+    });
+
+    it('should escape single quotes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', "it's"]).getGremlin(), "g.has('x', 'it\\'s')");
+    });
+
+    it('should escape backslashes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', 'a\\b']).getGremlin(), "g.has('x', 'a\\\\b')");
+    });
+
+    it('should handle array arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[1, 2, 3]]).getGremlin(), 'g.V([1, 2, 3])');

Review Comment:
   Can you add an extra case to cover "special numbers" `Infinity`, `-Infinity` and `NaN`?



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#issuecomment-3861855277

   ## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3304?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 78.31%. Comparing base ([`cfd6889`](https://app.codecov.io/gh/apache/tinkerpop/commit/cfd68897ea111e3ae7100dbc2f0f7a364fd0ffa5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`66ee665`](https://app.codecov.io/gh/apache/tinkerpop/commit/66ee66501e9e188e3debabd1838c6548ed81c41c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 762 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #3304      +/-   ##
   ============================================
   + Coverage     77.87%   78.31%   +0.44%     
   ============================================
     Files          1015       26     -989     
     Lines         59308     5506   -53802     
     Branches       6835        0    -6835     
   ============================================
   - Hits          46184     4312   -41872     
   + Misses        10817     1035    -9782     
   + Partials       2307      159    -2148     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/3304?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2776147978


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/gremlin-lang.ts:
##########
@@ -0,0 +1,102 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import { P, TextP, EnumValue } from './traversal.js';
+import { OptionsStrategy, TraversalStrategy } from './traversal-strategy.js';
+
+export default class GremlinLang {
+  private gremlin: string = '';
+  private optionsStrategies: OptionsStrategy[] = [];
+  
+  constructor(toClone?: GremlinLang) {
+    if (toClone) {
+      this.gremlin = toClone.gremlin;
+      this.optionsStrategies = [...toClone.optionsStrategies];
+    }
+  }
+  
+  getOptionsStrategies(): OptionsStrategy[] {
+    return this.optionsStrategies;
+  }
+  
+  private _argAsString(arg: any): string {
+    if (arg === null || arg === undefined) return 'null';
+    if (typeof arg === 'boolean') return arg ? 'true' : 'false';
+    if (typeof arg === 'number') return String(arg);

Review Comment:
   We should also handle [`utils.Long`](https://github.com/apache/tinkerpop/blob/88ff81736982a7bd636a17e5a57187613feb3554/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/utils.ts#L33) here to cover integers which fit into Gremlin/Java's long, but are too big for javascripts `number`.
   
   We will also need to further consider what to do with the small number types like byte, short, and float. There isn't necessarily a nice answer as the numeric types just don't map well into JS. I think we can leave that out of scope for this PR, but we will have to face it before we can get all of the feature tests running again.



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2776147978


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/gremlin-lang.ts:
##########
@@ -0,0 +1,102 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import { P, TextP, EnumValue } from './traversal.js';
+import { OptionsStrategy, TraversalStrategy } from './traversal-strategy.js';
+
+export default class GremlinLang {
+  private gremlin: string = '';
+  private optionsStrategies: OptionsStrategy[] = [];
+  
+  constructor(toClone?: GremlinLang) {
+    if (toClone) {
+      this.gremlin = toClone.gremlin;
+      this.optionsStrategies = [...toClone.optionsStrategies];
+    }
+  }
+  
+  getOptionsStrategies(): OptionsStrategy[] {
+    return this.optionsStrategies;
+  }
+  
+  private _argAsString(arg: any): string {
+    if (arg === null || arg === undefined) return 'null';
+    if (typeof arg === 'boolean') return arg ? 'true' : 'false';
+    if (typeof arg === 'number') return String(arg);

Review Comment:
   We should also handle [`utils.Long`](https://github.com/apache/tinkerpop/blob/88ff81736982a7bd636a17e5a57187613feb3554/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/utils.ts#L33) here to cover integers which fit into Gremlin/Java's long, but are too big for javascripts `number`.



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2775947380


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/gremlin-lang-test.js:
##########
@@ -0,0 +1,254 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import assert from 'assert';
+import GremlinLang from '../../lib/process/gremlin-lang.js';
+import { P, TextP, order, t } from '../../lib/process/traversal.js';
+import { OptionsStrategy, ReadOnlyStrategy } from '../../lib/process/traversal-strategy.js';
+
+describe('GremlinLang', function () {
+  describe('#getGremlin()', function () {
+    it('should return "g" for empty traversal with default prefix', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin(), 'g');
+    });
+
+    it('should return custom prefix for empty traversal', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin('__'), '__');
+    });
+  });
+
+  describe('#addStep()', function () {
+    it('should add single step', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').getGremlin(), 'g.V()');
+    });
+
+    it('should chain multiple steps', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').addStep('count').getGremlin(), 'g.V().count()');
+    });
+
+    it('should handle number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [1]).getGremlin(), 'g.V(1)');
+    });
+
+    it('should handle boolean arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['active', true]).getGremlin(), "g.has('active', true)");
+    });
+
+    it('should handle null arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('property', ['x', null]).getGremlin(), "g.property('x', null)");
+    });
+
+    it('should handle multiple number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['count', 42]).getGremlin(), "g.has('count', 42)");
+    });
+
+    it('should handle string arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['name', 'josh']).getGremlin(), "g.has('name', 'josh')");
+    });
+
+    it('should escape single quotes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', "it's"]).getGremlin(), "g.has('x', 'it\\'s')");
+    });
+
+    it('should escape backslashes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', 'a\\b']).getGremlin(), "g.has('x', 'a\\\\b')");
+    });
+
+    it('should handle array arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[1, 2, 3]]).getGremlin(), 'g.V([1, 2, 3])');
+    });
+
+    it('should handle empty array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[]]).getGremlin(), 'g.V([])');
+    });
+
+    it('should handle nested arrays', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [[[1, 2], [3, 4]]]).getGremlin(), 'g.inject([[1, 2], [3, 4]])');
+    });
+
+    it('should handle array with mixed types', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [['a', 1, true, null]]).getGremlin(), "g.inject(['a', 1, true, null])");
+    });
+  });
+
+  describe('clone support', function () {
+    it('should create independent copy', function () {
+      const original = new GremlinLang();
+      original.addStep('V').addStep('count');
+      const clone = new GremlinLang(original);
+      assert.strictEqual(clone.getGremlin(), original.getGremlin());
+    });
+
+    it('should not affect original when modifying clone', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      clone.addStep('count');
+      assert.strictEqual(original.getGremlin(), 'g.V()');
+      assert.strictEqual(clone.getGremlin(), 'g.V().count()');
+    });
+
+    it('should not affect clone when modifying original', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      original.addStep('count');
+      assert.strictEqual(clone.getGremlin(), 'g.V()');
+      assert.strictEqual(original.getGremlin(), 'g.V().count()');
+    });
+  });
+
+  describe('P and TextP predicates', function () {
+    it('should handle P.gt predicate', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5)]).getGremlin(), "g.has('age', gt(5))");
+    });
+
+    it('should handle P.within with array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [P.within([1, 2, 3])]).getGremlin(), "g.V(within([1, 2, 3]))");
+    });
+
+    it('should handle P.within with empty array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [P.within([])]).getGremlin(), "g.V(within([]))");
+    });
+
+    it('should handle TextP.containing predicate', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['name', TextP.containing('foo')]).getGremlin(), "g.has('name', containing('foo'))");
+    });
+
+    it('should handle chained P predicates', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5).and(P.lt(10))]).getGremlin(), "g.has('age', and(gt(5), lt(10)))");

Review Comment:
   This looks like `P.and` is getting mixed up with the `and()` step here. The expected GremlinLang should be
   ```suggestion
         assert.strictEqual(gremlinLang.addStep('V').addStep('has', ['age', P.gt(5).and(P.lt(10))]).getGremlin(), "g.V().has("age",gt(5).and(lt(10)))");
   ```



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2775932797


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/gremlin-lang-test.js:
##########
@@ -0,0 +1,254 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import assert from 'assert';
+import GremlinLang from '../../lib/process/gremlin-lang.js';
+import { P, TextP, order, t } from '../../lib/process/traversal.js';
+import { OptionsStrategy, ReadOnlyStrategy } from '../../lib/process/traversal-strategy.js';
+
+describe('GremlinLang', function () {
+  describe('#getGremlin()', function () {
+    it('should return "g" for empty traversal with default prefix', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin(), 'g');
+    });
+
+    it('should return custom prefix for empty traversal', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.getGremlin('__'), '__');
+    });
+  });
+
+  describe('#addStep()', function () {
+    it('should add single step', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').getGremlin(), 'g.V()');
+    });
+
+    it('should chain multiple steps', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V').addStep('count').getGremlin(), 'g.V().count()');
+    });
+
+    it('should handle number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [1]).getGremlin(), 'g.V(1)');
+    });
+
+    it('should handle boolean arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['active', true]).getGremlin(), "g.has('active', true)");
+    });
+
+    it('should handle null arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('property', ['x', null]).getGremlin(), "g.property('x', null)");
+    });
+
+    it('should handle multiple number arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['count', 42]).getGremlin(), "g.has('count', 42)");
+    });
+
+    it('should handle string arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['name', 'josh']).getGremlin(), "g.has('name', 'josh')");
+    });
+
+    it('should escape single quotes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', "it's"]).getGremlin(), "g.has('x', 'it\\'s')");
+    });
+
+    it('should escape backslashes in strings', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['x', 'a\\b']).getGremlin(), "g.has('x', 'a\\\\b')");
+    });
+
+    it('should handle array arguments', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[1, 2, 3]]).getGremlin(), 'g.V([1, 2, 3])');
+    });
+
+    it('should handle empty array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [[]]).getGremlin(), 'g.V([])');
+    });
+
+    it('should handle nested arrays', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [[[1, 2], [3, 4]]]).getGremlin(), 'g.inject([[1, 2], [3, 4]])');
+    });
+
+    it('should handle array with mixed types', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('inject', [['a', 1, true, null]]).getGremlin(), "g.inject(['a', 1, true, null])");
+    });
+  });
+
+  describe('clone support', function () {
+    it('should create independent copy', function () {
+      const original = new GremlinLang();
+      original.addStep('V').addStep('count');
+      const clone = new GremlinLang(original);
+      assert.strictEqual(clone.getGremlin(), original.getGremlin());
+    });
+
+    it('should not affect original when modifying clone', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      clone.addStep('count');
+      assert.strictEqual(original.getGremlin(), 'g.V()');
+      assert.strictEqual(clone.getGremlin(), 'g.V().count()');
+    });
+
+    it('should not affect clone when modifying original', function () {
+      const original = new GremlinLang();
+      original.addStep('V');
+      const clone = new GremlinLang(original);
+      original.addStep('count');
+      assert.strictEqual(clone.getGremlin(), 'g.V()');
+      assert.strictEqual(original.getGremlin(), 'g.V().count()');
+    });
+  });
+
+  describe('P and TextP predicates', function () {
+    it('should handle P.gt predicate', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5)]).getGremlin(), "g.has('age', gt(5))");
+    });
+
+    it('should handle P.within with array', function () {
+      const gremlinLang = new GremlinLang();
+      assert.strictEqual(gremlinLang.addStep('V', [P.within([1, 2, 3])]).getGremlin(), "g.V(within([1, 2, 3]))");

Review Comment:
   Similar to above, this gremlin isn't quite right, as `V()` doesn't take predicates to match ids. Something like this would be preferred.
   ```suggestion
         assert.strictEqual(gremlinLang.addStep('V').addStep('has', [T.id, P.within([1, 2, 3])]).getGremlin(), "g.V().has(T.id,P.within([1,2,3]))");
   ```



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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


Re: [PR] Add GremlinLang class for JavaScript GLV [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3304:
URL: https://github.com/apache/tinkerpop/pull/3304#discussion_r2776178917


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/gremlin-lang.ts:
##########
@@ -0,0 +1,102 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+import { P, TextP, EnumValue } from './traversal.js';
+import { OptionsStrategy, TraversalStrategy } from './traversal-strategy.js';
+
+export default class GremlinLang {
+  private gremlin: string = '';
+  private optionsStrategies: OptionsStrategy[] = [];
+  
+  constructor(toClone?: GremlinLang) {
+    if (toClone) {
+      this.gremlin = toClone.gremlin;
+      this.optionsStrategies = [...toClone.optionsStrategies];
+    }
+  }
+  
+  getOptionsStrategies(): OptionsStrategy[] {
+    return this.optionsStrategies;
+  }
+  
+  private _argAsString(arg: any): string {
+    if (arg === null || arg === undefined) return 'null';
+    if (typeof arg === 'boolean') return arg ? 'true' : 'false';
+    if (typeof arg === 'number') return String(arg);
+    if (typeof arg === 'string') {
+      const escaped = arg.replace(/\\/g, '\\\\').replace(/'/g, "\\'");
+      return `'${escaped}'`;
+    }
+    if (arg instanceof P || arg instanceof TextP) {
+      return arg.toString();
+    }
+    if (arg instanceof EnumValue) {
+      return arg.toString();
+    }
+    if (arg instanceof TraversalStrategy && !(arg instanceof OptionsStrategy)) {
+      const simpleName = arg.fqcn.split('.').pop() || arg.fqcn;
+      const configEntries = Object.entries(arg.configuration);
+      if (configEntries.length === 0) {
+        return simpleName;
+      }
+      const configStr = configEntries.map(([k, v]) => `${k}:${this._argAsString(v)}`).join(', ');
+      return `new ${simpleName}(${configStr})`;
+    }
+    if (Array.isArray(arg)) {
+      return '[' + arg.map(a => this._argAsString(a)).join(', ') + ']';
+    }
+    if (typeof arg === 'object' && arg !== null && arg.constructor === Object) {
+      const entries = Object.entries(arg);
+      if (entries.length === 0) return '[:]';
+      return '[' + entries.map(([k, v]) => `${this._argAsString(k)}:${this._argAsString(v)}`).join(', ') + ']';
+    }
+    return String(arg);

Review Comment:
   There are a few missing types which will need cases here (and tests):
   
   - Date
   - Set
   - Map
   
   UUID is also missing however I think this can be ignored for now as I think it needs to be completely redone in JS for TP4.



-- 
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: commits-unsubscribe@tinkerpop.apache.org

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