vcs-ignore: improve mark ignored as excluded

* make "mark ignored as excluded" per-project option
* make "mark ignored as excluded" turned off by default
* suggest to exclude ignored directories with notification
* add registry key to control "mark ignored as excluded"

GitOrigin-RevId: 0602455449a448039d17d148e0a4863f1c7abef1

platform/dvcs-impl/src/com/intellij/dvcs/ignore/IgnoredToExcludedSynchronizer.kt

@@ -1,48 +1,136 @@
 // Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.dvcs.ignore
 
+import com.intellij.ProjectTopics
 import com.intellij.ide.projectView.actions.MarkExcludeRootAction
+import com.intellij.openapi.Disposable
 import com.intellij.openapi.application.runInEdt
+import com.intellij.openapi.application.runReadAction
+import com.intellij.openapi.diagnostic.logger
 import com.intellij.openapi.module.Module
 import com.intellij.openapi.module.ModuleUtil
 import com.intellij.openapi.progress.ProgressManager
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.roots.ModuleRootEvent
+import com.intellij.openapi.roots.ModuleRootListener
+import com.intellij.openapi.roots.OrderEnumerator
+import com.intellij.openapi.roots.ProjectFileIndex
 import com.intellij.openapi.util.io.FileUtil
+import com.intellij.openapi.util.registry.Registry
 import com.intellij.openapi.vcs.FilePath
-import com.intellij.openapi.vcs.VcsApplicationSettings
+import com.intellij.openapi.vcs.FilesProcessorWithNotificationImpl
+import com.intellij.openapi.vcs.VcsBundle.message
+import com.intellij.openapi.vcs.VcsConfiguration
 import com.intellij.openapi.vcs.changes.shelf.ShelveChangesManager
 import com.intellij.openapi.vfs.VirtualFile
 import java.util.*
 
+private const val ASKED_MARK_IGNORED_FILES_AS_EXCLUDED_PROPERTY = "ASKED_MARK_IGNORED_FILES_AS_EXCLUDED"
+
+private val LOG = logger<IgnoredToExcludedSynchronizer>()
+
 private val excludeAction = object : MarkExcludeRootAction() {
   fun exclude(module: Module, dirs: Collection<VirtualFile>) = runInEdt { modifyRoots(module, dirs.toTypedArray()) }
 }
 
