User: joe Date: 11 Nov 26 02:43 Revision: 31ff07e672a89f38cf405960582f13213d9af620 Summary: Apply review TeamCity URL: http://ci.mcdev.io:80/viewModification.html?tab=vcsModificationFiles&modId=10707&personal=false The following binary files were skipped: src/main/resources/assets/icons/general/Annotater.png src/main/resources/assets/icons/general/Annotater@2x.png src/main/resources/assets/icons/general/AnnotaterLarge.png src/main/resources/assets/icons/general/AnnotaterLarge@2x.png src/main/resources/assets/icons/general/tiny_potato.png src/main/resources/assets/icons/general/tiny_potato@2x.png src/main/resources/assets/icons/general/tiny_potato_large.png src/main/resources/assets/icons/general/tiny_potato_large@2x.png Index: annotater-review.md =================================================================== --- annotater-review.md (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ annotater-review.md (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) @@ -1,28 +0,0 @@ -- Add a way to delete a contribution (with an "are you sure" dialog). Only delete it locally, don't touch the remotes or any pull request. -- Add a checkbox in the pull request dialog (on by default) to check the build before creating the pull request. This is done by running `./gradlew check` in the annotater directory. This can take a minute or so to run, so a progress dialog is required. If the check fails, the user should be presented with a dialog containing the failure log, with options to cancel (default) or submit anyway. -- If a pull request has already been created for a contribution, the "create pull request" button should be replaced with an "update pull request" button, which should only appear if there are modifications to be pushed (just as "create pull request" should). -- When you open a pull request, why do you browse to it and then add a button to browse to it? I would just leave it at the button. -- `AnnotaterJavadocCommentInspection.findMarkdownMigrationFixes` should instead use reflection to construct the fix. Put the reference to the constructor in `McdevReflection`. -- The method names in `AnnotaterJavadocContributions`: `switchTo`, `checkUncommittedChanges`, and `switch` aren't clear enough, especially `checkUncommittedChanges` doesn't really describe that it does the switch afterwards. -- The `DocumentationProvider` docs recommend migrating to `DocumentationTarget`. Evaluate whether we can do that for `AnnotaterJavadocDocumentationProvider`, and if we can, do it. -- `AnnotaterJavadocEditing.edit` can be renamed to an overload of `open` (can therefore move the methods into a more logical order). -- `AnnotaterJavadocEditing.openEditor` uses `ReadAction.compute`, which is deprecated. -- Rename the icon to `tiny_potato`. -- What is `AnnotaterJavadocInlayPassFactory.Pass.active` for? If it's a workaround for something, a comment would be nice. -- Companion objects inside extension point impl classes sometimes generate an intellij warning. Where these appear, change them to named objects, as we do in the rest of the project. -- Storing the inlay for `offset` in `AnnotaterJavadocInlayRenderer` looks like a bit of a hack. Is there precedent for this in IntelliJ's codebase? Why can't `initialOffset` always be used? -- `AnnotaterJavadocInlayRenderer` attempts to get proper Javadoc highlighting colors with the token thing, and while it does properly turn the text green, it doesn't highlight other things, like turning the color of referenced parameters. Fix this if it's an easy fix, otherwise tell me how you would fix it and wait for my approval before fixing it. -- `AnnotaterJavadocInlineEditor` uses `Alarm` which is obsolete. Follow the advice in the `Alarm` docs to replace it, if possible. If that's kinda complicated, then not saving until editing is finished is acceptable. -- Since it now should be difficult to accidentally edit readonly sections, the `ReadonlyFragmentModificationHandler` can be removed. -- The document modification guards should be not greedy at the end close to the edited javadoc, and greedy on the other end. -- In `AnnotaterJavadocInlineEditor.dispose`, it disposes the inlay directly. Isn't the normal solution to this to register the inlay with `Disposer.register` instead? -- Why does `AnnotaterJavadocInlineEditor.ProjectDisposable` exist, why not register the inline editor disposable directly with the project (which is itself disposable)? -- In `AnnotaterJavadocRefresher`, use `BulkFileListenerBackgroundable` instead. -- Why is `AnnotaterJavadocRendering.getJavadoc` in that class? I feel like that would fit better in another class. -- Make it so `AnnotaterJavadocSourceCopy.createVirtualFile` uses the language level of the file it's copied from, or Java 23 (the version Markdown Javadocs were introduced), whichever is newer. Same in `AnnotaterJavadocVirtualFile`. -- Why do we have both `AnnotaterJavadocSourceCopy.createVirtualFile` and `AnnotaterJavadocVirtualFile`? -- Local classes should be possible to document. Members within local and anonymous classes should also be possible to document. -- There's a lot of methods throughout this feature that are public but could be made private. -- The `EnigmaFileWriter` `VisitOrder` doesn't match what is used by annotater. Also you should update `mapping-io` in mcdev to the latest version. -- Fix the lint error on `EditAnnotaterJavadocIntention`. -- Find a built-in or library equivalent to `AnnotaterConfig.validatePath`, rather than writing it yourself. \ No newline at end of file Index: gradle/libs.versions.toml =================================================================== --- gradle/libs.versions.toml (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ gradle/libs.versions.toml (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -39,7 +39,7 @@ coroutines-swing = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-swing", version.ref = "coroutines" } -mappingIo = "net.fabricmc:mapping-io:0.8.0" +mappingIo = "net.fabricmc:mapping-io:0.9.2" mixinExtras-expressions = "io.github.llamalad7:mixinextras-expressions:0.0.6" jgraphx = "com.github.vlsi.mxgraph:jgraphx:4.2.2" Index: src/main/kotlin/asset/GeneralAssets.kt =================================================================== --- src/main/kotlin/asset/GeneralAssets.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/asset/GeneralAssets.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -26,6 +26,6 @@ val MC_TEMPLATE = loadIcon("/assets/icons/general/MinecraftTemplate.png") - val ANNOTATER = loadIcon("/assets/icons/general/Annotater.png") - val ANNOTATER_LARGE = loadIcon("/assets/icons/general/AnnotaterLarge.png") + val TINY_POTATO = loadIcon("/assets/icons/general/tiny_potato.png") + val TINY_POTATO_LARGE = loadIcon("/assets/icons/general/tiny_potato_large.png") } Index: src/main/kotlin/platform/fabric/annotater/AnnotaterCheckout.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/AnnotaterCheckout.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/AnnotaterCheckout.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -34,5 +34,5 @@ val mappingsDir: Path = root.resolve(config.mappings) - val javadocStore = AnnotaterJavadocStore(mappingsDir) + val javadocStore = AnnotaterJavadocStore.forDirectory(mappingsDir) } Index: src/main/kotlin/platform/fabric/annotater/AnnotaterCheckoutService.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/AnnotaterCheckoutService.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/AnnotaterCheckoutService.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -22,6 +22,7 @@ import com.demonwav.mcdev.facet.MinecraftFacet import com.demonwav.mcdev.platform.fabric.FabricModuleType +import com.demonwav.mcdev.platform.fabric.annotater.javadoc.AnnotaterJavadocStore import com.demonwav.mcdev.platform.minecraft.MinecraftModuleSettings import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.components.Service @@ -32,7 +33,6 @@ import com.intellij.openapi.util.Key import com.intellij.openapi.vfs.VirtualFile import com.intellij.psi.PsiElement -import com.intellij.psi.util.PsiUtilCore import java.io.IOException import java.nio.file.Path import java.util.concurrent.ConcurrentHashMap @@ -52,6 +52,7 @@ object : AnnotaterListener { override fun checkoutsChanged() { checkouts.clear() + AnnotaterJavadocStore.invalidateAll() } }, ) @@ -69,7 +70,7 @@ /** * Finds the checkout for [version] according to the user's settings, ignoring whether the feature is enabled. */ - fun statusForVersion(version: String): Status { + private fun statusForVersion(version: String): Status { val settings = AnnotaterSettings.instance return when (settings.checkoutMode) { AnnotaterSettings.CheckoutMode.MANAGED -> { @@ -105,7 +106,7 @@ if (!AnnotaterSettings.instance.enabled) { return Status.Disabled } - val file = PsiUtilCore.getVirtualFile(element) ?: return Status.UnknownMinecraftVersion + val file = virtualFileOf(element) ?: return Status.UnknownMinecraftVersion val version = minecraftVersion(element.project, file) ?: return Status.UnknownMinecraftVersion return statusForVersion(version) } @@ -118,7 +119,7 @@ if (!AnnotaterSettings.instance.enabled) { return null } - val file = PsiUtilCore.getVirtualFile(element) ?: return null + val file = virtualFileOf(element) ?: return null return checkoutFor(element.project, file) } @@ -179,10 +180,17 @@ * prominently there. */ fun isFabricProject(project: Project): Boolean { - return ModuleManager.getInstance(project).modules.any { MinecraftFacet.getInstance(it, FabricModuleType) != null } + return ModuleManager.getInstance(project).modules.any { + MinecraftFacet.getInstance(it, FabricModuleType) != null - } + } + } /** + * The file containing [element]. Elements of a class file's decompiled text are in the class file. + */ + fun virtualFileOf(element: PsiElement): VirtualFile? = element.containingFile?.originalFile?.virtualFile + + /** * Whether [file] is a class or source file from a Minecraft jar, as opposed to project sources or another * library. */ Index: src/main/kotlin/platform/fabric/annotater/AnnotaterConfig.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/AnnotaterConfig.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/AnnotaterConfig.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -22,6 +22,7 @@ import com.google.gson.Gson import com.google.gson.JsonParseException +import com.intellij.openapi.util.io.FileUtil import java.io.IOException import java.nio.file.Path import kotlin.io.path.exists @@ -58,23 +59,18 @@ val version = raw.version?.takeIf { it.isNotBlank() } ?: throw IOException("$FILE_NAME is missing \"version\"") - val mappings = validatePath(raw.mappings ?: throw IOException("$FILE_NAME is missing \"mappings\"")) - val unpick = raw.unpickDefinitions?.let(::validatePath) + val mappings = validatePath(root, raw.mappings ?: throw IOException("$FILE_NAME is missing \"mappings\"")) + val unpick = raw.unpickDefinitions?.let { validatePath(root, it) } return AnnotaterConfig(version, mappings, unpick) } /** - * Paths must be relative subdirectories using forward slashes, like mcsrc requires. + * Paths must be relative paths to subdirectories, like mcsrc requires. */ @Throws(IOException::class) - private fun validatePath(path: String): String { - val segments = path.split('/') - val valid = path.isNotEmpty() && - !path.startsWith('/') && - '\\' !in path && - ':' !in path && - segments.none { it.isEmpty() || it == "." || it == ".." } - if (!valid) { + private fun validatePath(root: Path, path: String): String { + val resolved = runCatching { root.resolve(path).normalize() }.getOrNull() + if (resolved == null || !FileUtil.isAncestor(root.normalize().toFile(), resolved.toFile(), true)) { throw IOException("Invalid path in $FILE_NAME: \"$path\"") } return path Index: src/main/kotlin/platform/fabric/annotater/AnnotaterConfigurable.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/AnnotaterConfigurable.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/AnnotaterConfigurable.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -59,7 +59,11 @@ override fun createComponent(): JComponent = panel { row( - JLabel(MCDevBundle("fabric.annotater.settings.title"), GeneralAssets.ANNOTATER_LARGE, SwingConstants.LEADING) + JLabel( + MCDevBundle("fabric.annotater.settings.title"), + GeneralAssets.TINY_POTATO_LARGE, + SwingConstants.LEADING, + ) .apply { font = font.deriveFont(Font.BOLD, font.size * 1.5f) }, ) {}.bottomGap(BottomGap.MEDIUM) @@ -72,6 +76,9 @@ } indent { row { + text(contributionGuidelinesNote()) + } + row { checkBox(MCDevBundle("fabric.annotater.settings.show_javadocs")) .bindSelected(settings::showAnnotaterJavadocs) .comment(MCDevBundle("fabric.annotater.settings.show_javadocs.comment")) @@ -162,6 +169,26 @@ return null } + /** + * A reminder to follow annotater's contribution guidelines, which can differ between version branches, linking to + * those of the versions used by open projects, or of the default branch if there are none. + */ + private fun contributionGuidelinesNote(): String { + fun link(branch: String, text: String) = + "$text" + + val versions = knownMinecraftVersions() + val links = when (versions.size) { + 0 -> link("HEAD", MCDevBundle("fabric.annotater.settings.guidelines.link")) + 1 -> link(versions.single(), MCDevBundle("fabric.annotater.settings.guidelines.link")) + else -> MCDevBundle( + "fabric.annotater.settings.guidelines.link.version", + versions.joinToString(", ") { link(it, it) }, + ) + } + return MCDevBundle("fabric.annotater.settings.guidelines", links) + } + private fun cloneOrUpdate(vcs: AnnotaterVcs) { val versions = knownMinecraftVersions() val version = when (versions.size) { @@ -173,6 +200,7 @@ ) return } + 1 -> versions.single() else -> Messages.showEditableChooseDialog( MCDevBundle("fabric.annotater.settings.clone_now.version"), @@ -209,18 +237,18 @@ override fun reset() = panel.reset() - companion object { - /** - * The Minecraft versions of all modules in open projects, as configured in their Minecraft module settings. - */ + /** + * The Minecraft versions of all modules in open projects, as configured in their Minecraft module settings. + */ - fun knownMinecraftVersions(): List { + private fun knownMinecraftVersions(): List { - return ProjectManager.getInstance().openProjects.flatMap { project -> - ModuleManager.getInstance(project).modules.mapNotNull { - MinecraftModuleSettings.getInstance(it).state.minecraftVersion - } - }.filter { it.isNotBlank() }.distinct() - } + return ProjectManager.getInstance().openProjects.flatMap { project -> + ModuleManager.getInstance(project).modules.mapNotNull { + MinecraftModuleSettings.getInstance(it).state.minecraftVersion + } + }.filter { it.isNotBlank() }.distinct() + } + object Util { fun show(project: Project?) { ShowSettingsUtil.getInstance().showSettingsDialog(project, AnnotaterConfigurable::class.java) } Index: src/main/kotlin/platform/fabric/annotater/AnnotaterNotifications.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/AnnotaterNotifications.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/AnnotaterNotifications.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -29,7 +29,7 @@ object AnnotaterNotifications { const val GROUP_ID = "Minecraft annotater" - fun describe(status: AnnotaterCheckoutService.Status): String = when (status) { + private fun describe(status: AnnotaterCheckoutService.Status): String = when (status) { is AnnotaterCheckoutService.Status.Available -> status.checkout.root.toString() AnnotaterCheckoutService.Status.Disabled -> MCDevBundle("fabric.annotater.status.disabled") AnnotaterCheckoutService.Status.UnknownMinecraftVersion -> @@ -64,7 +64,7 @@ } notification.addAction( NotificationAction.createSimpleExpiring(MCDevBundle("fabric.annotater.configure")) { - AnnotaterConfigurable.show(project) + AnnotaterConfigurable.Util.show(project) }, ) notification.notify(project) Index: src/main/kotlin/platform/fabric/annotater/AnnotaterVcs.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/AnnotaterVcs.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/AnnotaterVcs.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -73,6 +73,18 @@ indicator: ProgressIndicator, ) + /** + * Deletes the contribution [branch] from the managed [checkout]. Only the local branch is deleted, not any branch + * pushed for it or its pull request. If [branch] is checked out, its uncommitted changes are discarded and the + * upstream version branch is checked out instead. Must not be called on the EDT. + */ + fun deleteContribution( + project: Project?, + checkout: AnnotaterCheckout, + branch: String, + indicator: ProgressIndicator, + ) + companion object { val instance: AnnotaterVcs? get() = ApplicationManager.getApplication().getService(AnnotaterVcs::class.java) @@ -108,10 +120,19 @@ * Creating pull requests to annotater. Only available when the GitHub plugin is installed. */ interface AnnotaterPullRequests { - fun canCreatePullRequest(checkout: AnnotaterCheckout): Boolean = checkout.managed + enum class Action { CREATE, UPDATE } - fun createPullRequest(project: Project, checkout: AnnotaterCheckout) + /** + * Whether a pull request can be created or updated with the changes in [checkout], or null if there's nothing to + * submit. This is cheap enough to call from any thread, so it may be null until the changes have been looked up. + */ + fun availableAction(checkout: AnnotaterCheckout): Action? + /** + * Creates a pull request with the changes in [checkout], or pushes them to its existing pull request. + */ + fun submit(project: Project, checkout: AnnotaterCheckout) + companion object { val instance: AnnotaterPullRequests? get() = ApplicationManager.getApplication().getService(AnnotaterPullRequests::class.java) Index: src/main/kotlin/platform/fabric/annotater/git/AnnotaterGitVcs.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/git/AnnotaterGitVcs.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/git/AnnotaterGitVcs.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -36,6 +36,7 @@ import com.intellij.openapi.vcs.VcsException import com.intellij.openapi.vfs.LocalFileSystem import com.intellij.ui.EditorNotifications +import com.intellij.util.concurrency.AppExecutorUtil import git4idea.commands.Git import git4idea.commands.GitCommand import git4idea.commands.GitLineHandler @@ -68,6 +69,39 @@ */ private val titledBranches = ConcurrentHashMap.newKeySet() + /** + * The [PushStatus] of each managed checkout by root, so that the editor banner can show it without running git. + */ + private val pushStatuses = ConcurrentHashMap() + + /** + * The checkouts whose [pushStatuses] have been requested, to refresh them when they change. + */ + private val pushStatusCheckouts = ConcurrentHashMap() + + /** + * The checkouts whose [pushStatuses] are waiting to be refreshed. + */ + private val pendingPushStatuses = ConcurrentHashMap.newKeySet() + + private val pushStatusExecutor = + AppExecutorUtil.createBoundedApplicationPoolExecutor("Annotater push status", 1) + + init { + ApplicationManager.getApplication().messageBus.connect().subscribe( + AnnotaterListener.TOPIC, + object : AnnotaterListener { + override fun checkoutsChanged() { + pushStatusCheckouts.values.forEach(::refreshPushStatus) + } + + override fun javadocsChanged(checkoutRoot: Path, outerClass: String) { + pushStatusCheckouts[checkoutRoot]?.let(::refreshPushStatus) + } + }, + ) + } + override fun cloneOrUpdate( project: Project?, version: String, @@ -118,7 +152,7 @@ } @Throws(VcsException::class) - fun cloneOrUpdateNow(project: Project?, version: String, indicator: ProgressIndicator): AnnotaterCheckout { + private fun cloneOrUpdateNow(project: Project?, version: String, indicator: ProgressIndicator): AnnotaterCheckout { val gitProject = project ?: ProjectManager.getInstance().defaultProject synchronized(lock) { val root = AnnotaterSettings.instance.effectiveManagedRoot @@ -290,6 +324,85 @@ EditorNotifications.updateAll() } + /** + * Whether the contribution checked out in a managed checkout has a pull request, and whether it has changes that + * haven't been pushed to it, or to a new pull request if it doesn't have one. + */ + data class PushStatus(val pullRequestUrl: String?, val hasUnpushedChanges: Boolean) + + /** + * The [PushStatus] of the managed [checkout], or null if it isn't known yet, in which case it's looked up in the + * background and the editor banners are updated. This is cheap enough to call from any thread. + */ + fun pushStatus(checkout: AnnotaterCheckout): PushStatus? { + if (!checkout.managed) { + return null + } + if (pushStatusCheckouts.put(checkout.root, checkout) == null) { + refreshPushStatus(checkout) + } + return pushStatuses[checkout.root] + } + + private fun refreshPushStatus(checkout: AnnotaterCheckout) { + val root = checkout.root + if (!pendingPushStatuses.add(root)) { + return + } + pushStatusExecutor.execute { + pendingPushStatuses.remove(root) + val status = runCatching { computePushStatus(ProjectManager.getInstance().defaultProject, checkout) } + .getOrNull() + val old = if (status == null) pushStatuses.remove(root) else pushStatuses.put(root, status) + if (old != status) { + EditorNotifications.updateAll() + } + } + } + + private fun computePushStatus(project: Project, checkout: AnnotaterCheckout): PushStatus { + val branch = currentBranch(project, checkout.root) ?: return PushStatus(null, false) + val pullRequestUrl = getBranchConfig(project, checkout, branch, PULL_REQUEST_KEY) + val pushedCommit = getBranchConfig(project, checkout, branch, PUSHED_COMMIT_KEY) + val hasUnpushedChanges = when { + uncommittedMappingFiles(project, checkout).isNotEmpty() -> true + pullRequestUrl != null && pushedCommit != null -> headCommit(project, checkout) != pushedCommit + else -> commitsAheadOfUpstream(project, checkout) > 0 + } + return PushStatus(pullRequestUrl, hasUnpushedChanges) + } + + /** + * The URL of the pull request opened for the contribution [branch], if any. + */ + fun pullRequestUrl(project: Project, checkout: AnnotaterCheckout, branch: String): String? = + getBranchConfig(project, checkout, branch, PULL_REQUEST_KEY) + + /** + * Remembers that the contribution [branch] has been pushed to the pull request at [url], so that the editor banner + * offers to update it when there are new changes. + */ + fun setPullRequest(project: Project, checkout: AnnotaterCheckout, branch: String, url: String) { + run(project, checkout.root, GitCommand.CONFIG, "branch.$branch.$PULL_REQUEST_KEY", url) + run( + project, + checkout.root, + GitCommand.CONFIG, + "branch.$branch.$PUSHED_COMMIT_KEY", + headCommit(project, checkout), + ) + refreshPushStatus(checkout) + } + + private fun getBranchConfig(project: Project, checkout: AnnotaterCheckout, branch: String, key: String): String? { + // git config exits with 1 when the key isn't set + return runCatching { run(project, checkout.root, GitCommand.CONFIG, "--get", "branch.$branch.$key") } + .getOrNull()?.firstOrNull()?.trim()?.takeIf { it.isNotEmpty() } + } + + private fun headCommit(project: Project, checkout: AnnotaterCheckout): String = + run(project, checkout.root, GitCommand.REV_PARSE, "HEAD").first().trim() + override fun uncommittedMappingFiles(project: Project?, checkout: AnnotaterCheckout): List { val output = run( project ?: ProjectManager.getInstance().defaultProject, @@ -374,6 +487,45 @@ ApplicationManager.getApplication().messageBus.syncPublisher(AnnotaterListener.TOPIC).checkoutsChanged() } + override fun deleteContribution( + project: Project?, + checkout: AnnotaterCheckout, + branch: String, + indicator: ProgressIndicator, + ) { + val gitProject = project ?: ProjectManager.getInstance().defaultProject + synchronized(lock) { + if (currentBranch(gitProject, checkout.root) == branch) { + // Discard the contribution's uncommitted changes, including new mapping files + val untracked = run( + gitProject, + checkout.root, + GitCommand.STATUS, + "--porcelain", + "--untracked-files=all", + "--", + checkout.config.mappings, + ).filter { it.startsWith("?? ") }.map { checkout.root.resolve(it.substring(3)) } + run( + gitProject, + checkout.root, + GitCommand.CHECKOUT, + "--force", + "--detach", + "$UPSTREAM_REMOTE/${checkout.version}", + ) + untracked.forEach { Files.deleteIfExists(it) } + } + // Also deletes the branch's configuration, such as its pull request title + run(gitProject, checkout.root, GitCommand.BRANCH, "-D", branch) + titles.remove(branch) + contributionTitles.remove(branch) + titledBranches.remove(branch) + } + LocalFileSystem.getInstance().refreshNioFiles(listOf(checkout.root), true, true, null) + ApplicationManager.getApplication().messageBus.syncPublisher(AnnotaterListener.TOPIC).checkoutsChanged() + } + private fun branchPrefix(checkout: AnnotaterCheckout) = "mcdev/${checkout.version}/" private fun currentBranch(project: Project, dir: Path): String? { @@ -411,14 +563,21 @@ companion object { private const val REPO_DIR = ".repo" private const val HEAD_REF_PREFIX = "ref: refs/heads/" - const val UPSTREAM_REMOTE = "origin" + private const val UPSTREAM_REMOTE = "origin" /** + * The git config keys of contribution branches, under `branch.`, storing the URL of their pull request + * and the commit last pushed to it. + */ + private const val PULL_REQUEST_KEY = "mcdevPullRequest" + private const val PUSHED_COMMIT_KEY = "mcdevPushedCommit" + + /** * The GitHub repository annotater is cloned from and pull requests are opened against, as `owner/name`. Can * be overridden to test against a fork without opening real pull requests. */ val upstreamRepository: String = System.getProperty("mcdev.annotater.upstream", "FabricMC/annotater") - val upstreamUrl = "https://github.com/$upstreamRepository.git" + private val upstreamUrl = "https://github.com/$upstreamRepository.git" val instance: AnnotaterGitVcs? get() = AnnotaterVcs.instance as? AnnotaterGitVcs Index: src/main/kotlin/platform/fabric/annotater/github/AnnotaterBuildCheck.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/github/AnnotaterBuildCheck.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) +++ src/main/kotlin/platform/fabric/annotater/github/AnnotaterBuildCheck.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -0,0 +1,123 @@ +/* + * Minecraft Development for IntelliJ + * + * https://mcdev.io/ + * + * Copyright (C) 2026 minecraft-dev + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation, version 3.0 only. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + + +package com.demonwav.mcdev.platform.fabric.annotater.github + +import com.demonwav.mcdev.asset.MCDevBundle +import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterCheckout +import com.intellij.execution.configurations.GeneralCommandLine +import com.intellij.execution.process.CapturingProcessHandler +import com.intellij.execution.process.ProcessOutput +import com.intellij.openapi.progress.ProgressIndicator +import com.intellij.openapi.progress.ProgressManager +import com.intellij.openapi.progress.Task +import com.intellij.openapi.project.Project +import com.intellij.openapi.projectRoots.JavaSdkType +import com.intellij.openapi.roots.ProjectRootManager +import com.intellij.openapi.ui.DialogWrapper +import com.intellij.openapi.util.SystemInfo +import com.intellij.ui.components.JBScrollPane +import com.intellij.ui.components.JBTextArea +import com.intellij.ui.dsl.builder.AlignX +import com.intellij.ui.dsl.builder.AlignY +import com.intellij.ui.dsl.builder.panel +import com.intellij.util.ui.JBUI +import java.awt.Dimension +import java.awt.Font + +/** + * Runs annotater's own checks with `gradlew check` before a pull request is submitted. + */ +object AnnotaterBuildCheck { + /** + * Runs the checks in [checkout] with a modal progress dialog. If they fail, the user is shown the output and can + * choose to submit anyway. + * + * @return whether to continue submitting + */ + fun run(project: Project, checkout: AnnotaterCheckout): Boolean { + var result: ProcessOutput? = null + var error: Throwable? = null + // Modal tasks finish before this returns + ProgressManager.getInstance().run( + object : Task.Modal(project, MCDevBundle("fabric.annotater.pr.check_build.progress"), true) { + override fun run(indicator: ProgressIndicator) { + indicator.isIndeterminate = true + result = CapturingProcessHandler(commandLine(project, checkout)) + .runProcessWithProgressIndicator(indicator) + } + + override fun onThrowable(e: Throwable) { + error = e + } + }, + ) + error?.let { return FailedDialog(project, it.message ?: it.toString()).showAndGet() } + val output = result?.takeUnless { it.isCancelled } ?: return false + if (output.exitCode == 0 && !output.isTimeout) { + return true + } + val log = (output.stdout + "\n" + output.stderr).trim() + return FailedDialog(project, log).showAndGet() + } + + private fun commandLine(project: Project, checkout: AnnotaterCheckout): GeneralCommandLine { + val wrapper = checkout.root.resolve(if (SystemInfo.isWindows) "gradlew.bat" else "gradlew") + val commandLine = GeneralCommandLine(wrapper.toString(), "check", "--console=plain") + .withWorkingDirectory(checkout.root) + .withCharset(Charsets.UTF_8) + // Minecraft projects need a recent Java version, which the environment's JAVA_HOME may not be + val sdk = ProjectRootManager.getInstance(project).projectSdk?.takeIf { it.sdkType is JavaSdkType } + sdk?.homePath?.let { commandLine.withEnvironment("JAVA_HOME", it) } + return commandLine + } + + /** + * Shows the output of the failed checks, offering to cancel submitting, which is the default, or submit anyway. + */ + private class FailedDialog(project: Project, private val log: String) : DialogWrapper(project) { + init { + title = MCDevBundle("fabric.annotater.pr.check_build.failed") + setOKButtonText(MCDevBundle("fabric.annotater.pr.check_build.submit_anyway")) + okAction.putValue(DEFAULT_ACTION, null) + cancelAction.putValue(DEFAULT_ACTION, true) + init() + } + + override fun createCenterPanel() = panel { + row { + label(MCDevBundle("fabric.annotater.pr.check_build.failed.message")) + } + row { + val textArea = JBTextArea(log).apply { + isEditable = false + font = JBUI.Fonts.create(Font.MONOSPACED, font.size) + // The cause of a failure is usually at the end + caretPosition = document.length + } + cell(JBScrollPane(textArea)) + .align(AlignX.FILL) + .align(AlignY.FILL) + .applyToComponent { preferredSize = Dimension(JBUI.scale(800), JBUI.scale(400)) } + }.resizableRow() + } + } +} Index: src/main/kotlin/platform/fabric/annotater/github/AnnotaterGitHubPullRequests.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/github/AnnotaterGitHubPullRequests.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/github/AnnotaterGitHubPullRequests.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -52,10 +52,16 @@ * request, using accounts from the GitHub plugin. */ class AnnotaterGitHubPullRequests : AnnotaterPullRequests { - override fun canCreatePullRequest(checkout: AnnotaterCheckout) = - checkout.managed && AnnotaterGitVcs.instance != null + override fun availableAction(checkout: AnnotaterCheckout): AnnotaterPullRequests.Action? { + val status = AnnotaterGitVcs.instance?.pushStatus(checkout)?.takeIf { it.hasUnpushedChanges } ?: return null + return if (status.pullRequestUrl != null) { + AnnotaterPullRequests.Action.UPDATE + } else { + AnnotaterPullRequests.Action.CREATE + } + } - override fun createPullRequest(project: Project, checkout: AnnotaterCheckout) { + override fun submit(project: Project, checkout: AnnotaterCheckout) { val git = AnnotaterGitVcs.instance ?: return object : Task.Backgroundable(project, MCDevBundle("fabric.annotater.pr.checking"), true) { @@ -64,6 +70,7 @@ private var commitsAhead = 0 private var branch: String? = null private var existingTitle: String? = null + private var existingUrl: String? = null override fun run(indicator: ProgressIndicator) { changes = git.mappingFilesChangedSinceUpstream(project, checkout) @@ -71,6 +78,7 @@ commitsAhead = git.commitsAheadOfUpstream(project, checkout) branch = git.currentContribution(checkout)?.branch existingTitle = branch?.let { git.pullRequestTitle(project, checkout, it) } + existingUrl = branch?.let { git.pullRequestUrl(project, checkout, it) } } override fun onSuccess() { @@ -83,7 +91,7 @@ ) return } - showDialog(project, checkout, git, branch, changes, uncommittedChanges, existingTitle) + showDialog(project, checkout, git, branch, changes, uncommittedChanges, existingTitle, existingUrl) } override fun onThrowable(error: Throwable) { @@ -100,6 +108,7 @@ changes: List, uncommittedChanges: List, existingTitle: String?, + existingUrl: String?, ) { if (GHAccountsUtil.accounts.isEmpty()) { GHAccountsUtil.requestNewAccount(GithubServerPath.DEFAULT_SERVER, null, project) ?: return @@ -115,30 +124,47 @@ } val defaultAccount = GHAccountsUtil.getSingleOrDefaultAccount(project)?.takeIf { it in accounts } - val dialog = - AnnotaterPullRequestDialog(project, accounts, defaultAccount, changes, uncommittedChanges, existingTitle) + val dialog = AnnotaterPullRequestDialog( + project, + accounts, + defaultAccount, + changes, + uncommittedChanges, + existingTitle, + update = existingUrl != null, + ) if (!dialog.showAndGet()) { return } val account = dialog.account ?: return + if (dialog.checkBuild && !AnnotaterBuildCheck.run(project, checkout)) { + return + } val token = GHCompatibilityUtil.getOrRequestToken(account, project) ?: return - val request = PullRequest(dialog.prTitle, dialog.prDescription, dialog.commitMessage, branch) - object : Task.Backgroundable(project, MCDevBundle("fabric.annotater.pr.creating"), false) { + val request = PullRequest(dialog.prTitle, dialog.prDescription, dialog.commitMessage, branch, existingUrl) + val title = if (existingUrl != null) "fabric.annotater.pr.updating" else "fabric.annotater.pr.creating" + object : Task.Backgroundable(project, MCDevBundle(title), false) { private var url: String? = null override fun run(indicator: ProgressIndicator) { - url = + val url = submit(project, checkout, git, account, token, request, uncommittedChanges.isNotEmpty(), indicator) + this.url = url + git.setPullRequest(project, checkout, request.branch, url) + if (existingUrl == null) { - // Label the contribution with its pull request, for switching back to it later - runCatching { git.setContributionTitle(project, checkout, request.branch, request.title) } - } + // Label the contribution with its pull request, for switching back to it later + runCatching { git.setContributionTitle(project, checkout, request.branch, request.title) } + } + } override fun onSuccess() { val url = url ?: return + val notificationTitle = + if (existingUrl != null) "fabric.annotater.pr.updated" else "fabric.annotater.pr.created" Notification( AnnotaterNotifications.GROUP_ID, - MCDevBundle("fabric.annotater.pr.created"), + MCDevBundle(notificationTitle), url, NotificationType.INFORMATION, ).addAction( @@ -147,10 +173,9 @@ }, ).addAction( NotificationAction.createSimpleExpiring(MCDevBundle("fabric.annotater.pr.start_new")) { - AnnotaterJavadocContributions.switchTo(project, checkout, null) + AnnotaterJavadocContributions.switchContribution(project, checkout, null) }, ).notify(project) - BrowserUtil.browse(url) } override fun onThrowable(error: Throwable) { @@ -159,8 +184,20 @@ }.queue() } - private class PullRequest(val title: String, val body: String, val commitMessage: String, val branch: String) + /** + * @property existingUrl the URL of the pull request to update, or null to create one + */ + private class PullRequest( + val title: String, + val body: String, + val commitMessage: String, + val branch: String, + val existingUrl: String?, + ) + /** + * Commits and pushes the changes, and creates the pull request unless it already exists. Returns its URL. + */ @Throws(IOException::class, VcsException::class) private fun submit( project: Project, @@ -176,7 +213,7 @@ val upstream = AnnotaterGitVcs.upstreamRepository // Otherwise GitHub only rejects the pull request with "base invalid" after committing, forking and pushing - if (!api.exists("/repos/$upstream/branches/${checkout.version}")) { + if (request.existingUrl == null && !api.exists("/repos/$upstream/branches/${checkout.version}")) { throw IOException(MCDevBundle("fabric.annotater.pr.no_base_branch", upstream, checkout.version)) } @@ -185,6 +222,7 @@ git.commitMappings(project, checkout, request.commitMessage) } + // Returns the existing fork if there is one indicator.text = MCDevBundle("fabric.annotater.pr.forking") val fork = api.post("/repos/$upstream/forks", JsonObject()) val forkName = fork.get("full_name").asString @@ -194,6 +232,9 @@ indicator.text = MCDevBundle("fabric.annotater.pr.pushing") git.push(project, checkout, cloneUrl, request.branch) + if (request.existingUrl != null) { + return request.existingUrl + } indicator.text = MCDevBundle("fabric.annotater.pr.opening") val body = JsonObject().apply { Index: src/main/kotlin/platform/fabric/annotater/github/AnnotaterPullRequestDialog.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/github/AnnotaterPullRequestDialog.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/github/AnnotaterPullRequestDialog.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -27,6 +27,7 @@ import com.intellij.ui.CollectionComboBoxModel import com.intellij.ui.SimpleListCellRenderer import com.intellij.ui.dsl.builder.AlignX +import com.intellij.ui.dsl.builder.bindSelected import com.intellij.ui.dsl.builder.bindText import com.intellij.ui.dsl.builder.panel import com.intellij.ui.dsl.builder.rows @@ -40,19 +41,29 @@ private val changes: List, private val uncommittedChanges: List, existingTitle: String?, + /** + * Whether the changes are pushed to an existing pull request, so there's no title or description to enter. + */ + private val update: Boolean, ) : DialogWrapper(project) { private val accountModel = CollectionComboBoxModel(accounts, defaultAccount ?: accounts.first()) var prTitle = existingTitle ?: AnnotaterContribution.defaultTitle(changes) var prDescription = "" var commitMessage = AnnotaterContribution.defaultTitle(uncommittedChanges) + var checkBuild = true val account: GithubAccount? get() = accountModel.selected init { + if (update) { + title = MCDevBundle("fabric.annotater.update_pull_request") + setOKButtonText(MCDevBundle("fabric.annotater.pr.dialog.update")) + } else { - title = MCDevBundle("fabric.annotater.create_pull_request") - setOKButtonText(MCDevBundle("fabric.annotater.pr.dialog.ok")) + title = MCDevBundle("fabric.annotater.create_pull_request") + setOKButtonText(MCDevBundle("fabric.annotater.pr.dialog.ok")) + } init() } @@ -61,17 +72,27 @@ comboBox(accountModel, SimpleListCellRenderer.create("") { it.name }) .align(AlignX.FILL) } + if (!update) { - row(MCDevBundle("fabric.annotater.pr.dialog.title")) { - textField().bindText(::prTitle).align(AlignX.FILL).focused() - } - row(MCDevBundle("fabric.annotater.pr.dialog.description")) { - textArea().bindText(::prDescription).rows(6).align(AlignX.FILL) - } + row(MCDevBundle("fabric.annotater.pr.dialog.title")) { + textField().bindText(::prTitle).align(AlignX.FILL).focused() + } + row(MCDevBundle("fabric.annotater.pr.dialog.description")) { + textArea().bindText(::prDescription).rows(6).align(AlignX.FILL) + } + } if (uncommittedChanges.isNotEmpty()) { row(MCDevBundle("fabric.annotater.pr.dialog.commit_message")) { - textField().bindText(::commitMessage).align(AlignX.FILL) + val field = textField().bindText(::commitMessage).align(AlignX.FILL) + if (update) { + field.focused() - } - } + } + } + } + row { + checkBox(MCDevBundle("fabric.annotater.pr.dialog.check_build")) + .bindSelected(::checkBuild) + .comment(MCDevBundle("fabric.annotater.pr.dialog.check_build.comment")) + } if (changes.isNotEmpty()) { row { comment( Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocCommentInspection.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocCommentInspection.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocCommentInspection.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -21,8 +21,9 @@ package com.demonwav.mcdev.platform.fabric.annotater.javadoc import com.demonwav.mcdev.asset.MCDevBundle +import com.demonwav.mcdev.util.McdevReflection +import com.demonwav.mcdev.util.McdevReflection.newInstanceUnwrap import com.intellij.codeInspection.LocalInspectionTool -import com.intellij.codeInspection.LocalQuickFix import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder import com.intellij.psi.JavaElementVisitor @@ -30,7 +31,6 @@ import com.intellij.psi.PsiElementVisitor import com.intellij.psi.javadoc.PsiDocComment import com.intellij.psi.util.PsiTreeUtil -import com.siyeh.ig.migration.MarkdownDocumentationCommentsMigrationInspection /** * Annotater only supports Markdown (`///`) Javadoc comments. @@ -56,7 +56,7 @@ comment, MCDevBundle("fabric.annotater.javadoc.error.not_markdown"), ProblemHighlightType.GENERIC_ERROR, - *findMarkdownMigrationFixes(holder, comment), + McdevReflection.CONSTRUCTOR_MARKDOWN_DOCUMENTATION_COMMENTS_MIGRATION_FIX.newInstanceUnwrap(), ) } @@ -73,16 +73,4 @@ } } } - - /** - * Reuses the IDE's own "convert to Markdown documentation comment" quick fix. - */ - private fun findMarkdownMigrationFixes(holder: ProblemsHolder, comment: PsiDocComment): Array { - val migrationHolder = ProblemsHolder(holder.manager, holder.file, holder.isOnTheFly) - comment.accept(MarkdownDocumentationCommentsMigrationInspection().buildVisitor(migrationHolder, true)) - return migrationHolder.results - .flatMap { it.fixes.orEmpty().asIterable() } - .filterIsInstance() - .toTypedArray() - } +} -} Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocContributions.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocContributions.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocContributions.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -42,7 +42,8 @@ import javax.swing.JComponent /** - * Switching a managed annotater checkout between contributions, so that a pull request can be updated after review. + * Switching a managed annotater checkout between contributions, so that a pull request can be updated after review, + * and deleting contributions. */ object AnnotaterJavadocContributions { /** @@ -52,8 +53,8 @@ vcs.currentContribution(checkout)?.displayName ?: MCDevBundle("fabric.annotater.contribution.new") /** - * Shows a popup under [component] for switching between contributions if [checkout] is managed, and configuring - * annotater. + * Shows a popup under [component] for switching between and deleting contributions if [checkout] is managed, and + * configuring annotater. */ fun showPopup(project: Project, checkout: AnnotaterCheckout, component: JComponent) { val vcs = AnnotaterVcs.instance?.takeIf { checkout.managed } @@ -92,14 +93,27 @@ } group.add( DumbAwareAction.create(MCDevBundle("fabric.annotater.contribution.new"), AllIcons.General.Add) { - switchTo(project, checkout, null) + switchContribution(project, checkout, null) }, ) + if (contributions.isNotEmpty()) { + val deleteGroup = + DefaultActionGroup.createPopupGroup { MCDevBundle("fabric.annotater.contribution.delete") } + deleteGroup.templatePresentation.icon = AllIcons.General.Delete + for (contribution in contributions) { + deleteGroup.add( + DumbAwareAction.create(contribution.displayName) { + deleteContribution(project, checkout, vcs, contribution, contribution.branch == current) + }, + ) + } + group.add(deleteGroup) + } group.add(Separator.getInstance()) } group.add( DumbAwareAction.create(MCDevBundle("fabric.annotater.configure"), AllIcons.General.Settings) { - AnnotaterConfigurable.show(project) + AnnotaterConfigurable.Util.show(project) }, ) JBPopupFactory.getInstance() @@ -125,7 +139,7 @@ ) { override fun actionPerformed(e: AnActionEvent) { if (!current) { - switchTo(project, checkout, contribution.branch) + switchContribution(project, checkout, contribution.branch) } } } @@ -135,7 +149,7 @@ * edited is saved first, any uncommitted changes are committed to the current contribution, and editing is then * restarted with the Javadoc from the new branch. */ - fun switchTo(project: Project, checkout: AnnotaterCheckout, branch: String?) { + fun switchContribution(project: Project, checkout: AnnotaterCheckout, branch: String?) { val vcs = AnnotaterVcs.instance ?: return val saves = listOfNotNull(javadocEditor(project, checkout)?.save()) CompletableFuture.allOf(*saves.toTypedArray()).thenAccept { @@ -148,12 +162,15 @@ ) return@invokeLater } - checkUncommittedChanges(project, checkout, vcs, branch) + askForCommitMessageThenSwitch(project, checkout, vcs, branch) } } } - private fun checkUncommittedChanges( + /** + * Asks for a message to commit the uncommitted changes with, if there are any, then switches to [branch]. + */ + private fun askForCommitMessageThenSwitch( project: Project, checkout: AnnotaterCheckout, vcs: AnnotaterVcs, @@ -168,7 +185,7 @@ override fun onSuccess() { if (uncommitted.isEmpty()) { - switch(project, checkout, vcs, branch, null) + commitAndCheckOut(project, checkout, vcs, branch, null) return } val current = vcs.currentContribution(checkout)?.displayName @@ -181,7 +198,7 @@ AnnotaterContribution.defaultTitle(uncommitted), null, )?.takeIf { it.isNotBlank() } ?: return - switch(project, checkout, vcs, branch, message) + commitAndCheckOut(project, checkout, vcs, branch, message) } override fun onThrowable(error: Throwable) { @@ -190,7 +207,11 @@ }.queue() } - private fun switch( + /** + * Commits the uncommitted changes with [commitMessage] if it isn't null, then checks out [branch], restarting the + * editing of the Javadoc being edited. + */ + private fun commitAndCheckOut( project: Project, checkout: AnnotaterCheckout, vcs: AnnotaterVcs, @@ -217,6 +238,56 @@ }.queue() } + /** + * Deletes [contribution] after asking for confirmation. Only the local branch is deleted. + */ + private fun deleteContribution( + project: Project, + checkout: AnnotaterCheckout, + vcs: AnnotaterVcs, + contribution: AnnotaterContribution, + current: Boolean, + ) { + val message = MCDevBundle("fabric.annotater.contribution.delete.confirm", contribution.displayName) + + if (current) "\n\n" + MCDevBundle("fabric.annotater.contribution.delete.confirm.current") else "" + val confirmed = Messages.showOkCancelDialog( + project, + message, + MCDevBundle("fabric.annotater.contribution.delete.title"), + MCDevBundle("fabric.annotater.contribution.delete.ok"), + Messages.getCancelButton(), + Messages.getWarningIcon(), + ) == Messages.OK + if (!confirmed) { + return + } + if (current) { + // The Javadoc being edited is from the deleted branch, and its changes are discarded + javadocEditor(project, checkout)?.finish(force = true) + } + object : Task.Backgroundable(project, MCDevBundle("fabric.annotater.contribution.deleting"), false) { + override fun run(indicator: ProgressIndicator) { + vcs.deleteContribution(project, checkout, contribution.branch, indicator) + } + + override fun onSuccess() { + AnnotaterNotifications.notifyInfo( + project, + MCDevBundle("fabric.annotater.contribution.deleted"), + MCDevBundle("fabric.annotater.contribution.deleted.content", contribution.displayName), + ) + } + + override fun onThrowable(error: Throwable) { + AnnotaterNotifications.notifyError( + project, + MCDevBundle("fabric.annotater.contribution.delete.failed"), + error, + ) + } + }.queue() + } + private fun javadocEditor(project: Project, checkout: AnnotaterCheckout): AnnotaterJavadocInlineEditor? = AnnotaterJavadocInlineEditor.current(project)?.takeIf { it.checkout.root == checkout.root } } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocDocumentationProvider.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocDocumentationProvider.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocDocumentationProvider.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -23,7 +23,6 @@ import com.intellij.lang.documentation.AbstractDocumentationProvider import com.intellij.openapi.project.DumbService import com.intellij.psi.PsiElement -import com.intellij.psi.PsiMember /** * Shows annotater Javadocs instead of the existing Javadoc of Minecraft classes in Quick Documentation. @@ -36,6 +35,6 @@ val member = AnnotaterJavadocEditing.normalize(element) ?: return null val checkout = AnnotaterJavadocRendering.checkoutForDisplay(member) ?: return null val target = AnnotaterJavadocTarget.fromPsi(member) ?: return null - return AnnotaterJavadocRendering.generateDoc(member, AnnotaterJavadocRendering.getJavadoc(checkout, target)) + return AnnotaterJavadocRendering.generateDoc(member, checkout.javadocStore.getOrNull(target)) } } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocEditing.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocEditing.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocEditing.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -36,24 +36,26 @@ import com.intellij.openapi.progress.Task import com.intellij.openapi.project.Project import com.intellij.openapi.wm.IdeFocusManager -import com.intellij.psi.PsiAnonymousClass -import com.intellij.psi.PsiClass import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiElement -import com.intellij.psi.PsiField import com.intellij.psi.PsiFile import com.intellij.psi.PsiJavaFile import com.intellij.psi.PsiMember -import com.intellij.psi.PsiMethod -import com.intellij.psi.PsiTypeParameter import com.intellij.psi.SmartPointerManager import com.intellij.psi.impl.compiled.ClsElementImpl import com.intellij.psi.util.PsiTreeUtil -import com.intellij.psi.util.PsiUtilCore import java.io.IOException object AnnotaterJavadocEditing { /** + * Whether to offer editing annotater Javadocs in [project]. In Fabric projects it's offered even before annotater + * is set up, to make it discoverable. + */ + fun isOffered(project: Project): Boolean { + return AnnotaterSettings.instance.enabled || AnnotaterCheckoutService.isFabricProject(project) + } + + /** * Finds the Minecraft class, field or method that the caret is on or in, which may be a reference to it. */ fun findTarget(editor: Editor, file: PsiFile): PsiMember? { @@ -70,40 +72,26 @@ } /** - * Edits the annotater Javadoc of the member found by [findTarget]. + * Converts an element of a decompiled class's text to the compiled element it was generated from, if there is + * one, and filters out elements that can't have annotater Javadocs. */ - fun edit(project: Project, editor: Editor, file: PsiFile) { - open(project, findTarget(editor, file) ?: return) + fun normalize(element: PsiElement): PsiMember? { + val member = (element.getUserData(ClsElementImpl.COMPILED_ELEMENT) ?: element) as? PsiMember ?: return null + return member.takeIf { AnnotaterJavadocTarget.fromPsi(it) != null } } - /** - * Converts an element of a decompiled class's text to the compiled element it was generated from, and filters out - * elements that can't have annotater Javadocs. - */ - fun normalize(element: PsiElement): PsiMember? { - val compiled = element.getUserData(ClsElementImpl.COMPILED_ELEMENT) ?: element - return when (compiled) { - is PsiTypeParameter, is PsiAnonymousClass -> null - is PsiClass -> compiled.takeIf { it.qualifiedName != null } - is PsiField, is PsiMethod -> compiled - else -> null + private fun isDocumentable(member: PsiMember): Boolean { + val file = AnnotaterCheckoutService.virtualFileOf(member) ?: return false + return AnnotaterCheckoutService.isMinecraftFile(member.project, file) - } + } - } /** - * Whether to offer editing annotater Javadocs in [project]. In Fabric projects it's offered even before annotater - * is set up, to make it discoverable. + * Starts editing the annotater Javadoc of the member found by [findTarget]. */ - fun isOffered(project: Project): Boolean { - return AnnotaterSettings.instance.enabled || AnnotaterCheckoutService.isFabricProject(project) + fun open(project: Project, editor: Editor, file: PsiFile) { + open(project, findTarget(editor, file) ?: return) } - fun isDocumentable(member: PsiMember): Boolean { - val file = PsiUtilCore.getVirtualFile(member) ?: return false - return AnnotaterCheckoutService.isMinecraftFile(member.project, file) && - AnnotaterJavadocTarget.fromPsi(member) != null - } - /** * Starts editing the annotater Javadoc of [member], in an editor embedded in the Minecraft class it's displayed in. */ @@ -111,7 +99,7 @@ val checkout = when (val status = AnnotaterCheckoutService.instance.status(member)) { is AnnotaterCheckoutService.Status.Available -> status.checkout AnnotaterCheckoutService.Status.Disabled -> { - AnnotaterConfigurable.show(project) + AnnotaterConfigurable.Util.show(project) return } @@ -154,7 +142,7 @@ val (host, hostMember) = findHost(project, member, target) ?: return val copy = try { - ReadAction.compute { + ReadAction.computeBlocking { AnnotaterJavadocSourceCopy.create(member, placeholderFor = target) { checkout.javadocStore.get(it) } } } catch (e: IOException) { @@ -183,7 +171,7 @@ } val file = PsiDocumentManager.getInstance(project).getPsiFile(editor.document) as? PsiJavaFile ?: return null - val hostMember = AnnotaterJavadocRendering.displayedMembers(file) + val hostMember = AnnotaterJavadocTarget.declarations(file) .firstOrNull { AnnotaterJavadocTarget.fromPsi(it) == target } ?: return null return editor to hostMember Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocEditorNotificationProvider.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocEditorNotificationProvider.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocEditorNotificationProvider.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -21,8 +21,8 @@ package com.demonwav.mcdev.platform.fabric.annotater.javadoc import com.demonwav.mcdev.asset.MCDevBundle -import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterPullRequests import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterCheckoutService +import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterPullRequests import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterVcs import com.intellij.icons.AllIcons import com.intellij.openapi.application.ApplicationManager @@ -47,6 +47,7 @@ val checkout = AnnotaterCheckoutService.instance.checkoutFor(project, file) ?: return null val javadocEditor = AnnotaterJavadocInlineEditor.current(project)?.takeIf { it.checkout.root == checkout.root } val error = javadocEditor?.file?.error + val pullRequests = AnnotaterPullRequests.instance return Function { fileEditor -> val status = @@ -59,14 +60,18 @@ } } - val pullRequests = AnnotaterPullRequests.instance - if (pullRequests != null && pullRequests.canCreatePullRequest(checkout)) { - panel.createActionLabel(MCDevBundle("fabric.annotater.create_pull_request")) { + val pullRequestAction = pullRequests?.availableAction(checkout) + if (pullRequests != null && pullRequestAction != null) { + val text = when (pullRequestAction) { + AnnotaterPullRequests.Action.CREATE -> MCDevBundle("fabric.annotater.create_pull_request") + AnnotaterPullRequests.Action.UPDATE -> MCDevBundle("fabric.annotater.update_pull_request") + } + panel.createActionLabel(text) { val saved = javadocEditor?.save() ?: CompletableFuture.completedFuture(true) saved.thenAccept { success -> if (success) { ApplicationManager.getApplication().invokeLater { - pullRequests.createPullRequest(project, checkout) + pullRequests.submit(project, checkout) } } } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlayPassFactory.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlayPassFactory.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlayPassFactory.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -85,7 +85,10 @@ private class Pass(private val file: PsiJavaFile, private val editor: Editor) : TextEditorHighlightingPass(file.project, editor.document, false) { private val documentStamp = editor.document.modificationStamp - private var active = false + + /** + * Whether annotater Javadocs replace the existing ones, which is never the case without a checkout. + */ private var showJavadocs = false private var items = emptyList() private var gutterIcons = emptyList() @@ -94,7 +97,6 @@ override fun doCollectInformation(progress: ProgressIndicator) { val first = file.classes.firstOrNull() ?: return val checkout = AnnotaterCheckoutService.instance.checkoutFor(first) ?: return - active = true showJavadocs = AnnotaterSettings.instance.showAnnotaterJavadocs val compiled = file is PsiCompiledElement // The Javadoc being edited is displayed by its editor instead @@ -103,14 +105,14 @@ val newItems = mutableListOf() val newGutterIcons = mutableListOf() val newFolds = mutableListOf() - for (member in AnnotaterJavadocRendering.displayedMembers(file)) { + for (member in AnnotaterJavadocTarget.declarations(file)) { progress.checkCanceled() val target = AnnotaterJavadocTarget.fromPsi(member) ?: continue val existing = if (compiled) null else (member as PsiJavaDocumentedElement).docComment if (existing != null && showJavadocs) { newFolds += existing.textRange } - val javadoc = AnnotaterJavadocRendering.getJavadoc(checkout, target) ?: continue + val javadoc = checkout.javadocStore.getOrNull(target) ?: continue val pointer = SmartPointerManager.createPointer(member) newGutterIcons += GutterIcon(member.textOffset, pointer) if (!showJavadocs || target == edited) { @@ -130,14 +132,14 @@ } val length = document.textLength - editor.getUserData(INLAYS_KEY)?.forEach { it.dispose() } + editor.getUserData(Const.INLAYS_KEY)?.forEach { it.dispose() } val inlays = items.filter { it.offset in 0..length }.mapNotNull { item -> val renderer = AnnotaterJavadocInlayRenderer(editor, item.offset, item.javadoc, item.pointer) editor.inlayModel.addBlockElement(item.offset, false, true, 0, renderer) } - editor.putUserData(INLAYS_KEY, inlays.takeIf { it.isNotEmpty() }) + editor.putUserData(Const.INLAYS_KEY, inlays.takeIf { it.isNotEmpty() }) - editor.getUserData(GUTTER_ICONS_KEY)?.forEach { it.dispose() } + editor.getUserData(Const.GUTTER_ICONS_KEY)?.forEach { it.dispose() } val highlighters = gutterIcons.filter { it.offset in 0..length }.map { icon -> editor.markupModel.addRangeHighlighter( icon.offset, @@ -147,9 +149,9 @@ HighlighterTargetArea.LINES_IN_RANGE, ).also { it.gutterIconRenderer = EditGutterIconRenderer(icon.pointer) } } - editor.putUserData(GUTTER_ICONS_KEY, highlighters.takeIf { it.isNotEmpty() }) + editor.putUserData(Const.GUTTER_ICONS_KEY, highlighters.takeIf { it.isNotEmpty() }) - val oldFolds = editor.getUserData(FOLDS_KEY).orEmpty() + val oldFolds = editor.getUserData(Const.FOLDS_KEY).orEmpty() if (oldFolds.isNotEmpty() || folds.isNotEmpty()) { val foldingModel = editor.foldingModel val newFolds = mutableListOf() @@ -165,23 +167,22 @@ region?.isExpanded = false } } - editor.putUserData(FOLDS_KEY, newFolds.takeIf { it.isNotEmpty() }) + editor.putUserData(Const.FOLDS_KEY, newFolds.takeIf { it.isNotEmpty() }) } - val hideRenderedDocs = active && showJavadocs - if (hideRenderedDocs && DocRenderManager.isDocRenderingEnabled(editor)) { + if (showJavadocs && DocRenderManager.isDocRenderingEnabled(editor)) { DocRenderManager.setDocRenderingEnabled(editor, false) - editor.putUserData(DOC_RENDERING_DISABLED_KEY, true) - } else if (!hideRenderedDocs && editor.getUserData(DOC_RENDERING_DISABLED_KEY) == true) { + editor.putUserData(Const.DOC_RENDERING_DISABLED_KEY, true) + } else if (!showJavadocs && editor.getUserData(Const.DOC_RENDERING_DISABLED_KEY) == true) { DocRenderManager.resetEditorToDefaultState(editor) - editor.putUserData(DOC_RENDERING_DISABLED_KEY, null) + editor.putUserData(Const.DOC_RENDERING_DISABLED_KEY, null) } } } private class EditGutterIconRenderer(private val pointer: SmartPsiElementPointer) : GutterIconRenderer() { - override fun getIcon(): Icon = GeneralAssets.ANNOTATER + override fun getIcon(): Icon = GeneralAssets.TINY_POTATO override fun getTooltipText() = MCDevBundle("fabric.annotater.javadoc.edit") @@ -202,10 +203,10 @@ } } - companion object { - private val INLAYS_KEY = Key.create>>("mcdev.annotater.javadocInlays") - private val FOLDS_KEY = Key.create>("mcdev.annotater.javadocFolds") - private val GUTTER_ICONS_KEY = Key.create>("mcdev.annotater.javadocGutterIcons") - private val DOC_RENDERING_DISABLED_KEY = Key.create("mcdev.annotater.docRenderingDisabled") + private object Const { + val INLAYS_KEY = Key.create>>("mcdev.annotater.javadocInlays") + val FOLDS_KEY = Key.create>("mcdev.annotater.javadocFolds") + val GUTTER_ICONS_KEY = Key.create>("mcdev.annotater.javadocGutterIcons") + val DOC_RENDERING_DISABLED_KEY = Key.create("mcdev.annotater.docRenderingDisabled") } } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlayRenderer.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlayRenderer.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlayRenderer.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -47,27 +47,17 @@ private val javadoc: String, private val pointer: SmartPsiElementPointer, ) : EditorCustomElementRenderer { - private var inlay: Inlay<*>? = null - - // The inlay constructor calculates the size before the inlay is added to the tree, while its offset is still - // meaningless - private val offset: Int - get() = (inlay?.takeIf { it.isValid }?.offset ?: initialOffset).coerceIn(0, editor.document.textLength) - override fun calcWidthInPixels(inlay: Inlay<*>): Int { - this.inlay = inlay - val indent = maxOf(0, startX() - editor.insets.left) + val indent = maxOf(0, startX(inlay) - editor.insets.left) return indent + lines().maxOf { line -> line.sumOf { it.width() } } } override fun calcHeightInPixels(inlay: Inlay<*>): Int { - this.inlay = inlay return lines().size * editor.lineHeight } override fun paint(inlay: Inlay<*>, g: Graphics, targetRegion: Rectangle, textAttributes: TextAttributes) { - this.inlay = inlay - val startX = startX() + val startX = startX(inlay) var y = targetRegion.y + editor.ascent for (line in lines()) { var x = startX @@ -87,7 +77,10 @@ /** * The x coordinate of the declaration, where the comment starts. */ - private fun startX(): Int { + private fun startX(inlay: Inlay<*>): Int { + // The size is first calculated while the inlay is being created, before it has a valid offset. The offset + // differs from initialOffset after the document changes, until the highlighting pass replaces the inlay + val offset = (if (inlay.isValid) inlay.offset else initialOffset).coerceIn(0, editor.document.textLength) val document = editor.document val lineStart = document.getLineStartOffset(document.getLineNumber(offset)) val contentStart = CharArrayUtil.shiftForward(document.immutableCharSequence, lineStart, " \t") Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlineEditor.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlineEditor.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocInlineEditor.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -38,6 +38,7 @@ import com.intellij.openapi.actionSystem.PlatformCoreDataKeys import com.intellij.openapi.actionSystem.UiDataProvider import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.EDT import com.intellij.openapi.components.Service import com.intellij.openapi.components.service import com.intellij.openapi.diagnostic.thisLogger @@ -45,10 +46,8 @@ import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.EditorFactory import com.intellij.openapi.editor.EditorKind -import com.intellij.openapi.editor.Inlay import com.intellij.openapi.editor.RangeMarker import com.intellij.openapi.editor.ScrollType -import com.intellij.openapi.editor.actionSystem.EditorActionManager import com.intellij.openapi.editor.event.CaretEvent import com.intellij.openapi.editor.event.CaretListener import com.intellij.openapi.editor.event.DocumentEvent @@ -64,7 +63,7 @@ import com.intellij.openapi.util.Disposer import com.intellij.openapi.util.Key import com.intellij.openapi.util.TextRange -import com.intellij.openapi.vfs.LocalFileSystem +import com.intellij.openapi.vfs.VfsUtil import com.intellij.openapi.wm.IdeFocusManager import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiJavaDocumentedElement @@ -73,7 +72,6 @@ import com.intellij.psi.SmartPsiElementPointer import com.intellij.ui.ColorUtil import com.intellij.ui.EditorNotifications -import com.intellij.util.Alarm import com.intellij.util.concurrency.AppExecutorUtil import com.intellij.util.ui.JBUI import java.awt.BorderLayout @@ -83,6 +81,12 @@ import java.io.IOException import java.util.concurrent.CompletableFuture import javax.swing.JPanel +import kotlin.time.Duration.Companion.milliseconds +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch /** * The editing of one annotater Javadoc, in an editor embedded above its declaration in the Minecraft class it's @@ -103,7 +107,10 @@ */ val source: SmartPsiElementPointer?, ) : Disposable { - private val alarm = Alarm(Alarm.ThreadToUse.SWING_THREAD, this) + /** + * The pending automatic save, which is delayed until the user stops typing. + */ + private var saveJob: Job? = null private val guards = mutableListOf() @@ -123,8 +130,6 @@ var editor: EditorEx? = null private set - private var inlay: Inlay<*>? = null - val checkout: AnnotaterCheckout get() = file.checkout @@ -133,18 +138,18 @@ init { installGuardedBlocks() - // The default handler shows a modal dialog, which is annoying when editing a guarded block by accident - EditorActionManager.getInstance().setReadonlyFragmentModificationHandler(document) { - val editor = editor ?: return@setReadonlyFragmentModificationHandler - HintManager.getInstance().showErrorHint(editor, MCDevBundle("fabric.annotater.javadoc.read_only")) - } document.addDocumentListener( object : DocumentListener { override fun documentChanged(event: DocumentEvent) { dirty = true - alarm.cancelAllRequests() - alarm.addRequest(::save, SAVE_DELAY_MS) + saveJob?.cancel() + saveJob = project.service().coroutineScope.launch(Dispatchers.EDT) { + delay(SAVE_DELAY_MS.milliseconds) + // Otherwise save would cancel this job, which cancels the save partway through + saveJob = null + save() - } + } + } }, this, ) @@ -166,21 +171,22 @@ /** * Makes everything outside of the edited doc comment read-only. The line break after the comment is read-only too, - * so that the declaration can't be joined onto the comment. + * so that the declaration can't be joined onto the comment. The guards are greedy at the ends of the document, so + * nothing can be inserted there, but not next to the comment, so that text can be inserted at its start and end. */ private fun installGuardedBlocks() { val range = commentRange() ?: return - addGuard(0, range.startOffset) - addGuard(range.endOffset, document.textLength) + addGuard(0, range.startOffset, greedyToLeft = true, greedyToRight = false) + addGuard(range.endOffset, document.textLength, greedyToLeft = false, greedyToRight = true) } - private fun addGuard(start: Int, end: Int) { + private fun addGuard(start: Int, end: Int, greedyToLeft: Boolean, greedyToRight: Boolean) { if (end <= start) { return } val guard = document.createGuardedBlock(start, end) - guard.isGreedyToLeft = false - guard.isGreedyToRight = false + guard.isGreedyToLeft = greedyToLeft + guard.isGreedyToRight = greedyToRight guards += guard } @@ -256,7 +262,7 @@ }, ) - inlay = EditorEmbeddedComponentManager.getInstance().addComponent( + val inlay = EditorEmbeddedComponentManager.getInstance().addComponent( host, component, EditorEmbeddedComponentManager.Properties( @@ -270,6 +276,7 @@ offset, ), ) + inlay?.let { Disposer.register(this, it) } // Finish editing when the Minecraft class is closed EditorFactory.getInstance().addEditorFactoryListener( @@ -351,7 +358,8 @@ */ fun save(): CompletableFuture { ApplicationManager.getApplication().assertIsDispatchThread() - alarm.cancelAllRequests() + saveJob?.cancel() + saveJob = null if (!dirty || project.isDisposed) { return CompletableFuture.completedFuture(file.error == null) } @@ -405,7 +413,9 @@ store.invalidate(outerClass) if (store.get(target) != javadoc) { store.setAll(mapOf(target to javadoc)) - LocalFileSystem.getInstance().refreshNioFiles(listOf(store.fileFor(outerClass))) + // Refreshing only the file wouldn't notice it if the VFS hasn't loaded it yet, such as when it's created + val mappingFile = store.fileFor(outerClass).toFile() + VfsUtil.markDirtyAndRefresh(false, false, true, mappingFile.parentFile, mappingFile) ApplicationManager.getApplication().messageBus.syncPublisher(AnnotaterListener.TOPIC) .javadocsChanged(checkout.root, outerClass) } @@ -453,15 +463,14 @@ if (wasCurrent) { project.putUserData(CURRENT_KEY, null) } - inlay?.let(Disposer::dispose) - inlay = null + saveJob?.cancel() + saveJob = null editor?.let(EditorFactory.getInstance()::releaseEditor) editor = null for (guard in guards) { document.removeGuardedBlock(guard) } guards.clear() - EditorActionManager.getInstance().setReadonlyFragmentModificationHandler(document, null) if (wasCurrent && !project.isDisposed) { restartHostHighlighting() EditorNotifications.getInstance(project).updateAllNotifications() @@ -470,7 +479,7 @@ } companion object { - private const val SAVE_DELAY_MS = 750 + private const val SAVE_DELAY_MS = 750L private val CURRENT_KEY = Key.create("mcdev.annotater.javadocInlineEditor") @@ -483,10 +492,12 @@ fun current(project: Project): AnnotaterJavadocInlineEditor? = project.getUserData(CURRENT_KEY) /** - * The parent of the inline editors of a project, so they're disposed with it. + * The parent of the inline editors of a project, so they're disposed with it, and the scope of their + * coroutines. Plugins shouldn't use the project itself as a parent disposable, as that leaks the plugin's + * classes if it's unloaded while the project is open. */ @Service(Service.Level.PROJECT) - private class ProjectDisposable : Disposable { + private class ProjectScope(val coroutineScope: CoroutineScope) : Disposable { override fun dispose() { } } @@ -521,7 +532,7 @@ return null } project.putUserData(CURRENT_KEY, inlineEditor) - Disposer.register(project.service(), inlineEditor) + Disposer.register(project.service(), inlineEditor) EditorNotifications.getInstance(project).updateAllNotifications() return inlineEditor } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocRefresher.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocRefresher.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocRefresher.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -25,7 +25,7 @@ import com.intellij.codeInsight.daemon.DaemonCodeAnalyzer import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.project.ProjectManager -import com.intellij.openapi.vfs.newvfs.BulkFileListener +import com.intellij.openapi.vfs.newvfs.BulkFileListenerBackgroundable import com.intellij.openapi.vfs.newvfs.events.VFileEvent import com.intellij.ui.EditorNotifications import java.nio.file.Path @@ -33,7 +33,7 @@ /** * Refreshes the places annotater Javadocs are displayed when they change, whether through the plugin or on disk. */ -class AnnotaterJavadocRefresher : AnnotaterListener, BulkFileListener { +class AnnotaterJavadocRefresher : AnnotaterListener, BulkFileListenerBackgroundable { override fun checkoutsChanged() { refresh(updateNotifications = true) } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocRendering.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocRendering.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocRendering.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -24,14 +24,10 @@ import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterCheckoutService import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterSettings import com.intellij.lang.java.JavaDocumentationProvider -import com.intellij.openapi.diagnostic.thisLogger -import com.intellij.psi.PsiClass import com.intellij.psi.PsiCompiledElement import com.intellij.psi.PsiJavaDocumentedElement -import com.intellij.psi.PsiJavaFile import com.intellij.psi.PsiMember import com.intellij.psi.util.PsiTreeUtil -import java.io.IOException /** * Renders annotater Javadocs with the IDE's own Java documentation renderer, by rendering the declaration in a copy of @@ -50,15 +46,6 @@ } /** - * All classes, fields and methods declared in [file] and its inner classes. - */ - fun displayedMembers(file: PsiJavaFile): Sequence = members(file.classes) - - private fun members(classes: Array): Sequence = classes.asSequence().flatMap { clazz -> - sequenceOf(clazz) + clazz.fields + clazz.methods + members(clazz.innerClasses) - } - - /** * The offset that the annotater Javadoc of [member] is displayed above: its declaration, after any existing * doc comment. */ @@ -68,15 +55,6 @@ ?: member.textRange.startOffset } - fun getJavadoc(checkout: AnnotaterCheckout, target: AnnotaterJavadocTarget): String? { - return try { - if (checkout.javadocStore.hasFile(target.outerClass)) checkout.javadocStore.get(target) else null - } catch (e: IOException) { - thisLogger().info("Failed to read annotater Javadoc of $target", e) - null - } - } - /** * Finds the declaration of [member] in a copy of its source file with [javadoc] as its doc comment. */ Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocSourceCopy.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocSourceCopy.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocSourceCopy.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -33,7 +33,6 @@ import com.intellij.psi.PsiManager import com.intellij.psi.PsiMember import com.intellij.psi.PsiWhiteSpace -import com.intellij.psi.impl.compiled.ClsElementImpl import com.intellij.psi.javadoc.PsiDocComment import com.intellij.psi.util.PsiTreeUtil import com.intellij.psi.util.PsiUtil @@ -44,13 +43,18 @@ * existing Javadocs replaced by the annotater Javadocs as Markdown doc comments. This lets the IDE's own Java support * be used to edit and render annotater Javadocs in their real context. * - * The member at index `i` of [members] documents `targets[i]`, or nothing if that is null, e.g. for members of other + * The member at index `i` of [AnnotaterJavadocTarget.declarations] documents `targets[i]`, or nothing if that is null, e.g. for members of other * top level classes in the same file. */ class AnnotaterJavadocSourceCopy( val fileName: String, val text: String, val targets: List, + /** + * The language level to parse the copy at: that of the source file, but at least the first that supports Markdown + * doc comments. + */ + val languageLevel: LanguageLevel, ) { /** * The internal name of the top level class whose Javadocs this copy edits. @@ -62,9 +66,8 @@ return PsiManager.getInstance(project).findFile(createVirtualFile()) as? PsiJavaFile } - fun createVirtualFile(): LightVirtualFile = LightVirtualFile(fileName, JavaFileType.INSTANCE, text).also { - // Markdown doc comments are only parsed at a high enough language level - it.putUserData(PsiUtil.FILE_LANGUAGE_LEVEL_KEY, LanguageLevel.HIGHEST) + private fun createVirtualFile(): LightVirtualFile = LightVirtualFile(fileName, JavaFileType.INSTANCE, text).also { + it.putUserData(PsiUtil.FILE_LANGUAGE_LEVEL_KEY, languageLevel) } /** @@ -72,14 +75,14 @@ */ fun memberOf(file: PsiJavaFile, target: AnnotaterJavadocTarget): PsiMember? { val index = targets.indexOf(target) - return if (index < 0) null else members(file).getOrNull(index) + return if (index < 0) null else AnnotaterJavadocTarget.declarations(file).getOrNull(index) } /** * Finds the target documented by [member] of [file], which must have been created from this copy. */ fun targetOf(file: PsiJavaFile, member: PsiMember): AnnotaterJavadocTarget? { - val index = members(file).indexOf(member) + val index = AnnotaterJavadocTarget.declarations(file).indexOf(member) return if (index < 0) null else targets.getOrNull(index) } @@ -109,7 +112,7 @@ } } - val members = members(file) + val members = AnnotaterJavadocTarget.declarations(file) if (members.size != targets.size) { return ExtractResult.Error(MCDevBundle("fabric.annotater.javadoc.error.structure_changed")) } @@ -124,21 +127,10 @@ companion object { /** - * Finds the classes, fields and methods declared in [file], in the order matching [targets]. Local and - * anonymous classes are not included, and neither are members that don't appear in the source, such as the - * implicit methods of records. - */ - fun members(file: PsiJavaFile): List = members(file.classes).toList() - - private fun members(classes: Array): Sequence = classes.asSequence().flatMap { clazz -> - sequenceOf(clazz) + clazz.fields + clazz.methods + members(clazz.innerClasses) - }.filter { it.node != null } - - /** * Extracts the Markdown text of a `///` doc comment, removing the `///` prefixes and the common indentation. * Returns null when the comment is blank. */ - fun extractMarkdown(comment: PsiDocComment): String? { + private fun extractMarkdown(comment: PsiDocComment): String? { val lines = comment.text.lines().map { line -> val trimmed = line.trimStart() if (trimmed.startsWith("///")) trimmed.substring(3) else trimmed @@ -153,7 +145,7 @@ /** * Formats Markdown as a `///` doc comment with the given indentation, ending with a newline. */ - fun formatMarkdown(markdown: String, indent: String): String { + private fun formatMarkdown(markdown: String, indent: String): String { return markdown.lines().joinToString("") { line -> if (line.isEmpty()) "$indent///\n" else "$indent/// $line\n" } @@ -163,7 +155,7 @@ * Formats Markdown as a `///` doc comment to insert at a column after [indent], followed by [indent] again for * the declaration after it. */ - fun formatInsertedMarkdown(markdown: String, indent: String): String { + private fun formatInsertedMarkdown(markdown: String, indent: String): String { // Leave a space after an empty first line, so the user can start typing there straight away return "/// " + formatMarkdown(markdown, indent).removePrefix("$indent///").removePrefix(" ") + indent } @@ -188,8 +180,9 @@ placeholderFor: AnnotaterJavadocTarget? = null, javadoc: (AnnotaterJavadocTarget) -> String?, ): AnnotaterJavadocSourceCopy? { - val outermost = generateSequence(element as? PsiClass ?: element.containingClass) { it.containingClass } - .lastOrNull() ?: return null + val outermost = PsiTreeUtil.getTopmostParentOfType(element, PsiClass::class.java) + ?: element as? PsiClass + ?: return null val outerClass = AnnotaterJavadocTarget.fromPsi(outermost)?.outerClass ?: return null val source = sourceFile(outermost) ?: return null val text = source.text @@ -197,10 +190,8 @@ val targets = mutableListOf() // The replacements of the existing doc comments, in text order val replacements = mutableListOf>() - for (member in members(source)) { - // Decompiled text is generated from the compiled class, which is more reliable to get the target of - val compiled = member.getUserData(ClsElementImpl.COMPILED_ELEMENT) ?: member - val target = AnnotaterJavadocTarget.fromPsi(compiled)?.takeIf { it.outerClass == outerClass } + for (member in AnnotaterJavadocTarget.declarations(source)) { + val target = AnnotaterJavadocTarget.fromPsi(member)?.takeIf { it.outerClass == outerClass } val existing = (member as PsiJavaDocumentedElement).docComment val start = existing?.textRange?.startOffset ?: member.textRange.startOffset val lineStart = text.lastIndexOf('\n', start - 1) + 1 @@ -227,7 +218,10 @@ for ((range, replacement) in replacements.sortedByDescending { it.first.startOffset }) { sb.replace(range.startOffset, range.endOffset, replacement) } - return AnnotaterJavadocSourceCopy("${outermost.name}.java", sb.toString(), targets) + // Need at least Java 23 to parse Markdown Javadocs + val languageLevel = PsiUtil.getLanguageLevel(source).takeIf { it.isAtLeast(LanguageLevel.JDK_23) } + ?: LanguageLevel.JDK_23 + return AnnotaterJavadocSourceCopy("${outermost.name}.java", sb.toString(), targets, languageLevel) } } } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocStore.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocStore.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocStore.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -20,6 +20,7 @@ package com.demonwav.mcdev.platform.fabric.annotater.javadoc +import com.intellij.openapi.diagnostic.thisLogger import java.io.IOException import java.io.StringReader import java.io.StringWriter @@ -33,6 +34,7 @@ import net.fabricmc.mappingio.MappedElementKind import net.fabricmc.mappingio.MappingUtil import net.fabricmc.mappingio.MappingVisitor +import net.fabricmc.mappingio.adapter.EmptyElementFilter import net.fabricmc.mappingio.adapter.ForwardingMappingVisitor import net.fabricmc.mappingio.format.enigma.EnigmaFileReader import net.fabricmc.mappingio.format.enigma.EnigmaFileWriter @@ -42,16 +44,17 @@ /** * Reads and writes the Javadoc comments stored in an annotater `mappings` directory. Comments are stored in Enigma - * format, with one file per top level class at `/.mapping`. The reading and writing - * matches mcsrc.dev so that files edited by either tool produce the same diffs. + * format, with one file per top level class at `/.mapping`. The files are written + * in the same way as annotater formats them, so that saving doesn't reformat unrelated parts of a file. + * + * Use [forDirectory] rather than the constructor outside of tests, so that every checkout of a directory sees the + * comments the others write. */ class AnnotaterJavadocStore(val mappingsDir: Path) { private val cache = ConcurrentHashMap() fun fileFor(outerClass: String): Path = mappingsDir.resolve("$outerClass.mapping") - fun hasFile(outerClass: String): Boolean = fileFor(outerClass).exists() - @Throws(IOException::class) fun get(target: AnnotaterJavadocTarget): String? { val clazz = treeFor(target.outerClass).getClass(target.owner) ?: return null @@ -63,6 +66,18 @@ } /** + * Like [get], but returns null if the `.mapping` file can't be read, for displaying Javadocs. + */ + fun getOrNull(target: AnnotaterJavadocTarget): String? { + return try { + if (fileFor(target.outerClass).exists()) get(target) else null + } catch (e: IOException) { + thisLogger().info("Failed to read annotater Javadoc of $target", e) + null + } + } + + /** * Sets the comment for [target], or removes it when [comment] is null or empty, and writes the `.mapping` file * of its outer class. The file is deleted when it no longer contains any comments. */ @@ -103,10 +118,6 @@ cache.remove(outerClass) } - fun invalidateAll() { - cache.clear() - } - @Throws(IOException::class) private fun treeFor(outerClass: String): MemoryMappingTree { cache[outerClass]?.let { return it } @@ -116,8 +127,27 @@ } companion object { + private val stores = ConcurrentHashMap() + + /** + * The store of [mappingsDir], shared by all of its checkouts. The checkout objects are replaced whenever the + * checkouts change, such as when a contribution branch is created on the first edit, but an editor started + * before that keeps saving through the old one. + */ + fun forDirectory(mappingsDir: Path): AnnotaterJavadocStore = + stores.computeIfAbsent(mappingsDir.normalize(), ::AnnotaterJavadocStore) + + /** + * Forgets the cached comments of every store, for when the checkouts change, such as by switching branch. + */ + fun invalidateAll() { + for (store in stores.values) { + store.cache.clear() + } + } + @Throws(IOException::class) - fun readFile(text: String, path: String, expectedOuterClass: String? = null): MemoryMappingTree { + private fun readFile(text: String, path: String, expectedOuterClass: String? = null): MemoryMappingTree { val tree = MemoryMappingTree() try { EnigmaFileReader.read(StringReader(text), withoutMethodVariables(tree)) @@ -138,7 +168,7 @@ } @Throws(IOException::class) - fun writeTree(tree: MemoryMappingTree): String { + private fun writeTree(tree: MemoryMappingTree): String { val meaningful = newTree() for (clazz in tree.classes) { if (isMeaningful(clazz)) { @@ -150,8 +180,9 @@ } val output = StringWriter() + // The same as annotater's own formatting EnigmaFileWriter(output).use { writer -> - meaningful.accept(writer, VisitOrder.createByName().classesBySrcNameShortFirst()) + meaningful.accept(EmptyElementFilter(writer), VisitOrder.createByName()) } return output.toString() } Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocTarget.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocTarget.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocTarget.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -22,10 +22,17 @@ import com.demonwav.mcdev.util.descriptor import com.demonwav.mcdev.util.internalName +import com.intellij.psi.PsiAnonymousClass import com.intellij.psi.PsiClass +import com.intellij.psi.PsiCompiledElement import com.intellij.psi.PsiElement import com.intellij.psi.PsiField +import com.intellij.psi.PsiJavaFile +import com.intellij.psi.PsiMember import com.intellij.psi.PsiMethod +import com.intellij.psi.PsiTypeParameter +import com.intellij.psi.SyntaxTraverser +import com.intellij.psi.impl.compiled.ClsElementImpl /** * A class, field or method that can have a Javadoc comment in annotater. All names are internal (slash separated) @@ -57,28 +64,26 @@ ) : AnnotaterJavadocTarget() companion object { + /** + * The target documented by [element], which may be an element of a class file's decompiled text. Anonymous + * classes can't be documented, but their members can. + */ fun fromPsi(element: PsiElement): AnnotaterJavadocTarget? { - return when (element) { - is PsiClass -> { - if (element.qualifiedName == null) { - // Anonymous and local classes can't be documented + // Decompiled text is generated from the compiled class, which is more reliable to get the target of + val compiled = element.getUserData(ClsElementImpl.COMPILED_ELEMENT) ?: element + if (compiled !is PsiMember || !isDocumentable(compiled)) { - return null - } + return null + } - ClassTarget(element.internalName ?: return null) - } - + return when (compiled) { + is PsiClass -> ClassTarget(compiled.internalName ?: return null) is PsiField -> { - val owner = element.containingClass?.takeIf { it.qualifiedName != null } ?: return null - FieldTarget(owner.internalName ?: return null, element.name, element.descriptor ?: return null) + val owner = compiled.containingClass?.internalName ?: return null + FieldTarget(owner, compiled.name, compiled.descriptor ?: return null) } is PsiMethod -> { - val owner = element.containingClass?.takeIf { it.qualifiedName != null } ?: return null - MethodTarget( - owner.internalName ?: return null, - element.internalName, - element.descriptor ?: return null - ) + val owner = compiled.containingClass?.internalName ?: return null + MethodTarget(owner, compiled.internalName, compiled.descriptor ?: return null) } else -> null @@ -86,6 +91,29 @@ } /** + * The classes, fields and methods declared in the source of [file] in text order, including those in local + * and anonymous classes, but not members that don't appear in the source, such as the implicit methods of + * records. For a class file, these are the declarations in its decompiled text. + */ + fun declarations(file: PsiJavaFile): List { + val source = (if (file is PsiCompiledElement) file.mirror else file) as? PsiJavaFile ?: return emptyList() + return SyntaxTraverser.psiTraverser(source) + .filter(PsiMember::class.java) + .filter(::isDocumentable) + .toList() + } + + /** + * Whether [member] is a kind of declaration that can be documented: a class, field or method, but not a type + * parameter or anonymous class (including enum constant bodies), which are also [PsiClass]es. + */ + private fun isDocumentable(member: PsiMember): Boolean = when (member) { + is PsiTypeParameter, is PsiAnonymousClass -> false + is PsiClass, is PsiField, is PsiMethod -> true + else -> false + } + + /** * Returns the top level class of a nested class name, treating `$` directly after a package separator or at * the start of a name as part of the class name, matching mcsrc. */ Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocUnqualifiedReferenceInspection.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocUnqualifiedReferenceInspection.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocUnqualifiedReferenceInspection.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -53,7 +53,7 @@ if (PsiTreeUtil.getParentOfType(reference, PsiDocComment::class.java) == null) { return } - val qualifiedName = qualifiedNameIfUnqualified(reference) ?: return + val qualifiedName = Util.qualifiedNameIfUnqualified(reference) ?: return holder.registerProblem( reference, MCDevBundle("fabric.annotater.inspection.unqualified.problem", qualifiedName), @@ -79,7 +79,7 @@ } } - companion object { + private object Util { /** * Returns the fully qualified name of the class [reference] refers to, if it isn't already written as such. */ Index: src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocVirtualFile.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocVirtualFile.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/AnnotaterJavadocVirtualFile.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -22,8 +22,6 @@ import com.demonwav.mcdev.platform.fabric.annotater.AnnotaterCheckout import com.intellij.ide.highlighter.JavaFileType -import com.intellij.openapi.vfs.VirtualFile -import com.intellij.pom.java.LanguageLevel import com.intellij.psi.PsiFile import com.intellij.psi.util.PsiUtil import com.intellij.testFramework.LightVirtualFile @@ -47,7 +45,7 @@ get() = copy.outerClass init { - putUserData(PsiUtil.FILE_LANGUAGE_LEVEL_KEY, LanguageLevel.HIGHEST) + putUserData(PsiUtil.FILE_LANGUAGE_LEVEL_KEY, copy.languageLevel) } companion object { Index: src/main/kotlin/platform/fabric/annotater/javadoc/EditAnnotaterJavadocAction.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/EditAnnotaterJavadocAction.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/EditAnnotaterJavadocAction.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -32,7 +32,7 @@ override fun update(e: AnActionEvent) { e.presentation.text = MCDevBundle("fabric.annotater.javadoc.edit") - e.presentation.icon = GeneralAssets.ANNOTATER + e.presentation.icon = GeneralAssets.TINY_POTATO val editor = e.getData(CommonDataKeys.EDITOR) val file = e.getData(CommonDataKeys.PSI_FILE) val project = e.project @@ -47,6 +47,6 @@ val project = e.project ?: return val editor = e.getData(CommonDataKeys.EDITOR) ?: return val file = e.getData(CommonDataKeys.PSI_FILE) ?: return - AnnotaterJavadocEditing.edit(project, editor, file) + AnnotaterJavadocEditing.open(project, editor, file) } } Index: src/main/kotlin/platform/fabric/annotater/javadoc/EditAnnotaterJavadocIntention.kt =================================================================== --- src/main/kotlin/platform/fabric/annotater/javadoc/EditAnnotaterJavadocIntention.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/platform/fabric/annotater/javadoc/EditAnnotaterJavadocIntention.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -45,7 +45,7 @@ if (editor == null || file == null) { return } - AnnotaterJavadocEditing.edit(project, editor, file) + AnnotaterJavadocEditing.open(project, editor, file) } override fun generatePreview(project: Project, editor: Editor, file: PsiFile): IntentionPreviewInfo = Index: src/main/kotlin/util/McdevReflection.kt =================================================================== --- src/main/kotlin/util/McdevReflection.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/kotlin/util/McdevReflection.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -22,10 +22,12 @@ import com.intellij.codeInsight.generation.ClassMember import com.intellij.codeInsight.generation.GenerateMembersHandlerBase +import com.intellij.codeInspection.LocalQuickFix import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project import com.intellij.openapi.updateSettings.impl.PluginDownloader import com.intellij.psi.PsiClass +import java.lang.reflect.Constructor import java.lang.reflect.Field import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method @@ -64,6 +66,14 @@ ) @JvmStatic + val CLASS_MARKDOWN_DOCUMENTATION_COMMENTS_MIGRATION_FIX: Class<*> = + Class.forName("com.siyeh.ig.migration.MarkdownDocumentationCommentsMigrationInspection\$MarkdownDocumentationCommentsMigrationFix") + + @JvmStatic + val CONSTRUCTOR_MARKDOWN_DOCUMENTATION_COMMENTS_MIGRATION_FIX: Constructor = + CLASS_MARKDOWN_DOCUMENTATION_COMMENTS_MIGRATION_FIX.getDeclaredConstructorEx() + + @JvmStatic val FIELD_PLUGIN_DOWNLOADER_MY_FILE: Field = PluginDownloader::class.java.getDeclaredFieldEx("myFile", Path::class.java) @@ -80,6 +90,18 @@ } /** + * Like [Class.getDeclaredConstructor], but makes the constructor accessible. + */ + private inline fun Class<*>.getDeclaredConstructorEx(vararg params: Class<*>): Constructor { + check(T::class.java.isAssignableFrom(this)) { + "Class ${this.name} is not a subclass of ${T::class.java.name}" + } + val constructor = this.asSubclass(T::class.java).getDeclaredConstructor(*params) + constructor.isAccessible = true + return constructor + } + + /** * Like [Class.getDeclaredMethod], but makes the method accessible and checks that it has the expected [returnType]. */ private fun Class<*>.getDeclaredMethodEx(name: String, returnType: Class<*>, vararg params: Class<*>): Method { @@ -92,6 +114,18 @@ } /** + * Like [Constructor.newInstance], but rethrows the exception thrown by the invoked constructor rather than wrapping + * it in an [InvocationTargetException]. + */ + fun Constructor.newInstanceUnwrap(vararg args: Any?): T { + try { + return newInstance(*args) + } catch (e: InvocationTargetException) { + throw e.cause ?: e + } + } + + /** * Like [Method.invoke], but rethrows the exception thrown by the invoked method rather than wrapping it in an * [InvocationTargetException]. */ Index: src/main/resources/META-INF/plugin.xml =================================================================== --- src/main/resources/META-INF/plugin.xml (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/resources/META-INF/plugin.xml (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -1722,7 +1722,7 @@ + topic="com.intellij.openapi.vfs.newvfs.BulkFileListenerBackgroundable"/> Index: src/main/resources/assets/icons/general/Annotater.LICENSE.txt =================================================================== --- src/main/resources/assets/icons/general/Annotater.LICENSE.txt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/resources/assets/icons/general/Annotater.LICENSE.txt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) @@ -1,3 +0,0 @@ -Annotater.png, Annotater@2x.png, AnnotaterLarge.png and AnnotaterLarge@2x.png are downscaled from -https://github.com/FabricMC/community/blob/main/media/tater-wall/png/potato.png by FabricMC, licensed under the -Creative Commons Attribution 4.0 International license (https://creativecommons.org/licenses/by/4.0/). Index: src/main/resources/assets/icons/general/tiny_potato.LICENSE.txt =================================================================== --- src/main/resources/assets/icons/general/tiny_potato.LICENSE.txt (revision 31ff07e672a89f38cf405960582f13213d9af620) +++ src/main/resources/assets/icons/general/tiny_potato.LICENSE.txt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -0,0 +1,3 @@ +tiny_potato.png, tiny_potato@2x.png, tiny_potato_large.png and tiny_potato_large@2x.png are downscaled from +https://github.com/FabricMC/community/blob/main/media/tater-wall/png/potato.png by FabricMC, licensed under the +Creative Commons Attribution 4.0 International license (https://creativecommons.org/licenses/by/4.0/). Index: src/main/resources/intentionDescriptions/EditAnnotaterJavadocIntention/after.java.template =================================================================== --- src/main/resources/intentionDescriptions/EditAnnotaterJavadocIntention/after.java.template (revision 31ff07e672a89f38cf405960582f13213d9af620) +++ src/main/resources/intentionDescriptions/EditAnnotaterJavadocIntention/after.java.template (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -0,0 +1,6 @@ +/// Clamps a value between a minimum and a maximum. +/// +/// @return the clamped value +public static int clamp(int value, int min, int max) { + return Math.min(Math.max(value, min), max); +} Index: src/main/resources/intentionDescriptions/EditAnnotaterJavadocIntention/before.java.template =================================================================== --- src/main/resources/intentionDescriptions/EditAnnotaterJavadocIntention/before.java.template (revision 31ff07e672a89f38cf405960582f13213d9af620) +++ src/main/resources/intentionDescriptions/EditAnnotaterJavadocIntention/before.java.template (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -0,0 +1,3 @@ +public static int clamp(int value, int min, int max) { + return Math.min(Math.max(value, min), max); +} Index: src/main/resources/messages/MinecraftDevelopment.properties =================================================================== --- src/main/resources/messages/MinecraftDevelopment.properties (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/main/resources/messages/MinecraftDevelopment.properties (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -406,6 +406,9 @@ fabric.annotater.settings.title=Annotater fabric.annotater.settings.enabled=Enable annotater Javadoc editing fabric.annotater.settings.enabled.comment=Edit Minecraft Javadocs for annotater with the Edit Annotater Javadoc action on Minecraft classes, fields and methods, or the annotater gutter icon on declarations that already have an annotater Javadoc. +fabric.annotater.settings.guidelines=Please follow annotater''s {0} when writing Javadocs and opening pull requests. +fabric.annotater.settings.guidelines.link=contribution guidelines +fabric.annotater.settings.guidelines.link.version=contribution guidelines for {0} fabric.annotater.settings.show_javadocs=Show annotater Javadocs instead of existing Minecraft Javadocs fabric.annotater.settings.show_javadocs.comment=Applies to quick documentation, decompiled classes and attached Minecraft sources. Rendered doc comments (reader mode) are turned off in Minecraft classes. fabric.annotater.settings.managed=Clone annotater automatically @@ -434,6 +437,7 @@ fabric.annotater.preparing=Preparing annotater checkout fabric.annotater.preparing.failed=Failed to prepare the annotater checkout fabric.annotater.create_pull_request=Create Pull Request... +fabric.annotater.update_pull_request=Update Pull Request... fabric.annotater.dropdown={0} \u25BE fabric.annotater.menu=Annotater fabric.annotater.contribution.tooltip=Switch to another contribution, for example to update a pull request after review @@ -443,6 +447,15 @@ fabric.annotater.contribution.failed=Failed to switch annotater contribution fabric.annotater.contribution.save_failed=Fix the errors in the open annotater editors before switching contribution. fabric.annotater.contribution.commit=The changes to "{0}" will be committed before switching. Commit message\: +fabric.annotater.contribution.delete=Delete Contribution +fabric.annotater.contribution.delete.title=Delete Contribution +fabric.annotater.contribution.delete.ok=Delete +fabric.annotater.contribution.delete.confirm=Delete the contribution "{0}"? It is only deleted locally. Any branch pushed to GitHub and any pull request for it are kept. +fabric.annotater.contribution.delete.confirm.current=This contribution is checked out, so its uncommitted changes will be lost. +fabric.annotater.contribution.deleting=Deleting annotater contribution +fabric.annotater.contribution.deleted=Annotater contribution deleted +fabric.annotater.contribution.deleted.content=Deleted the contribution "{0}". +fabric.annotater.contribution.delete.failed=Failed to delete annotater contribution fabric.annotater.status.disabled=Annotater is disabled in the Minecraft Development settings. fabric.annotater.status.unknown_version=The Minecraft version of this project is unknown. fabric.annotater.status.not_cloned=Annotater has not been cloned for Minecraft {0} yet. @@ -452,7 +465,6 @@ fabric.annotater.javadoc.edit=Edit Annotater Javadoc fabric.annotater.javadoc.open_failed=Failed to open annotater Javadoc -fabric.annotater.javadoc.read_only=Only the Javadoc comment can be edited. fabric.annotater.banner=Annotater Javadocs for Minecraft {0} from {1}. Use Edit Annotater Javadoc on a class, field or method to edit its Javadoc. Changes are saved automatically. fabric.annotater.javadoc.finish=Finish editing fabric.annotater.javadoc.save_failed=Failed to save annotater Javadoc @@ -474,16 +486,25 @@ fabric.annotater.pr.failed=Failed to create annotater pull request fabric.annotater.pr.no_base_branch={0} has no {1} branch to open a pull request against. fabric.annotater.pr.creating=Creating annotater pull request +fabric.annotater.pr.updating=Updating annotater pull request fabric.annotater.pr.committing=Committing changes... fabric.annotater.pr.forking=Forking annotater... fabric.annotater.pr.pushing=Pushing changes... fabric.annotater.pr.opening=Opening pull request... fabric.annotater.pr.created=Annotater pull request created +fabric.annotater.pr.updated=Annotater pull request updated fabric.annotater.pr.open=Open in Browser fabric.annotater.pr.start_new=Start New Contribution fabric.annotater.pr.dialog.ok=Create Pull Request +fabric.annotater.pr.dialog.update=Update Pull Request fabric.annotater.pr.dialog.account=GitHub account\: fabric.annotater.pr.dialog.title=Title\: fabric.annotater.pr.dialog.description=Description\: fabric.annotater.pr.dialog.commit_message=Commit message\: fabric.annotater.pr.dialog.changes=Changed files\:
{0} +fabric.annotater.pr.dialog.check_build=Check the build first +fabric.annotater.pr.dialog.check_build.comment=Runs gradlew check in the annotater checkout, which can take a minute. +fabric.annotater.pr.check_build.progress=Checking the annotater build +fabric.annotater.pr.check_build.failed=Annotater Build Check Failed +fabric.annotater.pr.check_build.failed.message=The annotater build check failed. Fix the problems below, or submit anyway. +fabric.annotater.pr.check_build.submit_anyway=Submit Anyway Index: src/test/kotlin/platform/fabric/annotater/AnnotaterJavadocEditorTest.kt =================================================================== --- src/test/kotlin/platform/fabric/annotater/AnnotaterJavadocEditorTest.kt (revision 8b7df1d2f39176871145907ee04ba4c74af56cf8) +++ src/test/kotlin/platform/fabric/annotater/AnnotaterJavadocEditorTest.kt (revision 31ff07e672a89f38cf405960582f13213d9af620) @@ -30,9 +30,9 @@ import com.demonwav.mcdev.platform.fabric.annotater.javadoc.AnnotaterJavadocUnqualifiedReferenceInspection import com.demonwav.mcdev.platform.fabric.annotater.javadoc.AnnotaterJavadocVirtualFile import com.intellij.codeInsight.intention.IntentionAction +import com.intellij.codeInsight.lookup.Lookup import com.intellij.ide.DataManager import com.intellij.ide.impl.HeadlessDataManager -import com.intellij.codeInsight.lookup.Lookup import com.intellij.lang.annotation.HighlightSeverity import com.intellij.openapi.actionSystem.PlatformCoreDataKeys import com.intellij.openapi.command.WriteCommandAction @@ -97,6 +97,25 @@ """.trimIndent(), ) java( + "net/minecraft/util/Ticker.java", + """ + package net.minecraft.util; + + public class Ticker { + public void tick() { + class Step { + int count; + } + Runnable runnable = new Runnable() { + @Override + public void run() { + } + }; + } + } + """.trimIndent(), + ) + java( "net/minecraft/world/level/Level.java", """ package net.minecraft.world.level; @@ -146,10 +165,28 @@ copy.text, ) assertEquals("Mth.java", copy.fileName) - assertEquals(listOf(AnnotaterJavadocTarget.ClassTarget("net/minecraft/util/Mth"), pi, clamp), copy.targets) + assertEquals(listOf(AnnotaterJavadocTarget.ClassTarget("net/minecraft/util/Mth"), clamp, pi), copy.targets) } @Test + @DisplayName("Local classes and members of local and anonymous classes can be documented") + fun localAndAnonymousClasses() { + val copy = AnnotaterJavadocSourceCopy.create(findMember("net.minecraft.util.Ticker")) { null }!! + assertEquals( + listOf( + AnnotaterJavadocTarget.ClassTarget("net/minecraft/util/Ticker"), + AnnotaterJavadocTarget.MethodTarget("net/minecraft/util/Ticker", "tick", "()V"), + AnnotaterJavadocTarget.ClassTarget("net/minecraft/util/Ticker\$1Step"), + AnnotaterJavadocTarget.FieldTarget("net/minecraft/util/Ticker\$1Step", "count", "I"), + AnnotaterJavadocTarget.MethodTarget("net/minecraft/util/Ticker\$1", "run", "()V"), + ), + // Type parameters and anonymous classes are also PsiClasses, but aren't declarations that can be documented + copy.targets, + ) + assertEquals("net/minecraft/util/Ticker", copy.outerClass) + } + + @Test @DisplayName("Source copy adds a placeholder Javadoc to the edited member") fun sourceCopyPlaceholder() { val copy = clampCopy(null, placeholderFor = clamp) @@ -185,6 +222,7 @@ copy.fileName, copy.text.replace("/// Clamps a value.", "/** Clamps a value. */"), copy.targets, + copy.languageLevel, ) val result = changed.extractJavadocs(changed.createPsiFile(project)!!) assertInstanceOf(AnnotaterJavadocSourceCopy.ExtractResult.Error::class.java, result)