Thanks for getting to this so quickly. I appreciate it very much. I had an agent take a quick look at the merged changes and it should cover everything we need. I'll do a quick test against the RC version when that's available on Maven Central, but based on static analysis it should be good. Thanks again.
On Sat, Sep 12, 2026 at 7:01 PM Daniel Sun <[email protected]> wrote: > Hi Ken, > > Thanks for the careful write-up. That matches what disappeared in > the JLine 3 groovysh rewrite, and the four asks look reasonable > to me. > > I'd rather not hang embedding hooks off the binding Map that > Main.start already takes. Those keys would collide with user > variables and are awkward from Java. A small GroovyshOptions type > (incubating) plus Main.start(GroovyshOptions, String[]) keeps > bindings as bindings. > > The rest maps fairly directly onto what you suggested: > > 1. GroovyEngine(CompilerConfiguration), and a protected > createShell(...) for subclasses. ImportCustomizer and > ThreadInterrupt then apply at compile time instead of being > re-prepended as snippets on every evaluation. > > 2. GroovyshOptions for extra CommandRegistry instances, a prompt > supplier, result/error handlers, a banner toggle, and an > optional Terminal. > > 3. ExtraConsoleCommands as a public top-level class. The nested > Main.ExtraConsoleCommands can remain as a deprecated subclass > so existing Groovy callers still compile. > > 4. GroovyEngine.getClassLoader(). > > This is new public API, so I'm inclined to land it on the current > development > line rather than a 5.1.x backport. Marking GroovyshOptions > incubating leaves room to adjust once Gremlin Console is actually > on it. > > I've started a candidate along these lines and will share a PR[1]. > Please do say if that would be enough for TinkerPop, or if I'm > still missing a seam you need. > > [1] https://github.com/apache/groovy/pull/2927 > > Cheers, > Daniel Sun > > > On 2026/09/11 21:17:57 Ken Hu wrote: > > Hi all, > > > > I'm working on upgrading Apache TinkerPop from Groovy 4.0.32 to 5.1.2. > > TinkerPop ships the Gremlin Console, the primary REPL for the Gremlin > > language, and it is built by embedding groovysh. The Groovy 5 rewrite > onto > > JLine 3 removed the extension points we depended on. I'd like to discuss > > what a supported embedding path could look like and whether the Groovy > > community would accept this change for Groovy 5 or defer to Groovy 6. > > > > The Gremlin Console preloads the Gremlin DSL so users can type > > "g.V().out()" directly. That is 63 imports: 40 package star imports and > 23 > > "import static X.*". In Groovy 4 we installed them as an ImportCustomizer > > on Groovysh's CompilerConfiguration, and we added an > > ASTTransformationCustomizer(ThreadInterrupt) so ctrl+C can interrupt a > > long-running query. > > > > In 5.1.2, GroovyEngine creates its shell as > > > > new GroovyShell(classLoader, sharedData) > > > > at GroovyEngine.java:171, with no configuration parameter, and the shell > > field is private final. CompilerConfiguration.DEFAULT is immutable in 5, > so > > there is no indirect route either. > > > > The only supported alternative is executing import statements, which > > GroovyEngine records as snippets and textually re-prepends to every > > evaluation but as we've talked about previously on this list, this can > > lead to a substantial performance loss. > > > > ------------------------------ > > > > Main.start(Map<String,?>, String...) accepts only binding variables, and > > the body hardcodes the terminal, parser, engine, registries, prompt, > banner > > and REPL loop. We need to register our own commands, render results our > own > > way (Gremlin auto-iterates result sets with a user-configurable cap), and > > control the prompt and error rendering. Today that means copying roughly > > 230 lines of Main into TinkerPop and re-diffing it against every Groovy > > release. > > > > Since start already takes a Map, the smallest version of this could be a > > few recognized keys: extra CommandRegistry instances, a prompt supplier, > a > > result handler, an error handler, a banner toggle. > > > > ------------------------------ > > > > Main.ExtraConsoleCommands provides /clear /pwd /cd /ls /cat /grep /head > > /tail /wc /sort /date /echo and /!. It is protected static nested in > Main. > > It currently compiles from another package only because Groovy does not > > enforce protected access at compile time, which would not hold from Java > or > > under CompileStatic. Anyone embedding the REPL has to copy it. > > > > ------------------------------ > > > > Our :install command uses Grape plus ServiceLoader against the shell's > > classloader so freshly downloaded plugins load without a restart. The > > classLoader field is protected, so subclassing works today. A getter > would > > just avoid requiring a subclass in order to read one field. > > > > To recap, I have several asks: > > Ask 1: add GroovyEngine(CompilerConfiguration), or a protected > GroovyShell > > createShell(ClassLoader, Binding) seam that a subclass can override. > > Ask 2: embedding hooks on Main > > Ask 3: make Main.ExtraConsoleCommands a public top-level class > > Ask 4: a public accessor for the engine's classloader (minor) > > > > Please let me know if I'm going about this the wrong way and there are > > actually already extension points that I should be using instead. > > > > Thanks, > > Ken > > >
