mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-27 10:03:11 +07:00
WEB-79599 markdown: notice a Figma link added to an open file
The banner over a Markdown file was answered only when the file was opened. EditorNotificationsImpl recomputes on a file being opened, on dumb mode, on a root change and on a plugin loading or unloading (EditorNotificationsImpl.kt:97-132). Editing a file is none of those, so a link added to an open README waited for the file to be opened again. FigmaLinkDocumentListener is registered on com.intellij.editorFactoryDocumentListener and asks EditorNotifications to recompute that one file. Every document in the IDE reaches it, so the questions are ordered by cost: the registry switch, then the file's extension, then a search. The search reads a window around the change and not the file, and it keeps a match only when the match meets the change. A link the change created always does: a match that is there after a change and was not there before it holds a character the change wrote, and a match a pure deletion produced spans the position the deletion left behind. The pattern matches at most 29 characters, so such a match lies within FIGMA_URL_MAX_MATCH of the change. The overlap is what makes the ask one per link. Asking for every change with a link merely near it charges one recompute per keystroke for the life of a file that holds a link, and the platform's own coalescing sits after an EDT dispatch, so it would coalesce the recompute and not the dispatch. ProjectLocator.getProjectsForFile answers which projects to ask. It keeps a project whose file index reports the file as in content or as excluded, so a file under no project root keeps the old behaviour and is answered when it is opened. The suppression of SplitModeApiUsage is written down at the class: the module is shared, and the listener reads a document and asks for one file's notifications. MarkdownFigmaAdvertiserBannerTest gains six cases. The URL is typed one character at a time, and the recorded list is asserted whole, so forty-eight keystrokes have to produce exactly one ask. The module reports 26 tests, 26 passed. Claude-Session: https://claude.ai/code/session_01ELVY5UTnoFX4drhGBHRVXK (cherry picked from commit 7cce24e103a18a6891a0b0fe70e2179d99945154) GitOrigin-RevId: 66ca03042d6642d5e9520592753e1da4126e64c6
This commit is contained in:
Ilya Muradyan
committed by
intellij-monorepo-bot
intellij-monorepo-bot
parent
d197caf807
commit
ea0da08563
@@ -19,6 +19,13 @@
|
||||
<pluginSuggestionProvider
|
||||
implementation="com.intellij.markdown.figmaAdvertiser.FigmaConnectPluginSuggestionProvider"/>
|
||||
|
||||
<!--
|
||||
The platform recomputes an editor banner when a file is opened and not while it is edited, so
|
||||
a Figma link added to an open file is answered by this listener rather than by the next open.
|
||||
-->
|
||||
<editorFactoryDocumentListener
|
||||
implementation="com.intellij.markdown.figmaAdvertiser.FigmaLinkDocumentListener"/>
|
||||
|
||||
<statistics.counterUsagesCollector
|
||||
implementationClass="com.intellij.markdown.figmaAdvertiser.FigmaAdvertiserUsagesCollector"/>
|
||||
</extensions>
|
||||
|
||||
+53
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
+44
@@ -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<String> = 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"
|
||||
|
||||
+109
@@ -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<VirtualFile> = 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,
|
||||
|
||||
Reference in New Issue
Block a user