[ 
https://issues.apache.org/jira/browse/GROOVY-12448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18131649#comment-18131649
 ] 

ASF GitHub Bot commented on GROOVY-12448:
-----------------------------------------

paulk-asert commented on PR #2976:
URL: https://github.com/apache/groovy/pull/2976#issuecomment-6392693261

   Thanks Daniel. I checked the new head (5aa82c4422) against each item, 
running the missing-jar cases for real rather than only through the tests.
   
   Verified fixed:
   - groovysh without groovy-lsp/LSP4J/Gson on the classpath: one warning, Tab 
falls back to the shell completer, and the shell exits normally.
   - noSyntaxCheck set in groovysh_init.groovy is now honoured. (Side effect 
worth a line in groovysh.adoc: onReaderReady now runs after the init file 
rather than before it.)
   - Highlighting: the shift-by-delta plus the sorted merge is exactly what I 
was after.
   - @ASTTest: override && property is the right rule, and the THREAT_MODEL row 
is updated.
   - close() on the EDT returns right away, with the release done on the reaper 
thread.
   - All the should-fixes from my last comment.
   
   One item left, the console half of the missing-jar fallback. 
ConsoleTextEditor now falls back to GroovyFilter correctly, but 
Console.installLanguageHost() (Console.groovy:625-633) has no guard, and 
ConsoleLanguageHost's constructor calls GroovyLanguageClient.open. Compiler 
Assistance is on by default, so with groovy-lsp absent, Console.main throws 
NoClassDefFoundError right after the frame is shown. The script given on the 
command line is never loaded, and the stack trace goes into the console's own 
output pane (the interceptor is already installed), so the user just sees an 
empty editor. The same unguarded call is reached from the new-window path 
(:1168) and the Compiler Assistance toggle (:2364). The current tests simulate 
only the editor-filter path.
   
   Suggested fix: catch LinkageError in installLanguageHost, log the same 
one-line warning, and leave languageHost null. Ideally also disable the 
Compiler Assistance menu item, since it can't do anything in that case. Then 
add a test that uses a throwing seam for attach, in the same way as the filter 
test.
   
   Two nits:
   - ConsoleLanguageHost.groovy:195 has an orphaned Javadoc block ("Compiles 
text immediately and returns diagnostics") with no method under it.
   - ASTTestTransformation.currentThreadEnabled/setThreadEnabled are public, 
since that's the Groovy default. As they're tooling-only, would @Internal be 
worth adding?
   
   With the installLanguageHost guard in, I'm happy for this to go into master 
for 7.0.




> Wire Groovy Console and groovysh to the in-process language engine
> ------------------------------------------------------------------
>
>                 Key: GROOVY-12448
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12448
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Daniel Sun
>            Priority: Major
>
> h2. Context
> GROOVY-12411 added groovy-lsp and the in-process GroovyLanguageEngine. Groovy 
> Console and groovysh were left unconnected. This change attaches both hosts 
> to that engine. Diagnostics and completion for an unsaved buffer do not 
> generate class files.
> Scripts still run through GroovyShell in the console and through GroovyEngine 
> in groovysh.
> h2. User-visible behavior
> * *Groovy Console.* The editor compiles the unsaved buffer in the background 
> and highlights compiler diagnostics. {{Alt+/}} offers compiler-backed 
> completions. {{Ctrl+Space}} is left free, because that chord switches the 
> input method on Chinese desktops and on macOS. {{Ctrl+/}} is still comment 
> and uncomment. A multiline string keeps the color of its opening delimiter: 
> single quote, double quote, or slashy. The dollar sign that opens a GString 
> expression is not colored as part of the string. A jar or directory added 
> from the Script menu is picked up on the next background compile. The compile 
> uses the console compiler configuration, including imports and AST 
> customizers. Turning Allow interruption on or off edits that configuration, 
> and the next compile sees the edit.
> * *groovysh.* Tab completion and tail tips consult the compiler for the 
> current buffer: imports, types, methods, and binding variables. A syntax tail 
> tip reports diagnostics and does not evaluate the line. A method tail tip 
> asks for signatures on the full line, so an unclosed opening parenthesis is 
> visible, and uses the previous inspector only when the compiler has none. 
> {{/grab}} is still how Maven artifacts are added. The next completion or tail 
> tip notices the loader change. Customizers on the compiler configuration 
> given to the engine at construction apply to completion and tail tips as well 
> as to evaluation. An import typed at the prompt is buffer text, so the next 
> completion sees it as part of the script.
> h2. Design
> * groovy-console and groovy-groovysh depend on groovy-lsp with an 
> {{implementation}} dependency, not an {{api}} dependency. LSP4J is not part 
> of either module's published API. groovy-lsp stays an optional module and is 
> not listed directly by groovy-all. Consumers of the console or groovysh still 
> receive it transitively.
> * Each host keeps its own adapter and talks to the engine with JDK records. 
> Host source does not import LSP4J. The console adapter owns the buffer name 
> {{groovy-buffer:console}}. The groovysh adapter owns {{groovy-buffer:repl}}. 
> The engine does not assign those names. Both adapters are internal 
> implementation details, not an embedding API.
> * Positions are 0-based UTF-16. CR, LF, and CRLF are line breaks. A character 
> past the end of a line stays on that line. A zero-width diagnostic at the end 
> of the text is painted on the last character.
> * A query that is given the buffer text replaces that text first. Identical 
> text keeps the document version, so the snapshot can be reused. Publishing 
> the host compiler configuration copies the script base class and the 
> compilation customizers into the tooling compile, and drops the cached 
> compile on every call, including a repeat of the same instance. Hosts do that 
> when the configuration is replaced or edited, not before every query.
> * The console publishes its configuration when the host is created and again 
> when the host rebinds. Rebinding covers classpath changes and the Allow 
> interruption toggle. Later queries publish the parent loader only.
> * groovysh publishes the engine configuration at construction, and again only 
> when the configuration instance itself changes. That field is final. 
> Interactive imports are buffer snippets, not edits to an import customizer.
> * The compile fingerprint includes the URLs of a URLClassLoader, so an 
> in-place addURL is visible on the next compile.
> * groovysh builds one script for each question: the remembered buffer, a 
> declaration for each binding name the buffer does not already introduce, then 
> the current line. Those names come from the compiled AST. The walk does not 
> enter a closure or a lambda. It skips the generated no-arg run method and a 
> static main method whose only parameter is an array.
> * Console coloring uses one lenient lexer walk on the editor thread. It does 
> not paint the compiled name-kind overlay, because those kinds share the 
> identifier style. Brace matching still reads that lexer token list. groovysh 
> keeps its existing nanorc highlighter.
> * Snippet placeholders are removed from completion text before it is 
> inserted. groovysh keeps only the first line of that text.
> h2. Out of scope
> * Driving the AST browser from a language-engine snapshot.
> * A public embedding API for either adapter.
> * A separate engine jar that does not contain LSP4J. Feature services inside 
> groovy-lsp still use those types, so LSP4J remains a runtime dependency of 
> both hosts.
> * Noticing an in-place edit of a customizer that groovysh has already 
> published. The shell does not edit that object after construction. The 
> console does, and it publishes the configuration again after the edit.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

Reply via email to