-class IgnoredToExcludedSynchronizer(private val project: Project) : VcsIgnoredHolderUpdateListener {
+class IgnoredToExcludedSynchronizer(project: Project, parentDisposable: Disposable)
+  : VcsIgnoredHolderUpdateListener, FilesProcessorWithNotificationImpl(project, parentDisposable) {
+
+  override val askedBeforeProperty = ASKED_MARK_IGNORED_FILES_AS_EXCLUDED_PROPERTY
+  override val doForCurrentProjectProperty: String? = null
+  override val showActionText: String = message("ignore.to.exclude.notification.action.view")
+  override val forCurrentProjectActionText: String = message("ignore.to.exclude.notification.action.exclude")
+  override val forAllProjectsActionText: String? = null
+  override val muteActionText: String = message("ignore.to.exclude.notification.action.mute")
+  override fun notificationTitle() = ""
+  override fun notificationMessage(): String = message("ignore.to.exclude.notification.message")
+
+  init {
+    project.messageBus.connect(this).subscribe(ProjectTopics.PROJECT_ROOTS, object : ModuleRootListener {
+      override fun rootsChanged(event: ModuleRootEvent) = updateNotificationState()
+    })
+  }
+
+  private fun updateNotificationState() {
+    // in case if the project roots changed (e.g. by build tools) then the directories shown in notification can be outdated.
+    // filter directories which excluded or containing source roots and expire notification if needed.
+    if (notificationNotPresent()) return
+
+    val fileIndex = ProjectFileIndex.getInstance(project)
+    val sourceRoots = getProjectSourceRoots()
+
+    val acquiredFiles = acquireValidFiles()
+    LOG.debug("updateNotificationState, acquiredFiles", acquiredFiles)
+    val filesToRemove = acquiredFiles
+      .asSequence()
+      .filter { file -> fileIndex.isExcluded(file) || sourceRoots.contains(file) }
+      .toList()
+    LOG.debug("updateNotificationState, filesToRemove", filesToRemove)
+    removeFiles(filesToRemove)
+
+    val files = acquireValidFiles()
+    if (files.isEmpty()) {
+      expireNotification()
+    }
+  }
+
+  override fun doActionOnChosenFiles(files: Collection<VirtualFile>) {
+    markIgnoredAsExcluded(files)
+  }
+
+  override fun doFilterFiles(files: Collection<VirtualFile>) = files.filter(VirtualFile::isValid)
+
+  override fun rememberForAllProjects() {}
+
+  override fun rememberForCurrentProject() {
+    VcsConfiguration.getInstance(project).MARK_IGNORED_AS_EXCLUDED = true
+  }
+
+  override fun needDoForCurrentProject() = VcsConfiguration.getInstance(project).MARK_IGNORED_AS_EXCLUDED
 
   override fun updateFinished(ignoredPaths: Collection<FilePath>) {
     ProgressManager.checkCanceled()
-    if (!VcsApplicationSettings.getInstance().MARK_IGNORED_AS_EXCLUDED) return
+    if (!Registry.`is`("vcs.propose.add.ignored.directories.to.exclude", true)) return
+    if (!VcsConfiguration.getInstance(project).MARK_IGNORED_AS_EXCLUDED && wasAskedBefore()) return
 
-    markIgnoredAsExcluded(ignoredPaths)
+    processIgnored(ignoredPaths)
   }
 
-  private fun markIgnoredAsExcluded(ignoredPaths: Collection<FilePath>) {
-    val ignoredDirsByModule =
+  private fun processIgnored(ignoredPaths: Collection<FilePath>) {
+    val sourceRoots = getProjectSourceRoots()
+    val fileIndex = ProjectFileIndex.getInstance(project)
+    val ignoredDirs =
       ignoredPaths
         .asSequence()
         .filter(FilePath::isDirectory)
-        .filterNot(::isShelfDirectoryOrUnder) //shelf directory usually contains in project and excluding it prevents local history to work
+        //shelf directory usually contains in project and excluding it prevents local history to work on it
+        .filterNot(::containsShelfDirectoryOrUnderIt)
         .mapNotNull(FilePath::getVirtualFile)
+        .filterNot(fileIndex::isExcluded)
+        //do not propose to exclude if there is a source root inside
+        .filterNot { ignored -> sourceRoots.contains(ignored) }
+        .toList()
+    processFiles(ignoredDirs)
+  }
+
+  private fun markIgnoredAsExcluded(files: Collection<VirtualFile>) {
+    val ignoredDirsByModule =
+      files
+        .asSequence()
         .groupBy { ModuleUtil.findModuleForFile(it, project) }
-        .filterKeys(Objects::nonNull) //if the directory already excluded then ModuleUtil.findModuleForFile return null and this will filter out such directories from processing.
+        //if the directory already excluded then ModuleUtil.findModuleForFile return null and this will filter out such directories from processing.
+        .filterKeys(Objects::nonNull)
 
     for ((module, ignoredDirs) in ignoredDirsByModule) {
       excludeAction.exclude(module!!, ignoredDirs)
     }
   }
 
-  private fun isShelfDirectoryOrUnder(filePath: FilePath) =
-    FileUtil.isAncestor(ShelveChangesManager.getShelfPath(project), filePath.path, false);
+  private fun getProjectSourceRoots() =
+    runReadAction { OrderEnumerator.orderEntries(project).withoutSdk().withoutLibraries().sources().usingCache().roots }
+
+  private fun containsShelfDirectoryOrUnderIt(filePath: FilePath) =
+    FileUtil.isAncestor(ShelveChangesManager.getShelfPath(project), filePath.path, false)
+    || FileUtil.isAncestor(filePath.path, ShelveChangesManager.getShelfPath(project), false)
 }
 

platform/platform-resources-en/src/messages/VcsBundle.properties

@@ -616,6 +616,11 @@ external.files.add.notification.action.view=View Files
 external.files.add.notification.action.add=Always Add
 external.files.add.notification.action.mute=Don't Ask Again
 
+ignore.to.exclude.notification.message=Found ignored but not excluded directories
+ignore.to.exclude.notification.action.view=View
+ignore.to.exclude.notification.action.exclude=Always exclude
+ignore.to.exclude.notification.action.mute=Don't Ask Again
+
 ignoreTokenType./=<slash>
 ignoreTokenType.BRACKET_LEFT=<left bracket>
 ignoreTokenType.BRACKET_RIGHT=<right bracket>

platform/util/resources/misc/registry.properties

@@ -531,6 +531,9 @@ vcs.code.analysis.before.checkin.show.only.new.threshold.description=Show only n
 vcs.process.externally.added.files=true
 vcs.process.externally.added.files.description=Process externally added files. Add such files to VCS automatically or prompt user.
 
+vcs.propose.add.ignored.directories.to.exclude=false
+vcs.propose.add.ignored.directories.to.exclude.description=Show notification with proposal to add ignored (but not excluded) directories to exclude.
+
 psi.incremental.reparse.depth.limit=1000
 psi.deferIconLoading=true
 

platform/vcs-api/src/com/intellij/openapi/vcs/VcsApplicationSettings.java

@@ -25,7 +25,6 @@ public class VcsApplicationSettings implements PersistentStateComponent<VcsAppli
   public boolean MANAGE_IGNORE_FILES = false;
   public boolean DISABLE_MANAGE_IGNORE_FILES = false;
   public boolean MARK_EXCLUDED_AS_IGNORED = true;
-  public boolean MARK_IGNORED_AS_EXCLUDED = false;
 
   public static VcsApplicationSettings getInstance() {
     return ServiceManager.getService(VcsApplicationSettings.class);

platform/vcs-api/src/com/intellij/openapi/vcs/VcsConfiguration.java

@@ -73,6 +73,7 @@ public final class VcsConfiguration implements PersistentStateComponent<VcsConfi
   public boolean SHELVE_DETAILS_PREVIEW_SHOWN = false;
   public boolean VCS_LOG_DETAILS_PREVIEW_SHOWN = false;
   public boolean RELOAD_CONTEXT = true;
+  public boolean MARK_IGNORED_AS_EXCLUDED = false;
 
   @XCollection(elementName = "path", propertyElementName = "ignored-roots")
   public List<String> IGNORED_UNREGISTERED_ROOTS = new ArrayList<>();

platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/IgnoredSettingsPanel.kt

@@ -6,8 +6,10 @@ import com.intellij.ide.util.PropertiesComponent
 import com.intellij.openapi.options.BoundConfigurable
 import com.intellij.openapi.options.SearchableConfigurable
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.util.registry.Registry
 import com.intellij.openapi.vcs.VcsApplicationSettings
 import com.intellij.openapi.vcs.VcsBundle.message
+import com.intellij.openapi.vcs.VcsConfiguration
 import com.intellij.openapi.vcs.changes.ignore.IgnoreConfigurationProperty.ASKED_MANAGE_IGNORE_FILES_PROPERTY
 import com.intellij.openapi.vcs.changes.ignore.IgnoreConfigurationProperty.MANAGE_IGNORE_FILES_PROPERTY
 import com.intellij.openapi.vcs.changes.ui.IgnoredSettingsPanel.ManageIgnoredOption.*
@@ -19,6 +21,7 @@ internal class IgnoredSettingsPanel(private val project: Project) : BoundConfigu
                                                                                       "project.propVCSSupport.Ignored.Files"), SearchableConfigurable {
   internal var selectedManageIgnoreOption = getIgnoredOption()
   internal var settings = VcsApplicationSettings.getInstance()
+  internal var projectSettings = VcsConfiguration.getInstance(project)
 
   override fun apply() {
     val modified = isModified
@@ -52,8 +55,8 @@ internal class IgnoredSettingsPanel(private val project: Project) : BoundConfigu
           checkBox(message("ignored.file.excluded.to.ignored.label"), settings::MARK_EXCLUDED_AS_IGNORED)
         }
         row {
-          checkBox(message("ignored.file.ignored.to.excluded.label"), settings::MARK_IGNORED_AS_EXCLUDED)
-        }
+          checkBox(message("ignored.file.ignored.to.excluded.label"), projectSettings::MARK_IGNORED_AS_EXCLUDED)
+        }.visible = Registry.`is`("vcs.propose.add.ignored.directories.to.exclude", true)
       }
     }
 

