diff --git a/plugins/markdown/figmaAdvertiser/resources/intellij.markdown.figmaAdvertiser.xml b/plugins/markdown/figmaAdvertiser/resources/intellij.markdown.figmaAdvertiser.xml index e811c837124c..6ab4b0a32f40 100644 --- a/plugins/markdown/figmaAdvertiser/resources/intellij.markdown.figmaAdvertiser.xml +++ b/plugins/markdown/figmaAdvertiser/resources/intellij.markdown.figmaAdvertiser.xml @@ -19,6 +19,13 @@ + + + diff --git a/plugins/markdown/figmaAdvertiser/src/com/intellij/markdown/figmaAdvertiser/FigmaLinkDocumentListener.kt b/plugins/markdown/figmaAdvertiser/src/com/intellij/markdown/figmaAdvertiser/FigmaLinkDocumentListener.kt new file mode 100644 index 000000000000..a84a9581f556 --- /dev/null +++ b/plugins/markdown/figmaAdvertiser/src/com/intellij/markdown/figmaAdvertiser/FigmaLinkDocumentListener.kt @@ -0,0 +1,53 @@ +package com.intellij.markdown.figmaAdvertiser + +import com.intellij.openapi.editor.event.DocumentEvent +import com.intellij.openapi.editor.event.DocumentListener +import com.intellij.openapi.fileEditor.FileDocumentManager +import com.intellij.openapi.project.ProjectLocator +import com.intellij.ui.EditorNotifications +import org.jetbrains.annotations.ApiStatus + +/** + * Asks the platform to recompute the banner over a Markdown file whose text has just gained a link + * to a Figma design. + * + * `EditorNotificationsImpl` recomputes when a file is opened, when dumb mode starts or ends, when + * the roots change and when a plugin is loaded or unloaded + * (`EditorNotificationsImpl.kt:97-132`). Editing a file is none of those, so without this the offer + * over a file that is already open waits for the file to be opened again. + * + * Every document in the IDE reaches [documentChanged], which is why the questions are asked in the + * order below: two map reads and an extension check answer a keystroke that has nothing to do with + * Markdown, and the scan that follows reads a window around the change rather than the file. + */ +// `SplitModeApiUsage` asks for a frontend module, and this one is shared. What the listener does with +// the document is read the text around a change, and what it then asks for is a recompute of one +// file's notifications, which does nothing wherever no editor shows that file. +@Suppress("SplitModeApiUsage") +@ApiStatus.Internal +class FigmaLinkDocumentListener : DocumentListener { + + override fun documentChanged(event: DocumentEvent) { + if (!FigmaAdvertiserRegistry.isAdvertiserEnabled) return + val file = FileDocumentManager.getInstance().getFile(event.document) ?: return + if (!isMarkdownSuggestionFile(file.path)) return + + // Nothing below runs unless a Figma link meets the change itself, so an edit beside a link that + // was already there costs nothing and a link typed character by character is answered once. The + // link here is the matched prefix and stops at `/design/`, so an edit inside that prefix asks + // again and an edit in the file key after it does not. The platform coalesces the recompute per + // file (`EditorNotificationsImpl.kt:250-334`). + if (!changeTouchesFigmaUrl(event.document.immutableCharSequence, event.offset, event.offset + event.newLength)) return + if (isFigmaConnectLoaded()) return + + // Every open project the file belongs to. A file outside every content root is edited without a + // banner until it is opened again, which is the same answer the platform gives such a file for + // everything else it indexes. + for (project in ProjectLocator.getInstance().getProjectsForFile(file)) { + if (project.isDisposed || isFigmaSuggestionDismissed(project)) continue + // A project showing no editor for this file is answered by `updateNotifications` itself, which + // stops as soon as it finds none. + EditorNotifications.getInstance(project).updateNotifications(file) + } + } +} diff --git a/plugins/markdown/figmaAdvertiser/src/com/intellij/markdown/figmaAdvertiser/FigmaSuggestionDecision.kt b/plugins/markdown/figmaAdvertiser/src/com/intellij/markdown/figmaAdvertiser/FigmaSuggestionDecision.kt index 1be26a4275bd..6de9e27c3d1e 100644 --- a/plugins/markdown/figmaAdvertiser/src/com/intellij/markdown/figmaAdvertiser/FigmaSuggestionDecision.kt +++ b/plugins/markdown/figmaAdvertiser/src/com/intellij/markdown/figmaAdvertiser/FigmaSuggestionDecision.kt @@ -1,5 +1,6 @@ package com.intellij.markdown.figmaAdvertiser +import com.intellij.ide.plugins.PluginManager import org.jetbrains.annotations.ApiStatus /** @@ -37,6 +38,42 @@ fun isMarkdownSuggestionFile(filePath: String): Boolean { @ApiStatus.Internal fun containsFigmaUrl(text: CharSequence): Boolean = FIGMA_URL_PATTERN.containsMatchIn(text) +/** + * Whether a link to a Figma file in [text] meets the change over `[changeStart, changeEnd)`. + * + * A match that is there after a change and was not there before it holds at least one character the + * change wrote, and a match a pure deletion produced spans the position the deletion left behind. + * So a link the change created always meets the change, and a link that was already there and was + * not touched does not. Answering the wider question — a link anywhere near the change — would say + * yes to every keystroke within [FIGMA_URL_MAX_MATCH] characters of a link the author wrote + * yesterday. + * + * [FIGMA_URL_PATTERN] matches at most [FIGMA_URL_MAX_MATCH] characters, so a match that meets the + * change lies inside the changed range grown by that many characters at each end. Searching that + * window keeps an edit in a long Markdown file as cheap as an edit in a short one. + */ +@ApiStatus.Internal +fun changeTouchesFigmaUrl(text: CharSequence, changeStart: Int, changeEnd: Int): Boolean { + val from = (changeStart - FIGMA_URL_MAX_MATCH).coerceIn(0, text.length) + val to = (changeEnd + FIGMA_URL_MAX_MATCH).coerceIn(from, text.length) + // A deletion writes no character, so it marks the single position it left behind. + val touchedEnd = maxOf(changeEnd, changeStart + 1) + return FIGMA_URL_PATTERN.findAll(text.subSequence(from, to)).any { match -> + from + match.range.first < touchedEnd && changeStart < from + match.range.last + 1 + } +} + +/** + * Whether Figma Connect is loaded. + * + * [FigmaConnectPluginSuggestionProvider] is given this exclusion by `buildSuggestionIfNeeded`, which + * drops every plugin id already in `PluginManager.getLoadedPlugins()`. A caller that does not reach + * that call asks here. + */ +@ApiStatus.Internal +fun isFigmaConnectLoaded(): Boolean = + PluginManager.getLoadedPlugins().any { it.pluginId.idString == FIGMA_CONNECT_PLUGIN_ID } + /** * Lower case. A file system keeps the case a user typed, and `README.MD` names the same extension. * @@ -54,6 +91,13 @@ private val MARKDOWN_EXTENSIONS: Set = setOf("md", "markdown", "mdc") private val FIGMA_URL_PATTERN: Regex = Regex("""https?://(?:www\.)?figma\.com/(?:file|design|proto)/""", RegexOption.IGNORE_CASE) +/** + * A bound on the length of a string [FIGMA_URL_PATTERN] matches. The longest one is + * `https://www.figma.com/design/`, at 29 characters; the bound is set well above it so that a URL + * shape added to the pattern has room before the bound has to move with it. + */ +private const val FIGMA_URL_MAX_MATCH: Int = 64 + /** The plugin this module advertises. Owned by `plugins/figma/resources/META-INF/plugin.xml:2`. */ @ApiStatus.Internal const val FIGMA_CONNECT_PLUGIN_ID: String = "com.intellij.figma" diff --git a/plugins/markdown/test/src/org/intellij/plugins/markdown/figmaAdvertiser/MarkdownFigmaAdvertiserBannerTest.kt b/plugins/markdown/test/src/org/intellij/plugins/markdown/figmaAdvertiser/MarkdownFigmaAdvertiserBannerTest.kt index f579fe1bfee7..cb100f3a8c0c 100644 --- a/plugins/markdown/test/src/org/intellij/plugins/markdown/figmaAdvertiser/MarkdownFigmaAdvertiserBannerTest.kt +++ b/plugins/markdown/test/src/org/intellij/plugins/markdown/figmaAdvertiser/MarkdownFigmaAdvertiserBannerTest.kt @@ -17,6 +17,7 @@ import com.intellij.markdown.figmaAdvertiser.FigmaConnectPluginSuggestionProvide import com.intellij.markdown.figmaAdvertiser.FigmaSuggestionOffer import com.intellij.markdown.figmaAdvertiser.MarkdownFigmaAdvertiserBundle import com.intellij.markdown.figmaAdvertiser.ShownSuggestions +import com.intellij.markdown.figmaAdvertiser.dismissFigmaSuggestion import com.intellij.markdown.figmaAdvertiser.isFigmaSuggestionDismissed import com.intellij.openapi.Disposable import com.intellij.openapi.application.ApplicationManager @@ -29,6 +30,7 @@ import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.registry.Registry import com.intellij.openapi.updateSettings.impl.pluginsAdvertisement.PluginSuggestion import com.intellij.openapi.updateSettings.impl.pluginsAdvertisement.PluginSuggestionProvider +import com.intellij.openapi.vfs.VirtualFile import com.intellij.platform.pluginSystem.testFramework.PluginSetTestBuilder import com.intellij.platform.testFramework.loadPluginWithText import com.intellij.platform.testFramework.plugins.dependsIntellijModulesLang @@ -38,8 +40,11 @@ import com.intellij.psi.PsiFile import com.intellij.testFramework.fixtures.BasePlatformTestCase import com.intellij.testFramework.replaceService import com.intellij.ui.EditorNotificationPanel +import com.intellij.ui.EditorNotificationProvider +import com.intellij.ui.EditorNotifications import io.kotest.matchers.booleans.shouldBeFalse import io.kotest.matchers.booleans.shouldBeTrue +import io.kotest.matchers.collections.shouldBeEmpty import io.kotest.matchers.collections.shouldHaveSize import io.kotest.matchers.nulls.shouldBeNull import io.kotest.matchers.nulls.shouldNotBeNull @@ -286,6 +291,91 @@ class MarkdownFigmaAdvertiserBannerTest : BasePlatformTestCase() { enabler.enabled shouldBe listOf(FIGMA_CONNECT_PLUGIN_ID) } + /** + * The platform recomputes an editor banner when a file is opened and not while it is edited, so a + * link written into a file that is already open was answered only by opening the file again. + * + * The URL is typed one character at a time, which is what `type` does. So the recompute is asked + * for by the keystroke that finishes the link, and a paste large enough to carry the link whole is + * not what makes this pass. + * + * **The list is asserted whole, and its length is the point.** Forty-eight keystrokes ask once, + * because a link has to meet the change and not merely lie near it. A `distinct()` here would + * read the same for one ask and for sixteen. + */ + fun `test a Figma link typed into an open Markdown file asks for a recompute`() { + val note = myFixture.configureByText("notes.md", "# Checkout\n") + // Replaced after the file is opened, so what is recorded is what the editing asked for. + val notifications = recordNotifications() + + myFixture.type("See https://www.figma.com/design/AbC123/Checkout") + + notifications.updated shouldBe listOf(note.virtualFile) + } + + /** + * An edit beside a link that was already there writes no link, and the offer over that file was + * answered when it was opened. Asking again would charge the platform a recompute per keystroke + * for the life of the file. + */ + fun `test typing beside a link that is already there asks for nothing`() { + myFixture.configureByText("notes.md", "The spec is at https://www.figma.com/design/AbC123/Checkout\n") + val notifications = recordNotifications() + myFixture.editor.caretModel.moveToOffset(myFixture.file.textLength) + + myFixture.type("Ask Ada about the header.") + + notifications.updated.shouldBeEmpty() + } + + /** + * Every document in the IDE reaches the listener, so the text a user types in a Markdown file + * costs the platform nothing until it names a design. + */ + fun `test typing text that links to nothing asks for no recompute`() { + myFixture.configureByText("notes.md", "# Checkout\n") + val notifications = recordNotifications() + + myFixture.type("We talked about the design in the meeting.") + + notifications.updated.shouldBeEmpty() + } + + /** The path decides first here as well: a file Markdown does not claim is never scanned. */ + fun `test a Figma link typed into a file that is not Markdown asks for no recompute`() { + myFixture.configureByText("notes.txt", "# Checkout\n") + val notifications = recordNotifications() + + myFixture.type("See https://www.figma.com/design/AbC123/Checkout") + + notifications.updated.shouldBeEmpty() + } + + /** + * Tells a switched-off advertisement from an edit it has nothing to say about. The same typing + * asks for a recompute in the case above, so what changed is the key. + */ + fun `test no recompute is asked for while the advertiser is switched off`() { + myFixture.configureByText("notes.md", "# Checkout\n") + val notifications = recordNotifications() + Registry.get(FigmaAdvertiserRegistry.KEY_ADVERTISER_ENABLED).setValue(false, testRootDisposable) + + myFixture.type("See https://www.figma.com/design/AbC123/Checkout") + + notifications.updated.shouldBeEmpty() + } + + /** The answer the user already gave is not re-asked by their next keystroke. */ + fun `test no recompute is asked for on a project the offer is dismissed for`() { + myFixture.configureByText("notes.md", "# Checkout\n") + val notifications = recordNotifications() + dismissFigmaSuggestion(project) + + myFixture.type("See https://www.figma.com/design/AbC123/Checkout") + + notifications.updated.shouldBeEmpty() + } + /** The dismissal records what the user answered and nothing else. */ fun `test the dismissed event names the Markdown trigger`() { val panel = bannerOver(designNote()) @@ -323,6 +413,25 @@ class MarkdownFigmaAdvertiserBannerTest : BasePlatformTestCase() { } } + /** Puts a recording [EditorNotifications] on the project and answers it. */ + private fun recordNotifications(): RecordingEditorNotifications = RecordingEditorNotifications().also { + project.replaceService(EditorNotifications::class.java, it, testRootDisposable) + } + + /** Records which files something asked the platform to recompute the notifications for. */ + private class RecordingEditorNotifications : EditorNotifications() { + val updated: MutableList = mutableListOf() + + override fun updateNotifications(file: VirtualFile) { + updated += file + } + + @Suppress("OVERRIDE_DEPRECATION") + override fun updateNotifications(provider: EditorNotificationProvider) = Unit + + override fun updateAllNotifications() = Unit + } + private fun offerForTest(): FigmaSuggestionOffer = FigmaSuggestionOffer( project, FigmaAdvertiserUsagesCollector.SuggestionTrigger.MARKDOWN_FIGMA_LINK,