[
https://issues.apache.org/jira/browse/GROOVY-12429?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18125063#comment-18125063
]
ASF GitHub Bot commented on GROOVY-12429:
-----------------------------------------
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.
> GroovyConsole error link does not place the caret on the reported column
> ------------------------------------------------------------------------
>
> Key: GROOVY-12429
> URL: https://issues.apache.org/jira/browse/GROOVY-12429
> Project: Groovy
> Issue Type: Bug
> Reporter: Daniel Sun
> Priority: Major
>
> h2. Problem
> A compilation error in GroovyConsole prints a location such as {{line: 1,
> column: 9}}. Activating that link selects the whole line and leaves the caret
> after the last character. The status bar shows the end of the line, not the
> column named in the message, so the link does not show where the error is.
> On a later line the selection also starts at the newline that ends the
> previous line.
> h2. Steps to reproduce
> Run this script in GroovyConsole. The opening quote is not closed.
> {noformat}
> println '123
> {noformat}
> In the output pane, activate {{line: 1, column: 9}}.
> h2. Actual result
> The whole line is selected. The script is 12 characters long, the caret is
> left after the last character, and the status bar reads {{1:13}}.
> h2. Expected result
> The caret is on the opening quote, that character is selected, and the status
> bar reads {{1:9}}.
> When an error spans more than one character, the span is selected and the
> caret stays on the column named in the message. The status bar uses the same
> character columns as that message.
> h2. Other cases covered by the same defect
> * A location on a later line starts at the first character of that line.
> * A stack-trace link that names only a line still selects that whole line,
> from its first character. The status bar shows column 1 of that line.
> * A column past the end of the line stays on that line.
> * Activating the link from the keyboard moves the caret to the same place as
> a mouse click.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
