Program Structure Interface (PSI) implementation and HTML template editing support

myrjola commented 9 months ago

Overview of changes

These changes introduce HTML editing support for Templ files based on Grammar-Kit. I could not yet get Go and HTML to work together. Using the TemplateLanguageFileViewProvider, it's possible to get one language working nicely but adding support for other languages in the same file probably requires implementing language injection. I elected to support HTML before Go because there seems to be all kinds of magic done by TemplateLanguageFileViewProvider to support other plugins such as Tailwind autocomplete.

Open questions and calls for help

Lexer and parser

Reconsider if the naive lexer and parser is good enough. I wanted something quick-and-dirty to demonstrate how this all could work. I'm not deeply familiar with lexers and parsers. Ideally, we would follow closely how the Templ parser works. There's bound to be many bugs in the current parsing implementation. Does anyone have any pointers what's the best way to parse Templ in Jetbrains context?

The current implementation is relying on a JFlex lexer generated from src/main/grammar/_TemplLexer.flex that outputs very coarse tokens such as HTML and Go fragments that are then parsed using the Grammar Kit parser which is generated from src/main/grammar/Templ.bnf. The big drawback I see is that the lexer does not even try to understand Go or HTML. Instead, it recognizes a couple of keywords such as templ, {, } and outputs the lexical tokens based on that. Is this approach valid or should we reconsider another approach?

What's the minimal increments that could be merged to the Templ plugin?

What shape should this PR be in before being considered for merging? I think the aim is that it would only enhance the plugin and not break anything. LSP support seems to be working but I had to disable some other functionality in plugin.xml because I was getting crashes.

Another approach would be to split this PR into parts and keep working on it in a separate branch from main until it's of high enough quality. What do you think?

TODOs

[x] Fix exceptions such as Empty PSI elements must not be passed to createDescriptor. Start: PsiElement(XML_ATTRIBUTE_VALUE), end: PsiElement(XML_ATTRIBUTE_VALUE)
[x] Re-enable disabled features in plugin.xml with the aim of not breaking any existing plugin functionality
[x] Clean up code, add documentation etc.
[x] Increase test coverage once we're happy with the overall approach.

Testing notes

"Run Plugin" Run Configuration opens a sandboxed IDE with the plugin loaded. When you open a .templ file, you should notice the html parts work similarly to e.g. .gohtml files with the Go Template plugin. If you notice issues, you can troubleshoot the parser with the IDE action "View PSI Structure of Current File..." (see screenshot below). If you need to troubleshoot the Lexer, add more examples to the TemplLexerTest and check the resulting output.

Future work

Implementing Go language injection would be great for enhancing the editing experience further. The official docs are quite light, so probably best is to check usage of MultiHostInjector in the official open sourced plugins. Currently, this pull request contains TemplInjector but it's not yet working properly.

Relevant resources

Related issues

Fixes both #8 and #11.

Screenshots

Tailwind autocomplete demonstrates the IDE understands we're editing an HTML template.

PSI Viewer

alexisvisco commented 9 months ago

Woaw ! Maybe we should reach an author of a plugin to help you understand the strategy you need for getting go autocompletion well ?

joerdav commented 9 months ago

Oh wow! Stellar work! Thankyou for dedicating your time to this and being so thorough in your description!!

I'm fully supportive of this rewrite, and I agree that this should really only be merged to main once it's on par with the current implementation at least.

I'm happy to support in anyway I can but I think I'll need to take some time to understand what's been done, so please be patient with me on that!

Thanks again for this fantastic contribution.

myrjola commented 8 months ago

I'm happy to support in anyway I can but I think I'll need to take some time to understand what's been done, so please be patient with me on that!

Thanks! Take your time. Added a couple of fixes to the parsing. I'm hoping most of Templ syntax is now covered after adding support for css and script declarations. Next step is to test against complex templ files that they parse correctly. If we're happy with the naive parsing approach, I can start adding proper test coverage for the Lexer and Parser.

It's looking quite good with maintaining existing functionality. I have enabled everything except the spell checker in plugin.xml. Next lead to follow is to check the spell checking tutorial. These lines are currently commented out until I get the crash fixed.

<spellchecker.support language="templ" />
<spellchecker.bundledDictionaryProvider implementation="com.templ.templ.spellchecker.TemplBundledDictionaryProvider"/>

Stacktrace:

com.intellij.diagnostic.PluginException: implementation class is not specified [Plugin: com.templ.templ]
    at com.intellij.serviceContainer.LazyExtensionInstance.createInstance(LazyExtensionInstance.java:53)
    at com.intellij.serviceContainer.LazyExtensionInstance.getInstance(LazyExtensionInstance.java:44)
    at com.intellij.serviceContainer.BaseKeyedLazyInstance.getInstance(BaseKeyedLazyInstance.java:38)
    at com.intellij.openapi.util.KeyedExtensionCollector.instantiate(KeyedExtensionCollector.java:177)
    at com.intellij.openapi.util.KeyedExtensionCollector.buildExtensionsFromExtensionPoint(KeyedExtensionCollector.java:164)
    at com.intellij.openapi.util.KeyedExtensionCollector.buildExtensions(KeyedExtensionCollector.java:139)
    at com.intellij.lang.LanguageExtension.buildExtensions(LanguageExtension.java:146)
    at com.intellij.lang.LanguageExtension.buildExtensions(LanguageExtension.java:16)
    at com.intellij.openapi.util.KeyedExtensionCollector.forKey(KeyedExtensionCollector.java:111)
    at com.intellij.lang.LanguageExtension.collectAllForLanguage(LanguageExtension.java:122)
    at com.intellij.lang.LanguageExtension.allForLanguage(LanguageExtension.java:114)
    at com.intellij.spellchecker.inspections.SpellCheckingInspection.getSpellcheckingStrategy(SpellCheckingInspection.java:47)

myrjola commented 8 months ago

Spell checking is no longer crashing after introducing the default spell check strategy. It works inside HTML context but not yet elsewhere. This would require some plumbing by introducing our own strategy by following the tutorial. I propose to postpone that after Go language injection is added in a later pull request since that should already cover spell checking Go code.

joerdav commented 8 months ago

This looks to be shaping up! So in terms of TODOs before a merge what are your plans? I get the feeling we are okay to merge without language injection for now since the LSP functionality is still enabled?

myrjola commented 8 months ago

I cleaned up the code and added documentation and basic test coverage for the lexer and parser. The test suite is green.

Before merging, I would prefer adding more test coverage. It would also be good to get more hours of real-world usage to see if the plugin crashes. Any complex open-source Templ codebases are good candidates for this and anyone lurking in this thread is more than welcome to take this for a spin ❤️

I don't foresee any large changes coming in other than adding test coverage and fixing bugs. Therefore, now would be a good time to review.

nathan-isaac commented 8 months ago

How can I test it? I'd be happy to upload the plugin manually and use it.

myrjola commented 8 months ago

🐛 one ugly bug surfaced when checking the Templ generator tests. It highlights the drawbacks of a naive parser.

Reproduction in this file

<button
    @click="count++"
>Increment</button>

The @click should not be parsed as a component import because it's an HTML attribute. This made me notice lacking handling of comments inside HTML. Templ supports both classic HTML comments  and Go comments /* */. Given the following snippet:

<!--
  @click
-->
/*
  @click
*/
<button
     @click="count++"
>Increment</button>

Templ outputs the following HTML, note that the Go comment has been removed:

<button @click=\"count++\">Increment</button>

The current lexer detects a component import on every line that starts with @. To fix this, I'm planning the following Lexer improvements:

Handle Go comments globally, i.e., output a COMMENT token for all comments encountered so that they don't get parsed as Templ keywords.
Inside HTML, it needs to be aware when it's okay to detect Templ keywords such as @, if and so on. For example, @ keyword is not recognised inside html open tags and comments. Unfortunately, this increases Lexer complexity significantly because we're adding HTML parser to it 😅

For AlpineJS, the workaround is to use x-on:click instead of @click. Regardless, I doubt it's acceptable to merge before this bug is fixed.

myrjola commented 8 months ago

Thanks @nathan-isaac. To test, you can follow this guide to install the following plugin Templ-0.0.13.zip

If you want to build it yourself, checkout my branch and run the assemble Gradle task. It should build the same zip file to the build/distributions-folder.

nmcbride commented 8 months ago

I'm going to see if I can get this working and test it and help out in some way. I just do not know Kotlin.

Would it be possible, if we are re-writing the plugin to include some of the options that are in the VSCode plugin? Especially the one where you can configure a log?

nmcbride commented 8 months ago

When I run goland from the command line with the plugin (and the current released version), and it sits for a bit, I see the following:

