Copilot commented on code in PR #2521:
URL: https://github.com/apache/groovy/pull/2521#discussion_r3213166174
##########
subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/jline/GroovyCommands.groovy:
##########
@@ -214,6 +236,200 @@ class GroovyCommands extends JlineCommandRegistry
implements CommandRegistry {
return null
}
+ /**
+ * Displays an image inline using JLine's terminal-graphics support
+ * (Sixel, Kitty, iTerm2). Falls back to a summary line when the
+ * terminal doesn't speak any supported protocol; the {@code --gui}
+ * flag opens a Swing window instead.
+ *
+ * @param input parsed command input
+ * @return always {@code null}
+ */
+ def img(CommandInput input) {
+ checkArgCount(input, [0, 1, 2, 3, 4, 5])
+ if (maybePrintHelp(input, '/img')) return
+ try {
+ Integer width = null
+ Integer height = null
+ boolean preserveAspect = true
+ boolean gui = false
+ Object positional = null
+ String positionalLabel = null
+ for (int i = 0; i < input.args().length; i++) {
+ String a = input.args()[i]
+ if (a == null) {
+ // JLine puts null into args() when a $var reference
resolves
+ // to null — usually because the variable isn't defined
yet.
+ throw new IllegalArgumentException(
+ '/img: variable reference resolved to null ' +
+ '(undefined or not yet assigned) — define
it first, e.g. ' +
+ "'panel =
ScatterPlot.of(...).canvas().panel()'")
+ }
+ if (a == '-w' || a == '--width') {
+ width = Integer.parseInt(input.args()[++i])
+ } else if (a.startsWith('--width=')) {
+ width = Integer.parseInt(a.substring('--width='.length()))
Review Comment:
The `-w/--width` branch unconditionally consumes the next token via
`input.args()[++i]`. If the user passes `--width` without a value (or as the
last argument), this will throw an
`ArrayIndexOutOfBoundsException`/`NullPointerException` and end up as a less
helpful saved exception. Please add an explicit bounds/value check and throw a
targeted `IllegalArgumentException` explaining the missing width value.
##########
subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/jline/GroovyCommands.groovy:
##########
@@ -214,6 +236,200 @@ class GroovyCommands extends JlineCommandRegistry
implements CommandRegistry {
return null
}
+ /**
+ * Displays an image inline using JLine's terminal-graphics support
+ * (Sixel, Kitty, iTerm2). Falls back to a summary line when the
+ * terminal doesn't speak any supported protocol; the {@code --gui}
+ * flag opens a Swing window instead.
+ *
+ * @param input parsed command input
+ * @return always {@code null}
+ */
+ def img(CommandInput input) {
+ checkArgCount(input, [0, 1, 2, 3, 4, 5])
Review Comment:
`/img` currently restricts the argument count to 0..5 via `checkArgCount`,
but valid invocations can exceed this when using the space-separated forms of
`--width`/`--height` together with flags (e.g. `--width 64 --height 32 --gui
$img` => 6+ args). Consider removing the fixed arg-count check for this
command, or increasing it and/or validating based on parsed options + exactly
one positional argument instead of raw token count.
##########
subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/jline/GroovyCommands.groovy:
##########
@@ -214,6 +236,200 @@ class GroovyCommands extends JlineCommandRegistry
implements CommandRegistry {
return null
}
+ /**
+ * Displays an image inline using JLine's terminal-graphics support
+ * (Sixel, Kitty, iTerm2). Falls back to a summary line when the
+ * terminal doesn't speak any supported protocol; the {@code --gui}
+ * flag opens a Swing window instead.
+ *
+ * @param input parsed command input
+ * @return always {@code null}
+ */
+ def img(CommandInput input) {
+ checkArgCount(input, [0, 1, 2, 3, 4, 5])
+ if (maybePrintHelp(input, '/img')) return
+ try {
+ Integer width = null
+ Integer height = null
+ boolean preserveAspect = true
+ boolean gui = false
+ Object positional = null
+ String positionalLabel = null
+ for (int i = 0; i < input.args().length; i++) {
+ String a = input.args()[i]
+ if (a == null) {
+ // JLine puts null into args() when a $var reference
resolves
+ // to null — usually because the variable isn't defined
yet.
+ throw new IllegalArgumentException(
+ '/img: variable reference resolved to null ' +
+ '(undefined or not yet assigned) — define
it first, e.g. ' +
+ "'panel =
ScatterPlot.of(...).canvas().panel()'")
+ }
+ if (a == '-w' || a == '--width') {
+ width = Integer.parseInt(input.args()[++i])
+ } else if (a.startsWith('--width=')) {
+ width = Integer.parseInt(a.substring('--width='.length()))
+ } else if (a == '--height') {
+ height = Integer.parseInt(input.args()[++i])
+ } else if (a.startsWith('--height=')) {
+ height =
Integer.parseInt(a.substring('--height='.length()))
Review Comment:
The `--height` branch unconditionally consumes the next token via
`input.args()[++i]`. If the user passes `--height` without a value (or as the
last argument), this will throw and surface as an internal exception. Please
add an explicit bounds/value check and throw a clear `IllegalArgumentException`
for the missing height value.
##########
subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/commands/ImgTest.groovy:
##########
@@ -0,0 +1,151 @@
+/*
+ * 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.
+ */
+package org.apache.groovy.groovysh.commands
+
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.io.TempDir
+
+import javax.imageio.ImageIO
+import javax.swing.JLabel
+import java.awt.image.BufferedImage
+import java.nio.file.Files
Review Comment:
Unused import: `java.nio.file.Files` is imported but not referenced anywhere
in this test file.
##########
subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc:
##########
@@ -720,6 +720,75 @@ groovy> /imports
import java.util.concurrent.BlockingQueue
--------------
+[[GroovyShell-img]]
+==== `/img`
+
+The `/img` command displays an image inline using JLine's terminal-graphics
+support (Sixel, Kitty, or iTerm2 protocols, auto-detected). The argument is
+either a local file path, an `http(s)://` URL, or a Groovy variable
+reference using the standard `$` syntax.
+
+[source,jshell]
+--------------
+groovy> /img chart.png
+groovy> /img --width=80 https://example.com/diagram.png
+groovy> img = new java.awt.image.BufferedImage(200, 100, 1) // some
BufferedImage
+groovy> /img $img
+--------------
+
+When the argument resolves to a Groovy value (rather than a path), `/img`
+will accept any of:
+
+* a `java.awt.image.BufferedImage` — used as-is
+* a `java.awt.image.RenderedImage` — drawn into a `BufferedImage`
+* anything with a `createBufferedImage(int, int)` method (e.g.
+ `org.jfree.chart.JFreeChart`) — duck-typed, no compile-time dependency
+* anything with a `toBufferedImage(int, int)` method (e.g. Smile's
+ `smile.plot.swing.Figure`) — duck-typed sibling for libraries that
+ follow the `to…` rather than `create…` naming convention
+* a `javax.swing.JComponent` (e.g. Smile's `Canvas` and `MultiFigurePane`) —
+ laid out and painted to a `BufferedImage` at the requested size
+
+So once you've grabbed JFreeChart you can render charts in the REPL
+without saving to a file:
+
+image:assets/img/repl_img_jfreechart.png[Using /img with JFreeChart]
+
+Smile is similar — `Figure.toBufferedImage(int, int)` matches the second
+duck-type, so a fresh `Figure` can be handed straight to `/img`. The
+following draws the classic Iris scatter plot:
+
+image:assets/img/repl_img_smile.png[Using /img with Smile]
Review Comment:
These new images use `image:assets/img/...` while the rest of this document
uses `image:{reldir_groovysh}/assets/img/...`. Using the consistent
`{reldir_groovysh}` prefix here will avoid broken image links when the docs are
built from different base directories.
##########
subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc:
##########
@@ -720,6 +720,75 @@ groovy> /imports
import java.util.concurrent.BlockingQueue
--------------
+[[GroovyShell-img]]
+==== `/img`
+
+The `/img` command displays an image inline using JLine's terminal-graphics
+support (Sixel, Kitty, or iTerm2 protocols, auto-detected). The argument is
+either a local file path, an `http(s)://` URL, or a Groovy variable
+reference using the standard `$` syntax.
+
+[source,jshell]
+--------------
+groovy> /img chart.png
+groovy> /img --width=80 https://example.com/diagram.png
+groovy> img = new java.awt.image.BufferedImage(200, 100, 1) // some
BufferedImage
+groovy> /img $img
+--------------
+
+When the argument resolves to a Groovy value (rather than a path), `/img`
+will accept any of:
+
+* a `java.awt.image.BufferedImage` — used as-is
+* a `java.awt.image.RenderedImage` — drawn into a `BufferedImage`
+* anything with a `createBufferedImage(int, int)` method (e.g.
+ `org.jfree.chart.JFreeChart`) — duck-typed, no compile-time dependency
+* anything with a `toBufferedImage(int, int)` method (e.g. Smile's
+ `smile.plot.swing.Figure`) — duck-typed sibling for libraries that
+ follow the `to…` rather than `create…` naming convention
+* a `javax.swing.JComponent` (e.g. Smile's `Canvas` and `MultiFigurePane`) —
+ laid out and painted to a `BufferedImage` at the requested size
+
+So once you've grabbed JFreeChart you can render charts in the REPL
+without saving to a file:
+
+image:assets/img/repl_img_jfreechart.png[Using /img with JFreeChart]
+
+Smile is similar — `Figure.toBufferedImage(int, int)` matches the second
+duck-type, so a fresh `Figure` can be handed straight to `/img`. The
+following draws the classic Iris scatter plot:
+
+image:assets/img/repl_img_smile.png[Using /img with Smile]
Review Comment:
These new images use `image:assets/img/...` while the rest of this document
uses `image:{reldir_groovysh}/assets/img/...`. Using the consistent
`{reldir_groovysh}` prefix here will avoid broken image links when the docs are
built from different base directories.
##########
subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/jline/GroovyCommands.groovy:
##########
@@ -871,6 +1087,18 @@ class GroovyCommands extends JlineCommandRegistry
implements CommandRegistry {
])
}
+ private CmdDesc imgCmdDesc(String name) {
+ new CmdDesc([
+ new AttributedString("$name [OPTIONS] FILE_OR_URL"),
Review Comment:
The `/img` help/usage line says `FILE_OR_URL`, but the implementation and
docs also accept a Groovy variable reference (e.g. `/img $imgVar`) and other
non-string values resolved via `xargs`. Updating the usage synopsis to reflect
the supported inputs would make `--help` less misleading.
##########
subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/jline/GroovyCommands.groovy:
##########
@@ -871,6 +1087,18 @@ class GroovyCommands extends JlineCommandRegistry
implements CommandRegistry {
])
}
+ private CmdDesc imgCmdDesc(String name) {
+ new CmdDesc([
+ new AttributedString("$name [OPTIONS] FILE_OR_URL"),
+ ], [], [
+ '-? --help' : doDescription('Displays
command help'),
+ '-w --width=COLS' : doDescription('Image width in
character columns'),
+ ' --height=ROWS' : doDescription('Image height in
character rows'),
+ '-p --no-preserve-aspect-ratio' : doDescription("Don't preserve
aspect ratio"),
Review Comment:
`--width`/`--height` are described here as character columns/rows, but for
generated-image inputs (`createBufferedImage`/`toBufferedImage`/`JComponent`)
the same options are interpreted as source-image pixels (as described later in
the user docs and implied by `dimsConsumedByGeneration`). Consider clarifying
the option descriptions (or adding a note) so `--help` matches the actual
behavior.
--
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]