plugins/git4idea/src/git4idea/repo/GitRepositoryImpl.java

@@ -64,7 +64,7 @@ public class GitRepositoryImpl extends RepositoryImpl implements GitRepository {
         new GitRepositoryIgnoredFilesHolder(project, this, GitRepositoryManager.getInstance(project), Git.getInstance());
       Disposer.register(this, myIgnoredRepositoryFilesHolder);
       myIgnoredRepositoryFilesHolder.addUpdateStateListener(new MyRepositoryIgnoredHolderUpdateListener(project));
-      myIgnoredRepositoryFilesHolder.addUpdateStateListener(new IgnoredToExcludedSynchronizer(project));
+      myIgnoredRepositoryFilesHolder.addUpdateStateListener(new IgnoredToExcludedSynchronizer(project, this));
 
       myConflictsHolder = new GitConflictsHolder(this);
       Disposer.register(this, myConflictsHolder);

plugins/git4idea/tests/git4idea/ignore/GitIgnoredToExcludedSynchronizerTest.kt

@@ -4,21 +4,26 @@ package git4idea.ignore
 import com.intellij.dvcs.ignore.VcsRepositoryIgnoredFilesHolderBase
 import com.intellij.openapi.application.invokeAndWaitIfNeeded
 import com.intellij.openapi.application.runWriteAction
+import com.intellij.openapi.module.Module
 import com.intellij.openapi.roots.ModuleRootManager
-import com.intellij.openapi.vcs.VcsApplicationSettings
+import com.intellij.openapi.util.registry.Registry
+import com.intellij.openapi.vcs.VcsBundle
+import com.intellij.openapi.vcs.VcsConfiguration
 import com.intellij.openapi.vfs.VirtualFile
 import git4idea.repo.GitRepositoryFiles.GITIGNORE
 import git4idea.test.GitSingleRepoTest
-import org.junit.Assume.assumeTrue
+
+const val GEN = "gen"
 
 class GitIgnoredToExcludedSynchronizerTest : GitSingleRepoTest() {
 
   private lateinit var out: VirtualFile
   private lateinit var excluded: VirtualFile
+  private lateinit var gen: VirtualFile
 
   override fun setUp() {
     super.setUp()
-    assumeTrue(VcsApplicationSettings.getInstance().MARK_IGNORED_AS_EXCLUDED)
+    Registry.get("vcs.propose.add.ignored.directories.to.exclude").setValue(true, testRootDisposable)
   }
 
   override fun setUpModule() {
@@ -30,25 +35,72 @@ class GitIgnoredToExcludedSynchronizerTest : GitSingleRepoTest() {
       //create file in dirs, otherwise the directory will be not treated as ignored by Git
       out = moduleDir.findOrCreateDir(OUT).apply { createFile("a.class") }
       excluded = moduleDir.findOrCreateDir(EXCLUDED).apply { createFile("b.class") }
+      gen = moduleDir.findOrCreateDir(GEN).apply { createFile("a.java") }
+      myModule.addSourceFolder(gen)
     }
   }
 
+  fun `test mark ignored directories as excluded notification`() {
+    assertEmpty(module.excludes())
+
+    createGitignoreAndWait("""
+                            /$EXCLUDED/
+                            /$OUT/
+                           """.trimIndent())
+
+    assertNotificationByContent(VcsBundle.message("ignore.to.exclude.notification.message"))
+  }
+
   fun `test mark ignored directories as excluded`() {
-    val ignoredHolderWaiter = (repo.ignoredFilesHolder as VcsRepositoryIgnoredFilesHolderBase<*>).createWaiter()
+    VcsConfiguration.getInstance(project).MARK_IGNORED_AS_EXCLUDED = true
 
-    file(GITIGNORE)
-      .create("""
-                /$EXCLUDED/
-                /$OUT/
-              """.trimIndent())
+    assertEmpty(module.excludes())
 
-    ignoredHolderWaiter.waitFor()
+    createGitignoreAndWait("""
+                            /$EXCLUDED/
+                            /$OUT/
+                           """.trimIndent())
 
     assertExcludedDirs(out, excluded)
   }
 
+  fun `test do not mark ignored source root directory as excluded`() {
+    VcsConfiguration.getInstance(project).MARK_IGNORED_AS_EXCLUDED = true
+
+    assertSourceDirs(gen)
+
+    createGitignoreAndWait("""
+                            /$EXCLUDED/
+                            /$OUT/
+                            /$GEN/
+                           """.trimIndent())
+
+    assertExcludedDirs(out, excluded)
+    assertSourceDirs(gen)
+  }
+
+  private fun createGitignoreAndWait(gitignoreContent: String) {
+    val ignoredHolderWaiter = (repo.ignoredFilesHolder as VcsRepositoryIgnoredFilesHolderBase<*>).createWaiter()
+
+    file(GITIGNORE).create(gitignoreContent)
+
+    ignoredHolderWaiter.waitFor()
+  }
+
+  private fun assertNotificationByContent(notificationContent: String) =
+    vcsNotifier.notifications.find { it.content == notificationContent }
+    ?: fail("Notification $notificationContent not found")
+
   private fun assertExcludedDirs(vararg expectedExcludes: VirtualFile) {
-    val excludes = invokeAndWaitIfNeeded { ModuleRootManager.getInstance(module).excludeRoots.toList() }
+    val excludes = module.excludes()
     assertContainsElements(excludes, *expectedExcludes)
   }
+
+  private fun assertSourceDirs(vararg expectedSources: VirtualFile) {
+    val sourceRoots = module.sourceRoots()
+    assertContainsElements(sourceRoots, *expectedSources)
+  }
+
+  private fun Module.sourceRoots() = invokeAndWaitIfNeeded { ModuleRootManager.getInstance(this).sourceRoots.toList() }
+  private fun Module.excludes() = invokeAndWaitIfNeeded { ModuleRootManager.getInstance(this).excludeRoots.toList() }
 }

plugins/git4idea/tests/git4idea/ignore/ModuleExtensions.kt

@@ -1,7 +1,7 @@
 // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package git4idea.ignore
 
-import com.intellij.openapi.application.ReadAction
+import com.intellij.openapi.application.runReadAction
 import com.intellij.openapi.module.Module
 import com.intellij.openapi.roots.ContentEntry
 import com.intellij.openapi.roots.ModifiableRootModel
@@ -16,12 +16,20 @@ import org.junit.Assert.assertFalse
 
 fun Module.addExclude(outDir: VirtualFile) {
   ModuleRootModificationUtil.updateModel(this) { model ->
-    ReadAction.run<RuntimeException> {
+    runReadAction {
       findContentEntry(model, outDir)?.addExcludeFolder(outDir)
     }
   }
 }
 
+fun Module.addSourceFolder(sourceDir: VirtualFile, isTestSource: Boolean = false) {
+  ModuleRootModificationUtil.updateModel(this) { model ->
+    runReadAction {
+      findContentEntry(model, sourceDir)?.addSourceFolder(sourceDir, isTestSource)
+    }
+  }
+}
+
 fun Module.addContentRoot(vDir: VirtualFile): ContentEntry? {
   ModuleRootModificationUtil.updateModel(this) { model -> model.addContentEntry(vDir) }
   for (entry in ModuleRootManager.getInstance(this).contentEntries) {

plugins/hg4idea/src/org/zmlx/hg4idea/repo/HgRepositoryImpl.java

@@ -2,7 +2,6 @@
 
 package org.zmlx.hg4idea.repo;
 
-import com.intellij.dvcs.ignore.IgnoredToExcludedSynchronizer;
 import com.intellij.dvcs.ignore.VcsIgnoredHolderUpdateListener;
 import com.intellij.dvcs.repo.RepositoryImpl;
 import com.intellij.openapi.Disposable;
@@ -58,7 +57,6 @@ public class HgRepositoryImpl extends RepositoryImpl implements HgRepository {
     myLocalIgnoredHolder.setupVfsListener();
     Disposer.register(this, myLocalIgnoredHolder);
     myLocalIgnoredHolder.addUpdateStateListener(new MyIgnoredHolderAsyncListener(getProject()));
-    myLocalIgnoredHolder.addUpdateStateListener(new IgnoredToExcludedSynchronizer(getProject()));
     update();
   }