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
>
