Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling proper code generation for comments (Comment with Line/Block Comment) #3582

Merged
merged 9 commits into from
Aug 3, 2024

Conversation

rNoz
Copy link
Contributor

@rNoz rNoz commented May 4, 2024

Enabling "context-aware" Comment Toggling (solving #2872)

What am I doing?

Improving comment-related actions, mainly focusing on preserving proper indentation and white-spaces when toggling line/block comments.

Why?

Back in mid 2022 I opened this issue, where I exposed what I would like regarding the action "Comment with Line Comment" and the like, as it is a recurrent operation I do everyday. Since the default behavior is really unproductive, I need to do a combination of toggling line comment, format region and sometimes even remove a bunch of white-spaces (between the # and the first character of the code being commented).

Approach

I originally thought I would have to work with some sort of Intellij AST, and create rules sophisticated enough to produce the expected behaviors based on where the cursor is located, what the code is like on the current line and the previous and next lines. But it seems that with the Commenter behavior you get the appropriate thing. I was unaware that this module is available, only that it is not enabled and cannot be manipulated via settings. I have found this out as I have fiddled with the code, exploring different directions to solve the problem.

Looking at the IDE configurations for other languages, I can see that the relevant options are in two regions under Editor > Code Style > WhateverLanguage:

  • Wrapping and Braces: Keep when reformatting
    • Comment at first column.
  • Code Generation: Comment Code:
    • Line comment at first column
    • Add a space at line comment start
      • Enforce on reformat
    • Block comment at first column
    • Add spaces around block comments

intellij-idea-java-code-style-settings-wrapping-and-braces

intellij-idea-java-code-style-settings-code-generation

These are the settings that I need to enable to have the expected behavior that supports proper indentation when toggling comments.

My approach involves creating a new panel (CodeGenerationPanel.kt) and adding settings and its manipulation via LanguageCodeStyleSettingsProvider.kt and ElixirCodeStyleMainPanel.kt. I haven't included a CodeSample because it is not present in any Code Generation panel that I have seen so far (for other languages).

Since there are two settings that are not available until a specific build version, I have included some logic to enable dynamically those if our IDE supports each of them.

LINE_COMMENT_ADD_SPACE_ON_REFORMAT' is available only since 221.5080.210 but the module is targeted for 201.6668.113+. It may lead to compatibility problems with IDEs prior to 221.5080.210. Note that this field might have had a different full signature in the previous IDEs. 0

'BLOCK_COMMENT_ADD_SPACE' is available only since 213.5744.223 but the module is targeted for 201.6668.113+. It may lead to compatibility problems with IDEs prior to 213.5744.223. Note that this field might have had a different full signature in the previous IDEs.

I decided not to include the option in Wrapping and Braces, since I haven't found a case where it is necessary to fix the original issue. But I will be happy to know when we need it.

Alternative approaches considered

The first part of the above section Approach is exactly the same. This alternative approach just differs in how to implement the Settings panel for Editor > Code Style > Code Generation.

I considered enabling the code generation panel via extension providers, with an independent XML entry. However, it is a bit slower, since it goes through all providers (the for loop), and since the current plugin is not using this feature for the MixFormatPanel, I am discarding it. Nevertheless, it seems it will be the way to go if we want to interact with other plugins or extensions.

Adding codeStyleSettingsProvider to resources/META-INF/plugin.xml:

<langCodeStyleSettingsProvider
    implementation="org.elixir_lang.formatter.settings.LanguageCodeStyleSettingsProvider"/>
<codeStyleSettingsProvider
    implementation="org.elixir_lang.formatter.settings.LanguageCodeGenerationSettingsProvider"/>
<externalFormatProcessor implementation="org.elixir_lang.formatter.MixFormatExternalFormatProcessor"/>

Creating the file src/org/elixir_lang/formatter/settings/LanguageCodeGenerationSettingsProvider.kt:

package org.elixir_lang.formatter.settings

import com.intellij.application.options.codeStyle.CommenterForm
import com.intellij.openapi.application.ApplicationBundle
import com.intellij.psi.codeStyle.CodeStyleConfigurable
import com.intellij.psi.codeStyle.CodeStyleSettings
import com.intellij.psi.codeStyle.CodeStyleSettingsProvider
import com.intellij.psi.codeStyle.DisplayPriority
import com.intellij.ui.IdeBorderFactory
import com.intellij.util.ui.JBInsets
import org.elixir_lang.ElixirLanguage
import javax.swing.BoxLayout
import javax.swing.JComponent
import javax.swing.JPanel

class LanguageCodeGenerationSettingsProvider : CodeStyleSettingsProvider() {

    override fun createConfigurable(settings: CodeStyleSettings, modelSettings: CodeStyleSettings): CodeStyleConfigurable {
        return LanguageCodeGenerationConfigurable(settings)
    }

    override fun getConfigurableDisplayName(): String = ApplicationBundle.message("title.code.generation")
    override fun getPriority(): DisplayPriority = DisplayPriority.CODE_SETTINGS
    override fun hasSettingsPage() = false
    override fun getLanguage(): ElixirLanguage = ElixirLanguage
}

class LanguageCodeGenerationConfigurable(private val mySettings: CodeStyleSettings) : CodeStyleConfigurable {

    private val myCommenterForm: CommenterForm = CommenterForm(ElixirLanguage)

    override fun getDisplayName(): String = "Code Generation"

    override fun createComponent(): JComponent {
        return JPanel().apply {
            layout = BoxLayout(this, BoxLayout.Y_AXIS)
            border = IdeBorderFactory.createEmptyBorder(JBInsets(0, 10, 10, 10))
            add(myCommenterForm.commenterPanel)
        }
    }

    override fun isModified(): Boolean {
        return myCommenterForm.isModified(mySettings)
    }

    override fun apply() {
        apply(mySettings)
    }

    override fun reset() {
        reset(mySettings)
    }

    override fun reset(settings: CodeStyleSettings) {
        myCommenterForm.reset(settings)
    }

    override fun apply(settings: CodeStyleSettings) {
        myCommenterForm.apply(settings)
    }
}

Modifying src/org/elixir_lang/formatter/settings/ElixirCodeStyleMainPanel.kt:
`

package org.elixir_lang.formatter.settings

import com.intellij.application.options.TabbedLanguageCodeStylePanel
import com.intellij.openapi.extensions.Extensions
import com.intellij.psi.codeStyle.CodeStyleSettings
import com.intellij.psi.codeStyle.CodeStyleSettingsProvider
import org.elixir_lang.ElixirLanguage

class ElixirCodeStyleMainPanel(currentSettings: CodeStyleSettings?, baseSettings: CodeStyleSettings) :
    TabbedLanguageCodeStylePanel(ElixirLanguage, currentSettings, baseSettings) {

    override fun initTabs(settings: CodeStyleSettings) {
        addTab(MixFormatPanel(settings))
        addIndentOptionsTab(settings)
        addSpacesTab(settings)
        addWrappingAndBracesTab(settings)
        // getExtensions (deprecated) should be updated to getExtensionList
        for (provider in Extensions.getExtensions(CodeStyleSettingsProvider.EXTENSION_POINT_NAME)) {
             if (provider.language == ElixirLanguage && !provider.hasSettingsPage()) {
                 createTab(provider)
             }
        }
    }
}

Testing

I haven't included any tests. However, I am using it in 3 WebStorm IDE and OS versions:

  • 2024.1 Linux
  • 2023.3 MacOS
  • 2024.1 MacOS

For the 2024.1 versions I require the changes of this other PR as well.

Some screenshots of the behavior provided by this PR. Also, considering the use cases exposed in the original issue.

By default, we provide this new panel with these options:
intellij-idea-runIde-elixir-code-style-settings-code-generation-default-opts

Then, we can modify these settings to support proper indentation:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts

Let's consider these use cases:

Before:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-before1
After:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-after1

Before:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-before2

After:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-after2

And considering something closer in indentation as I exposed back then for JS:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-before3

Before:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-before3-1

After (cursor is in the highlighted row):
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-after3-1

Before:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-before3-2

After:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-snippet-after3-2

So, it is aware of the context.

Finally, comparing mix format with the IDE, manipulating it directly by doing: Toggle Line Comment + Indent + Toggle Line Comment on every line.

Having a wrongly formatted file:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-wrong-comment-manual-indents

The produced code is the same with mix format than with the manual operations in the IDE:
intellij-idea-runIde-elixir-code-style-settings-code-generation-indentation-opts-wrong-comment-manual-indents-fixed-by-mix-format

Disclaimer

This is my first time working with Intellij SDK, Kotlin, intellij-elixir and the like. So, feel free to throw any advice, refactor suggestion or alternative strategy for this PR. For instance, one of the latest changes I have done before pushing the PR, is changing the companion object of CodeGenerationPanel. I didn't like that the logic behind CommenterForm was in 2 files (CodeGenerationPanel and LanguageCodeStyleSettingsProvider). So, I removed the logic in customizeCodeGenerationSettings to just call a static function present in CodeGenerationPanel, to return the valid options for the running IDE build.

I'll be happy to do any requested changes, as I have done it "in record time", studying the minimum necessary and using a pragmatic approach, because I was tired of waiting to include this functionality. I have done a quick exploration of other intellij extensions, made both in Scala and Java, and trying to extrapolate the ways to manipulate panels and extend language-styles capabilities, but trying to conform to the intellij-elixir code structure (also in Kotlin).

@joshuataylor
Copy link
Collaborator

Thank you so much for this, I will look at this right after the 2024.1/2 push. 😍

@joshuataylor
Copy link
Collaborator

This looks great, fantastic work and I absolutely love the thoroughness in your description.

A requirement of this repository is that all commits to be signed.

see here

If you could do this, we can push it in with 18.0.0, otherwise it'll be 18.0.1 -- no rush.

@rNoz rNoz force-pushed the rnoz/commenter-improvements branch 2 times, most recently from 85da1ba to a4d1e13 Compare July 16, 2024 05:31
@rNoz
Copy link
Contributor Author

rNoz commented Jul 16, 2024

Thank you for your words. Please, push again your changes here as somehow trying to sign my first commit got a weird scenario. I don't know why it said "merge conflicts" when just trying to sign that commit (no code changes, no edit commit message).

I copied the changes, just in case, but somehow they don't contain all commits.

git checkout -b rnoz/commenter-improvements-before-signing origin/rnoz/commenter-improvements
git push origin rnoz/commenter-improvements-before-signing:rnoz/commenter-improvements

So I am probably tired, but definitely couldn't sign without merge conflicts. Amending the commit e0bee82 got me in merge conflicts with CodeGenerationPanel.kt, specifically changes you have done later (why?). I tried in 2 ways, pasting my original file, or pasting your changes, and in the end this got weird. So I prefer to step back and wait for your advice.

rNoz and others added 8 commits July 27, 2024 09:25
* Update gradlewrapper to v7.6.4
./gradlew wrapper --gradle-version=7.6.4
To fix gradle issue: gradle/gradle#27156

* Remove use of `FileUtil.FILE_HASHING_STRATEGY` from Intellij FileUtil, it was removed in 2024.1. (Note, this is only used in the jps-build test suite).
This also removes references to Trove THashSet, and no longer stores File directly in sets or collections.
See JetBrains/intellij-community@560e8fc

* Add support for 2024.1 IDEs (and runs tests correctly against 2024.1)

* Update usages group wording for 2024.1

* Fix more sdk configuration commits in application RW thread, fixes compatibility with IDEs v2024.1

* Fix whitespace in tests due to 2024.1 change

I believe 2024.1 changed how the Usages work.

In 2023.x:
```kotlin
val usages =
myFixture.testFindUsagesUsingAction("module_attribute_usage.ex",
"kernel.ex")
.map { it as UsageInfo2UsageAdapter }
```

In the debugger shows:
```
0 = {ReadWriteAccessUsageInfo2UsageAdapter@19239} "4|def usage, |do|:
|@|module_attribute"
1 = {ReadWriteAccessUsageInfo2UsageAdapter@19240} "2|@|module_attribute|
|1"
```

For 2024.1, this shows:
```
0 = {ReadWriteAccessUsageInfo2UsageAdapter@19421} "2|
|@module_attribute| 1"
1 = {ReadWriteAccessUsageInfo2UsageAdapter@19422} "4|  def usage, do:
|@module_attribute"
```

I believe it not shows the whitespace for the file, where previously it
didn't.

* pin versions

* use BasePlatformTestCase to stop warning

* add do block match test

* set 241.0 as the version, to fix certain intellij warnings

* fix key

* revert tests

* use thread

---------

Co-authored-by: Josh Taylor <[email protected]>
@rNoz rNoz force-pushed the rnoz/commenter-improvements branch from a4d1e13 to f624052 Compare July 27, 2024 07:29
@rNoz
Copy link
Contributor Author

rNoz commented Jul 27, 2024

@joshuataylor I gave another try and signed the commits. Since it seems it rewrites history, you might need to re-sign the following ones.

I checked the code changes for the final 3 files we wanted to integrate, is the same as before signing (CodeGenerationPanel, ElixirCodeStyleMainPanel and LanguageCodeStyleSettingsProvider). However, it tracks more changes now. Nevertheless, in rnoz/commenter-improvements-before-signing you have the original branch before singing.

@joshuataylor
Copy link
Collaborator

Sorry, missed the last comment - I'll get this in ASAP for 18.0.1

@joshuataylor joshuataylor merged commit 4822fa5 into KronicDeth:main Aug 3, 2024
2 checks passed
@joshuataylor
Copy link
Collaborator

Thank you so much for your hard work on this!

When I get some time, I'll add these to the documentation.

I've released v18.0.1 with these changes.

❤️

https://github.com/KronicDeth/intellij-elixir/releases/tag/v18.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants