Copilot commented on code in PR #2948:
URL: https://github.com/apache/groovy/pull/2948#discussion_r4220026883
##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/Console.groovy:
##########
@@ -1257,14 +1258,42 @@ class Console implements CaretListener,
HyperlinkListener, ComponentListener, Fo
}
}
- private Style createLinkStyle(int errorLine) {
- String scriptFileName = scriptFile?.name ?: DEFAULT_SCRIPT_NAME_START
- def style = hyperlinkStyle
+ /**
+ * Writes {@code line: N, column: M} as one source link, then a blank line.
+ * Coordinates are 1-based. A column below 1 is the first character of
that line.
+ */
+ private void appendErrorLocation(Document doc, int line, int column) {
+ insertString(doc, doc.length, "line: ${line}, column: ${column}",
linkAttributes(line, column))
Review Comment:
The docstring states coordinates are 1-based, but `column` is printed
verbatim even when it is 0/-1 (which the new behavior treats as ‘start of
line’). This makes the comment misleading and the displayed `column: M`
potentially inconsistent with where the caret/status bar ends up. Either (a)
normalize the displayed column (e.g., print `max(1, column)` while still
storing the original if needed), or (b) update the comment to clarify that the
reported column may be <= 0 to indicate unknown/absent and will be clamped for
caret placement.
##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/Console.groovy:
##########
@@ -2308,39 +2337,82 @@ class Console implements CaretListener,
HyperlinkListener, ComponentListener, Fo
inputEditor.redoAction.actionPerformed(evt)
}
- /** Highlights the source line referenced by an activated output
hyperlink. */
+ /**
+ * Moves the editor caret to the source position on an activated output
link.
+ * The position is the link element's attribute. The href is not
interpreted:
+ * mouse and keyboard activation both supply the character element, while a
+ * raw href cannot carry a column and still be a legal URL.
+ */
void hyperlinkUpdate(HyperlinkEvent e) {
- if (e.eventType == HyperlinkEvent.EventType.ACTIVATED) {
- // URL of the form: file://myscript.groovy:32
- String url = e.getURL()
- int lineNumber = url[(url.lastIndexOf(':') + 1)..-1].toInteger()
-
- def editor = inputEditor.textEditor
- def text = editor.text
-
- int newlineBefore = 0
- int newlineAfter = 0
- int currentLineNumber = 1
-
- // let's find the previous and next newline surrounding the
offending line
- int i = 0
- for (ch in text) {
- if (ch == '\n') {
- currentLineNumber++
- }
- if (currentLineNumber == lineNumber) {
- newlineBefore = i
- def nextNewline = text.indexOf('\n', i + 1)
- newlineAfter = nextNewline > -1 ? nextNewline :
text.length()
- break
- }
- i++
- }
+ if (e.eventType != HyperlinkEvent.EventType.ACTIVATED) {
+ return
+ }
+ def position = sourcePosition(e.sourceElement)
+ if (position == null) {
+ position = sourcePositionBefore(e.sourceElement)
+ }
+ if (position == null) {
+ return
+ }
+ revealSourcePosition(position)
+ }
- // highlight / select the whole line
- editor.setCaretPosition(newlineBefore)
- editor.moveCaretPosition(newlineAfter)
+ private static SourcePosition sourcePosition(Element element) {
+ def raw = element?.attributes?.getAttribute(LINK_POSITION)
+ raw instanceof SourcePosition ? raw : null
+ }
+
+ /**
+ * Keyboard activation treats a link's exclusive end offset as inside the
+ * link, then resolves the character element at that offset. That element
+ * is the following run, so the position is on the run before it.
+ */
+ private static SourcePosition sourcePositionBefore(Element element) {
+ if (element == null || element.startOffset == 0) {
+ return null
+ }
+
sourcePosition(element.document.getCharacterElement(element.startOffset - 1))
+ }
+
+ /**
+ * Puts the caret on a 1-based line and column, counting characters the
+ * same way the status bar does. Nothing is selected. A column below 1
+ * lands on the first character of the line, and a column past the end
+ * of the line stays on that line.
+ */
+ private void revealSourcePosition(SourcePosition position) {
+ def editor = inputArea
Review Comment:
This changes navigation to use `inputArea` instead of the prior
`inputEditor.textEditor`. If `inputArea` is not the actual editor component in
all runtime configurations (e.g., when `inputEditor` is the canonical editor
reference), link activation may stop moving the caret in production while still
passing these tests (which explicitly set `inputArea`). Consider resolving the
editor from the same source as before (e.g., `inputEditor.textEditor`) or
adding a fallback (`inputArea ?: inputEditor?.textEditor`) so hyperlink
navigation works consistently.
--
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]
