refactor: handle filtering items in a decent manner. See comments in …

…code

src/main/kotlin/com/github/lppedd/cc/completion/CommitCompletionContributor.kt

@@ -18,20 +18,28 @@ import com.intellij.codeInsight.completion.*
 import com.intellij.codeInsight.completion.CompletionType.BASIC
 import com.intellij.codeInsight.completion.impl.CompletionSorterImpl
 import com.intellij.codeInsight.completion.impl.PreferStartMatching
+import com.intellij.codeInsight.lookup.Lookup
 import com.intellij.codeInsight.lookup.LookupElement
 import com.intellij.codeInsight.lookup.LookupElementWeigher
+import com.intellij.codeInsight.lookup.impl.LookupImpl
+import com.intellij.openapi.Disposable
 import com.intellij.openapi.fileTypes.PlainTextLanguage
 import com.intellij.openapi.progress.ProgressManager
+import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.vcs.ui.CommitMessage
 import com.intellij.patterns.PlatformPatterns
 import com.intellij.util.ReflectionUtil.findField
 import com.intellij.util.concurrency.Semaphore
 import org.jetbrains.annotations.ApiStatus
 import java.lang.reflect.Field
+import java.util.*
+import java.util.Collections.synchronizedMap
 import kotlin.LazyThreadSafetyMode.PUBLICATION
 import kotlin.internal.InlineOnly
 
 private val PLAIN_TEXT_PATTERN = PlatformPatterns.psiElement().withLanguage(PlainTextLanguage.INSTANCE)
+private val MENU_ENHANCER_MAP = synchronizedMap(IdentityHashMap<Lookup, MenuEnhancerLookupListener>(16))
+private val LOOKUP_DISPOSER_MAP = synchronizedMap(IdentityHashMap<Lookup, Disposable>(16))
 
 /**
  * Provides context-based completion items inside the VCS commit dialog.
@@ -125,8 +133,8 @@ private open class CommitCompletionContributor : CompletionContributor() {
         safelySetNoopListOnLookupArranger(process)
         safelyReleaseProcessSemaphore(process)
 
-        val filterableProviders = providers.flatMap { it.providers.take(3) }.take(6)
-        MenuEnhancerLookupListener(process.lookup).setProviders(filterableProviders)
+        val menuEnhancer = installAndGetMenuEnhancer(process.lookup)
+        menuEnhancer?.setProviders(providers.flatMap { it.providers.take(3) }.take(6))
       }
     }
 
@@ -146,6 +154,21 @@ private open class CommitCompletionContributor : CompletionContributor() {
       .withClassifier(CompletionSorterImpl.weighingFactory(weigher))
   }
 
+  private fun installAndGetMenuEnhancer(lookup: LookupImpl): MenuEnhancerLookupListener? {
+    val disposable = LOOKUP_DISPOSER_MAP.computeIfAbsent(lookup) {
+      Disposable {
+        LOOKUP_DISPOSER_MAP.remove(lookup)
+        MENU_ENHANCER_MAP.remove(lookup)
+      }
+    }
+
+    if (Disposer.findRegisteredObject(lookup, disposable) == null) {
+      Disposer.register(lookup, disposable)
+    }
+
+    return MENU_ENHANCER_MAP.computeIfAbsent(lookup) { MenuEnhancerLookupListener(lookup) }
+  }
+
   /**
    * This is a workaround for a standard behavior which stores some elements
    * on the `CompletionLookupArrangerImpl#myFrozenItems` list.

src/main/kotlin/com/github/lppedd/cc/completion/filter/FilterProviderAction.kt

@@ -8,6 +8,7 @@ import com.intellij.codeInsight.lookup.impl.LookupImpl
 import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
 import org.jetbrains.annotations.ApiStatus
+import java.util.*
 
 /**
  * @author Edoardo Luppi
@@ -83,4 +84,10 @@ internal class FilterProviderAction(
 
     backupItems = emptyList()
   }
+
+  override fun equals(other: Any?): Boolean =
+    other is FilterProviderAction && provider.getId() == other.provider.getId()
+
+  override fun hashCode(): Int =
+    Objects.hashCode(provider.getId())
 }

src/main/kotlin/com/github/lppedd/cc/completion/filter/MenuEnhancerLookupListener.kt

@@ -5,68 +5,45 @@ import com.github.lppedd.cc.emptyCollection
 import com.intellij.codeInsight.lookup.LookupEvent
 import com.intellij.codeInsight.lookup.LookupListener
 import com.intellij.codeInsight.lookup.impl.LookupImpl
-import com.intellij.openapi.actionSystem.AnAction
-import com.intellij.openapi.actionSystem.AnActionEvent
-import com.intellij.openapi.actionSystem.DataContext
+import com.intellij.codeInsight.lookup.impl.PrefixChangeListener
 import com.intellij.openapi.actionSystem.DefaultActionGroup
-import com.intellij.openapi.actionSystem.ex.AnActionListener
 import com.intellij.openapi.actionSystem.impl.ActionButton
-import com.intellij.openapi.application.ApplicationManager
-import com.intellij.openapi.command.CommandEvent
-import com.intellij.openapi.command.CommandListener
 import com.intellij.util.ReflectionUtil.getField
 import org.jetbrains.annotations.ApiStatus
-import java.util.concurrent.atomic.AtomicBoolean
-
-private val IS_MENU_MODIFIED = AtomicBoolean(false)
 
 /**
  * @author Edoardo Luppi
  */
 @ApiStatus.Internal
 internal class MenuEnhancerLookupListener(private val lookup: LookupImpl) :
     LookupListener,
-    AnActionListener,
-    CommandListener {
-  private var shouldAcceptActionsChanges = true
-  private var providers = emptyCollection<CommitTokenProvider>()
-  private var actions = emptyCollection<FilterProviderAction>()
-  private var messageBus = ApplicationManager.getApplication().messageBus.connect()
+    PrefixChangeListener {
+  @Volatile private var actions = emptyCollection<FilterProviderAction>()
 
   init {
     lookup.addLookupListener(this)
-    messageBus.subscribe(AnActionListener.TOPIC, this)
-    messageBus.subscribe(CommandListener.TOPIC, this)
+    lookup.addPrefixChangeListener(this, lookup)
   }
 
   fun setProviders(providers: Collection<CommitTokenProvider>) {
-    this.providers = providers
+    actions = providers.map { FilterProviderAction(lookup, it) }
   }
 
   override fun uiRefreshed() {
     try {
-      val myUi = getField<Any>(lookup.javaClass, lookup, null, "myUi")
-
-      if (myUi == null) {
-        IS_MENU_MODIFIED.set(false)
-        return
-      }
-
-      if (IS_MENU_MODIFIED.get() && lookup.isShown) {
-        if (shouldAcceptActionsChanges) {
-          actions.forEach(FilterProviderAction::reset)
-        }
-
-        return
-      }
-
-      IS_MENU_MODIFIED.compareAndSet(false, true)
-
+      // After much thoughts and trial-and-errors, keeping the Action list
+      // in memory and replacing each Action in the ActionGroup (the popup's menu)
+      // each time the UI is refreshed, is the only way to have a decent and consistent behavior.
+      // A positive side of this logic is that code is much simpler, and filters' state
+      // (filtered or not filtered) is maintained for all the Lookup lifecycle without any effort
+      val myUi = getField<Any>(lookup.javaClass, lookup, null, "myUi") ?: return
       val myMenuButton = getField<ActionButton>(myUi.javaClass, myUi, null, "myMenuButton")
       val menuActions = myMenuButton.action as DefaultActionGroup
 
-      actions = providers.map { FilterProviderAction(lookup, it) }
-      actions.forEach(menuActions::add)
+      actions.forEach {
+        menuActions.remove(it)
+        menuActions.add(it)
+      }
     } catch (ignored: ReflectiveOperationException) {
       // This should never happen, but in case I can't do anything about it,
       // so I'll just clean-up and let the user continue without applying any change
@@ -78,34 +55,15 @@ internal class MenuEnhancerLookupListener(private val lookup: LookupImpl) :
     cleanUp()
   }
 
-  override fun beforeActionPerformed(action: AnAction, dataContext: DataContext, event: AnActionEvent) {
-    if (action is FilterProviderAction) {
-      shouldAcceptActionsChanges = false
-    }
-  }
-
-  override fun afterActionPerformed(action: AnAction, dataContext: DataContext, event: AnActionEvent) {
-    if (action is FilterProviderAction) {
-      shouldAcceptActionsChanges = true
-    }
+  override fun beforeAppend(c: Char) {
+    actions.forEach(FilterProviderAction::reset)
   }
 
-  override fun commandStarted(event: CommandEvent) {
-    if ("Choose Lookup Item" == event.commandName) {
-      shouldAcceptActionsChanges = false
-    }
-  }
-
-  override fun commandFinished(event: CommandEvent) {
-    if ("Choose Lookup Item" == event.commandName) {
-      shouldAcceptActionsChanges = true
-    }
+  override fun beforeTruncate() {
+    actions.forEach(FilterProviderAction::reset)
   }
 
   private fun cleanUp() {
-    IS_MENU_MODIFIED.set(false)
-    providers = emptyCollection()
     lookup.removeLookupListener(this)
-    messageBus.disconnect()
   }
 }