2024-02-25 17:59:59,208 [ 466750]   WARN - #c.i.e.p.BaseOSProcessHandler - Process hasn't generated any output for a long time.
If it's a long-running mostly idle daemon process, consider overriding OSProcessHandler#readerOptions with 'BaseOutputReader.Options.forMostlySilentProcess()' to reduce CPU usage.
Command line: /var/home/nmcbride/go/bin/templ lsp
java.lang.Throwable: Process creation:
    at com.intellij.execution.process.BaseOSProcessHandler.<init>(BaseOSProcessHandler.java:32)
    at com.intellij.execution.process.OSProcessHandler.<init>(OSProcessHandler.java:44)
    at com.intellij.platform.lsp.api.LspServerDescriptor.startServerProcess(LspServerDescriptor.kt:95)
    at com.intellij.platform.lsp.impl.connector.Lsp4jServerConnectorStdio.<init>(Lsp4jServerConnectorStdio.java:19)
    at com.intellij.platform.lsp.impl.LspServerImpl.N(LspServerImpl.java:383)
    at com.intellij.openapi.application.impl.ApplicationImpl$2.run(ApplicationImpl.java:249)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:702)
    at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:699)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
    at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1.run(Executors.java:699)
    at java.base/java.lang.Thread.run(Thread.java:840)

nmcbride commented 8 months ago

@myrjola I do not seem to be getting CSS completion insidie of the template like you showed in your image. Is there something else that needs to be done other than installing the plugin from the zip?

proper-gentleman commented 8 months ago

nice work @myrjola , emmet is working very well and css completion seems to work, but only on outputted .css files, not for tailwind classes - maybe that is some misconfiguration on my part

joerdav commented 8 months ago

@nmcbride I wouldn't recommend adding those fields as a part of this PR, I think that can be handled separately.

myrjola commented 8 months ago

@proper-gentleman

css completion seems to work, but only on outputted .css files, not for tailwind classes - maybe that is some misconfiguration on my part

The bundled Tailwind plugin can be a bit tricky. There's an open bug for making it work with the standalone Tailwind CLI. I got it working by adding Node.js and following the official docs.

@nmcbride

I do not seem to be getting CSS completion insidie of the template like you showed in your image. Is there something else that needs to be done other than installing the plugin from the zip?

You might need to register directories containing your .css files as resource roots. Relevant Goland and Intellij docs. I'm using Intellij and noticed I had to nest the Resource root inside a Sources root to make it work.

nmcbride commented 8 months ago

@proper-gentleman

I am having that exact issue as well. I realized I'd be running into the nodejs issue early on when I started using the standalone CLI but all the UI kits required nodejs anyway. So for me goland already has nodejs detected correctly but it is still not working.

myrjola commented 8 months ago

Fixed the parsing bugs reported in https://github.com/templ-go/templ-jetbrains/pull/17#issuecomment-1963062584

The tests are green again and I didn't spot any obvious parsing bugs when going through the generator tests in the Templ repository.

I was able simplify some parts of the lexer by extending MergingLexerAdapter that collapses duplicate consecutive tokens into one. Unfortunately, adding richer Go and HTML parsing (strings, comments, and opening tags) did increase the overall complexity.

For those running into issues with Tailwind and other plugins, you could test if they work on vanilla HTML files. I'm interested to hear if you encounter any plugins that work on HTML files break against the Templ plugin.

For Tailwind specifically, I realised I had added templ in includeLanguages configuration. Hopefully that's the missing piece for @nmcbride and @proper-gentleman.

joerdav commented 8 months ago

@myrjola This seems to be progressing lots each time I look thanks for all the work!

One thing I've noticed is not a regression in what you've built, but there has been some recent changes that may affect the parser you have written.

Namely, there is more support in templ for multiline go snippets, we've managed to get this working in vscode driven by @alehechka

What I'm taking about are snippets such as multi-line arg functions (for templ, css and script components):

templ headerTemplate(
    name string,
    ) {
    <header data-testid="headerTemplate">
        <h1>{ name }</h1>
    </header>
}

As, well as things like templ element calls:

<body>
    @myviewmodel{
              data: data,
    }.View()
</body>

This does currently highlight incorrectly in the jetbrains plugin, but thought it may be worth thinking about (not necessarily in this PR)

nmcbride commented 8 months ago

@myrjola Thank you I will give that a shot. Are we still using the above .zip?

nmcbride commented 8 months ago

No luck so far. Even after the mapping.

No matter what I cannot get CSS to auto complete outside of what is pulled directly into the the base template via Githubissues.

Githubissues is a development platform for aggregating issues.

templ-go / templ-jetbrains