jdaugherty commented on code in PR #441: URL: https://github.com/apache/grails-intellij-plugin/pull/441#discussion_r4241096585
########## plugin/src/main/java/org/apache/grails/intellij/plugin/actions/AddCustomFolderAction.java: ########## @@ -0,0 +1,112 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.intellij.plugin.actions; + +import com.intellij.openapi.actionSystem.ActionUpdateThread; +import com.intellij.openapi.actionSystem.AnActionEvent; +import com.intellij.openapi.actionSystem.CommonDataKeys; +import com.intellij.openapi.project.DumbAwareAction; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VfsUtilCore; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiDirectory; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiManager; +import org.apache.grails.intellij.plugin.config.GrailsCustomFoldersProjectService; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.List; + +public class AddCustomFolderAction extends DumbAwareAction { + + @Override + public void update(@NotNull AnActionEvent e) { + Project project = e.getProject(); + if (project == null) { + e.getPresentation().setEnabledAndVisible(false); + return; + } + PsiDirectory directory = getSelectedDirectory(e); + if (directory == null || isProjectRoot(project, directory)) { + e.getPresentation().setEnabledAndVisible(false); + return; + } + + GrailsCustomFoldersProjectService service = project.getService(GrailsCustomFoldersProjectService.class); + if (service.getFolders().size() >= GrailsCustomFoldersProjectService.MAX_FOLDERS) { + e.getPresentation().setVisible(true); + e.getPresentation().setEnabled(false); + e.getPresentation().setDescription( + "Maximum of " + GrailsCustomFoldersProjectService.MAX_FOLDERS + " custom folders reached. Remove one first."); + } + else { + e.getPresentation().setEnabledAndVisible(true); + } + } + + @Override + public @NotNull ActionUpdateThread getActionUpdateThread() { + return ActionUpdateThread.BGT; + } + + @Override + public void actionPerformed(@NotNull AnActionEvent e) { + Project project = e.getProject(); + PsiDirectory directory = getSelectedDirectory(e); + if (project == null || directory == null) return; + if (isProjectRoot(project, directory)) return; + + VirtualFile virtualFile = directory.getVirtualFile(); + String path = VfsUtilCore.getRelativePath(virtualFile, project.getBaseDir(), '/'); Review Comment: **Blocking.** This stores the path relative to the project base dir, but `GrailsCustomFoldersNodeProvider.resolve` resolves relative paths against `application.getRoot()`. When the Grails app isn't at the project root (for example, a multi-project build with the app in `app/`), *Add* on `app/docs` stores `app/docs`. The provider then looks up `app/app/docs`, so the folder renders as an empty placeholder. Probe with a `TestGrailsApplication` rooted at `app/`: `app/docs` gives `LeafState.ALWAYS` with 0 children, while `docs` gives an expandable node. Please use one base on both sides. The most natural is to store the path relative to `GrailsApplicationManager.getInstance(project).findApplication(dir).getRoot()`, and to offer *Add* only inside an application. Also, `Project.getBaseDir()` is deprecated. These two calls are the only ones in `plugin/src/main`; the rest of the plugin uses `ProjectUtil.guessProjectDir`. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/nodes/GrailsCustomFolderNode.java: ########## @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.intellij.plugin.projectView.nodes; + +import com.intellij.icons.AllIcons; +import com.intellij.ide.projectView.PresentationData; +import com.intellij.ide.projectView.ViewSettings; +import com.intellij.ide.projectView.impl.nodes.PsiDirectoryNode; +import com.intellij.ide.util.treeView.AbstractTreeNode; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VfsUtilCore; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiDirectory; +import com.intellij.psi.PsiManager; +import com.intellij.ui.SimpleTextAttributes; +import com.intellij.ui.tree.LeafState; +import org.apache.grails.intellij.plugin.config.FolderPaths; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; +import java.util.List; + +/** + * A configured custom folder shown as a top-level node in the Grails project view pane. The folder + * may not exist on disk, in which case the node is a placeholder: it renders with the folder name + * and the configured path as tooltip, but has no children and contains nothing. + */ +public final class GrailsCustomFolderNode extends AbstractTreeNode<String> { Review Comment: **Blocking.** This extends `AbstractTreeNode<String>` instead of `ProjectViewNode`, so the platform never descends into it when revealing a file. `ProjectViewFileVisitor.contains` only calls `node.contains(file)` on a `ProjectViewNode`. For any other node it falls back to `getContent(node)`, which needs a `PsiElement` value. I checked with a probe test. For `docs/readme.md`, `AbstractProjectViewPane.createVisitor(file).visit(...)` returns `SKIP_CHILDREN` for this node, even though `contains(file)` returns `true`. For a `GrailsPsiDirectoryNode` on the same directory it returns `CONTINUE`. So *Select Opened File* and autoscroll-from-source can't reach anything inside a custom folder. The `String` value also means the node adds no `PSI_ELEMENT`/`VIRTUAL_FILE` to the data context. *New ▸*, *Copy Path*, *Find in Files*, *Open In* and *Delete* therefore don't work on the folder node itself. Suggestion: when the folder resolves, create a `GrailsPsiDirectoryNode` subclass that carries `configuredPath`, using the existing `(dir, settings, AllIcons.Nodes.Folder, NodeWeights.CUSTOM_FOLDER, label, null, configuredPath)` constructor. That gives you `contains`, the data context, the comparator weight and title handling in `postprocess`. It also gives you a location string, so `docs` and `app/docs` are distinguishable. Keep a lightweight placeholder node only for folders that don't exist. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/config/GrailsCustomFoldersConfigurable.java: ########## @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.intellij.plugin.config; + +import com.intellij.icons.AllIcons; +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.options.Configurable; +import com.intellij.openapi.options.ConfigurationException; +import com.intellij.openapi.project.Project; +import com.intellij.ui.DocumentAdapter; +import com.intellij.ui.components.JBLabel; +import com.intellij.ui.components.JBScrollPane; +import com.intellij.ui.components.JBTextArea; +import com.intellij.util.ui.FormBuilder; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.apache.grails.intellij.plugin.projectView.GrailsProjectViewPanes; + +import javax.swing.JComponent; +import javax.swing.JPanel; +import javax.swing.event.DocumentEvent; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * The single custom-folders settings page. It edits the global defaults (application service) and + * this project's own folders (project service) together and previews the folders the Grails view + * will actually show. The global defaults apply to every Grails project; this project's folders + * are added to them. + */ +public class GrailsCustomFoldersConfigurable implements Configurable { + + private final @NotNull GrailsCustomFoldersAppService appService; + private final @NotNull GrailsCustomFoldersProjectService projectService; + private final @NotNull Project project; + + private @Nullable JPanel component; + private @Nullable JBTextArea globalArea; + private @Nullable JBTextArea overrideArea; + private @Nullable JBTextArea effectiveArea; + private @Nullable JBLabel sourceLabel; + private @Nullable JBLabel warningLabel; + + public GrailsCustomFoldersConfigurable(@NotNull Project project) { + this.project = project; + this.appService = ApplicationManager.getApplication().getService(GrailsCustomFoldersAppService.class); + this.projectService = project.getService(GrailsCustomFoldersProjectService.class); + } + + @Override + public @NotNull String getDisplayName() { + return "Custom Folders Configuration"; + } + + @Override + public @NotNull JComponent createComponent() { + if (component == null) { + globalArea = foldersArea(true); + overrideArea = foldersArea(true); + effectiveArea = foldersArea(false); + + DocumentAdapter refreshPreview = new DocumentAdapter() { + @Override + protected void textChanged(@NotNull DocumentEvent e) { + updateEffectivePreview(); + } + }; + globalArea.getDocument().addDocumentListener(refreshPreview); + overrideArea.getDocument().addDocumentListener(refreshPreview); + + sourceLabel = new JBLabel(); + warningLabel = new JBLabel(); + warningLabel.setIcon(AllIcons.General.Warning); + + JBLabel description = new JBLabel( + "<html>Relative paths are resolved against the project root; absolute paths are used as-is.<br>" Review Comment: Relative paths are actually resolved against each application's root (`GrailsCustomFoldersNodeProvider.resolve`), not the project root. Whichever base you choose for the `AddCustomFolderAction` comment, this text should say the same. Also, "absolute paths are used as-is" conflicts with the README and IMPROVEMENT-PLAN, which say only project directories work (see the README comment). ########## plugin/src/test/java/org/apache/grails/intellij/plugin/projectView/impl/GrailsCustomFoldersNodeProviderTest.java: ########## @@ -0,0 +1,271 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.intellij.plugin.projectView.impl; + +import com.intellij.ide.projectView.ViewSettings; +import com.intellij.ide.util.treeView.AbstractTreeNode; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.ui.tree.LeafState; +import org.jetbrains.annotations.NotNull; +import org.apache.grails.intellij.plugin.config.GrailsCustomFoldersAppService; +import org.apache.grails.intellij.plugin.config.GrailsCustomFoldersProjectService; +import org.apache.grails.intellij.plugin.projectView.NodeWeights; +import org.apache.grails.intellij.plugin.projectView.nodes.GrailsCustomFolderNode; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public class GrailsCustomFoldersNodeProviderTest extends GrailsNodeProviderTestSupport { + + public void testNodesCreatedForEachConfiguredFolder() { + myFixture.addFileToProject("docs/readme.md", "# readme"); + myFixture.addFileToProject(".github/workflows/ci.yml", "jobs: {}"); + myFixture.addFileToProject("build/output.txt", "stray"); + + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("docs", ".github", "build")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("one node per configured folder", 3, nodes.size()); + for (GrailsCustomFolderNode node : nodes) { + assertEquals("every custom folder node carries the custom-folder weight", + NodeWeights.CUSTOM_FOLDER, node.getNodeWeight()); + } + assertEquals("each node is labelled with its folder name", + List.of(".github", "build", "docs"), + nodes.stream().map(GrailsCustomFoldersNodeProviderTest::label).toList()); + } + + public void testNodesCreatedForGlobalAndProjectFolders() { + myFixture.addFileToProject("docs/readme.md", "# readme"); + myFixture.addFileToProject("src/placeholder.txt", ""); + + withGlobalFolders(List.of("docs"), () -> { + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("src")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("global defaults and project folders each produce a node", + List.of("docs", "src"), + nodes.stream().map(GrailsCustomFoldersNodeProviderTest::label).toList()); + }); + } + + /** + * The rendered nodes are capped at three in total, with the project's own folders taking + * precedence over the global defaults. + */ + public void testNodesCappedAtThreeWithProjectFoldersFirst() { + myFixture.addFileToProject("global1/placeholder.txt", ""); + myFixture.addFileToProject("global2/placeholder.txt", ""); + myFixture.addFileToProject("custom1/placeholder.txt", ""); + myFixture.addFileToProject("custom2/placeholder.txt", ""); + + withGlobalFolders(List.of("global1", "global2"), () -> { + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("custom1", "custom2")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("two custom and one global folder are rendered", + List.of("custom1", "custom2", "global1"), + nodes.stream().map(GrailsCustomFoldersNodeProviderTest::label).toList()); + }); + } + + /** Three project folders fill the budget, so no global folder gets a node. */ + public void testThreeProjectFoldersExcludeGlobalFolders() { + myFixture.addFileToProject("global1/placeholder.txt", ""); + myFixture.addFileToProject("custom1/placeholder.txt", ""); + myFixture.addFileToProject("custom2/placeholder.txt", ""); + myFixture.addFileToProject("custom3/placeholder.txt", ""); + + withGlobalFolders(List.of("global1"), () -> { + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("custom1", "custom2", "custom3")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("only the three custom folders are rendered", + List.of("custom1", "custom2", "custom3"), + nodes.stream().map(GrailsCustomFoldersNodeProviderTest::label).toList()); + }); + } + + private void withGlobalFolders(@NotNull List<String> folders, @NotNull Runnable body) { + GrailsCustomFoldersAppService appService = GrailsCustomFoldersAppService.getStaticInstance(); + List<String> original = new ArrayList<>(appService.getFolders()); + try { + appService.setFolders(folders); + body.run(); + } + finally { + appService.setFolders(original); + } + } + + public void testTrailingSlashKeepsFolderName() { + myFixture.addFileToProject("docs/readme.md", "# readme"); + + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("docs/")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("a trailing slash still produces its node", 1, nodes.size()); + assertEquals("a trailing slash does not blank the folder name", "docs", label(nodes.get(0))); + assertFalse("a trailing slash still resolves the existing folder", nodes.get(0).getChildren().isEmpty()); + } + + public void testLeadingDotSlashStillResolves() { + myFixture.addFileToProject("docs/readme.md", "# readme"); + + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("./docs")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("a leading ./ still produces its node", 1, nodes.size()); + assertEquals("a leading ./ does not blank the folder name", "docs", label(nodes.get(0))); + assertFalse("a leading ./ still resolves the existing folder", nodes.get(0).getChildren().isEmpty()); + } + + public void testTrailingBackslashKeepsFolderName() { + myFixture.addFileToProject("docs/readme.md", "# readme"); + + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("docs\\")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("a trailing backslash still produces its node", 1, nodes.size()); + assertEquals("a trailing backslash does not blank the folder name", "docs", label(nodes.get(0))); + assertFalse("a trailing backslash still resolves the existing folder", nodes.get(0).getChildren().isEmpty()); + } + + public void testNodesSortedAlphabeticallyByLabel() { + myFixture.addFileToProject("zebra/z.txt", ""); + myFixture.addFileToProject("alpha/a.txt", ""); + myFixture.addFileToProject("middle/m.txt", ""); + + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("zebra", "alpha", "middle")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("nodes are ordered alphabetically by label regardless of configuration order", + List.of("alpha", "middle", "zebra"), + nodes.stream().map(GrailsCustomFoldersNodeProviderTest::label).toList()); + } + + public void testNonExistentFolderIsPlaceholder() throws Exception { + VirtualFile otherDir = myFixture.getTempDirFixture().findOrCreateDir("other"); + assertNotNull(otherDir); + + GrailsCustomFoldersProjectService service = getProject().getService(GrailsCustomFoldersProjectService.class); + service.setFolders(List.of("nonexistent")); + + List<GrailsCustomFolderNode> nodes = customFolderNodes( + new GrailsCustomFoldersNodeProvider().createNodes(testApplication(false), ViewSettings.DEFAULT)); + + assertEquals("a missing folder still gets its placeholder node", 1, nodes.size()); + GrailsCustomFolderNode node = nodes.get(0); + assertEquals("the placeholder is labelled with the configured folder name", "nonexistent", label(node)); + assertTrue("a placeholder has no children", node.getChildren().isEmpty()); + assertFalse("a placeholder contains nothing", node.contains(otherDir)); + assertEquals("a placeholder must report itself as a leaf, or the tree keeps asking for children", + LeafState.ALWAYS, node.getLeafState()); + } + + public void testContainsFileInsideFolder() { Review Comment: This passes, but nothing in the platform calls `GrailsCustomFolderNode.contains`: it isn't an override (see the comment on the node). To cover reveal, assert on the visitor the pane actually uses. For example, run `AbstractProjectViewPane.createVisitor(file).visit(new TreePath(node))` off the EDT under a read action and expect `CONTINUE`. That assertion fails with the current node. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/actions/RemoveCustomFolderAction.java: ########## @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.intellij.plugin.actions; + +import com.intellij.ide.projectView.ProjectView; +import com.intellij.ide.projectView.impl.AbstractProjectViewPane; +import com.intellij.openapi.actionSystem.ActionUpdateThread; +import com.intellij.openapi.actionSystem.AnActionEvent; +import com.intellij.openapi.project.DumbAwareAction; +import com.intellij.openapi.project.Project; +import com.intellij.util.ui.tree.TreeUtil; +import org.apache.grails.intellij.plugin.config.FolderPaths; +import org.apache.grails.intellij.plugin.config.GrailsCustomFoldersProjectService; +import org.apache.grails.intellij.plugin.projectView.nodes.GrailsCustomFolderNode; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.tree.TreePath; +import java.util.ArrayList; +import java.util.List; + +public class RemoveCustomFolderAction extends DumbAwareAction { + + @Override + public void update(@NotNull AnActionEvent e) { + Project project = e.getProject(); + boolean visible = false; + if (project != null) { + AbstractProjectViewPane pane = ProjectView.getInstance(project).getCurrentProjectViewPane(); Review Comment: This reads the Swing tree selection (`getCurrentProjectViewPane().getSelectedPath()`) from a BGT `update()`. The pane already snapshots its selection into the data context on the EDT, so `e.getData(PlatformCoreDataKeys.SELECTED_ITEMS)` gives you the selected nodes safely, from the pane the menu was opened in. It also makes the action testable with a plain data context: `testRemoveLogicRemovesConfiguredPathFromService` could then call the action instead of re-implementing its logic. The same applies to `ConfigureCustomFoldersAction.isCustomFolderSelected`. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/config/GrailsCustomFoldersProjectService.java: ########## @@ -0,0 +1,138 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.intellij.plugin.config; + +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.ModalityState; +import com.intellij.openapi.components.PersistentStateComponent; +import com.intellij.openapi.components.State; +import com.intellij.openapi.components.Storage; +import com.intellij.openapi.components.StoragePathMacros; +import com.intellij.openapi.project.Project; +import com.intellij.util.messages.Topic; +import org.apache.grails.intellij.plugin.projectView.GrailsProjectViewPanes; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +@State(name = "GrailsCustomFolders", storages = @Storage(StoragePathMacros.WORKSPACE_FILE)) +public final class GrailsCustomFoldersProjectService implements PersistentStateComponent<GrailsCustomFoldersProjectService.StateHolder> { + public static final int MAX_FOLDERS = 3; + + private final Project project; + + private List<String> folders = new ArrayList<>(); + + public GrailsCustomFoldersProjectService(@NotNull Project project) { + this.project = project; + } + + public @NotNull List<String> getFolders() { + return Collections.unmodifiableList(new ArrayList<>(folders)); + } + + public void setFolders(@NotNull List<String> folders) { + this.folders = clean(folders); + // Repaint on the UI thread: this runs from the settings dialog and from the tree actions, and the + // pane must be updated on the EDT. Going through the pane directly is what makes the view react + // immediately; the event below is only a notification for anything else listening. + project.getMessageBus().syncPublisher(FoldersChangedListener.TOPIC).foldersChanged(); + ApplicationManager.getApplication().invokeLater(() -> GrailsProjectViewPanes.refresh(project), + ModalityState.defaultModalityState(), project.getDisposed()); + } + + /** + * Returns the folders the Grails view shows for this project: this project's own folders + * first, then the global defaults, with the total limited to {@link #MAX_FOLDERS}. A folder + * configured in both areas appears once — duplicates are detected after + * {@link FolderPaths#normalize(String)}. + */ + public @NotNull List<String> getEffectiveFolders() { + return Collections.unmodifiableList(combine(folders, GrailsCustomFoldersAppService.getStaticInstance().getFolders())); + } + + /** + * Combines this project's folders with the global defaults the way the Grails view shows them: + * the project's own folders first (they are the more specific configuration), then the global + * defaults, with the total limited to {@link #MAX_FOLDERS}. Duplicates (after + * {@link FolderPaths#normalize(String)}) are dropped. + */ + static @NotNull List<String> combine(@NotNull List<String> projectFolders, @NotNull List<String> globalFolders) { + List<String> effective = new ArrayList<>(); + Set<String> seen = new HashSet<>(); + for (String folder : projectFolders) { + if (effective.size() >= MAX_FOLDERS) break; + if (seen.add(FolderPaths.normalize(folder))) { + effective.add(folder); + } + } + for (String folder : globalFolders) { + if (effective.size() >= MAX_FOLDERS) break; + if (seen.add(FolderPaths.normalize(folder))) { + effective.add(folder); + } + } + return effective; + } + + static @NotNull List<String> clean(@NotNull List<String> raw) { + List<String> res = new ArrayList<>(); + Set<String> seen = new HashSet<>(); + for (String folder : raw) { + if (folder == null) continue; + String trimmed = folder.trim(); + if (trimmed.isEmpty()) continue; + String normalized = FolderPaths.normalize(trimmed); + if (normalized.isEmpty()) continue; + if (seen.add(normalized)) { + res.add(trimmed); + } + if (res.size() >= MAX_FOLDERS) break; + } + return res; + } + + @Override + public StateHolder getState() { + StateHolder res = new StateHolder(); + res.folders = new ArrayList<>(folders); + return res; + } + + @Override + public void loadState(@NotNull StateHolder state) { + folders = clean(state.folders); + } + + public static final class StateHolder { + public List<String> folders = new ArrayList<>(); + } + + public interface FoldersChangedListener { Review Comment: Nothing subscribes to this topic, so the event currently goes nowhere. The javadoc of `testSetFoldersPublishesChangeEvent` says "The subscriber in the service calls `GrailsProjectViewPanes.refresh(project)`", but no such subscriber exists. Either drop the topic, or make it the only refresh path: the pane subscribes to it, the global defaults publish an application-level equivalent, and the direct `refresh` calls go away. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/nodes/GrailsApplicationNode.java: ########## @@ -50,7 +52,21 @@ public boolean shouldUpdateData() { public @NotNull Collection<AbstractTreeNode<?>> getChildren() { Collection<AbstractTreeNode<?>> result = new ArrayList<>(); for (GrailsViewNodeProvider provider : GrailsViewNodeProvider.EP_NAME.getExtensionList()) { - result.addAll(provider.createNodes(getValue(), getSettings())); + try { + result.addAll(provider.createNodes(getValue(), getSettings())); + } + catch (RuntimeException | LinkageError e) { + // The pane builds children on a background thread and a throw there aborts the whole repaint, + // blanking the Grails view. One broken provider must not take the others down with it. + // Control-flow exceptions are the exception to that: the platform cancels a background read + // like this when a write action becomes pending and retries the build afterwards, so + // swallowing one strands the pane on "Loading...". Rethrow those untouched. + if (e instanceof ControlFlowException) { + throw e; + } + Logger.getInstance(GrailsApplicationNode.class) + .warn("Grails view provider " + provider.getClass().getName() + " failed; skipping its nodes", e); Review Comment: Minor: isolating providers makes sense, but with `warn` a broken provider just silently loses its nodes. `LOG.error` still continues, but it surfaces the failure in the IDE error reporter and fails tests; the test can expect it with `LoggedErrorProcessor`. This change and the `getAppRoot()` null-safety in `GrailsAppNodeProvider`/`GrailsViewItems` alter behaviour for every provider, so please mention them in the PR description, which currently says "purely additive". ########## plugin/src/main/java/org/apache/grails/intellij/plugin/actions/AddCustomFolderAction.java: ########## @@ -0,0 +1,112 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.intellij.plugin.actions; + +import com.intellij.openapi.actionSystem.ActionUpdateThread; +import com.intellij.openapi.actionSystem.AnActionEvent; +import com.intellij.openapi.actionSystem.CommonDataKeys; +import com.intellij.openapi.project.DumbAwareAction; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VfsUtilCore; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiDirectory; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiManager; +import org.apache.grails.intellij.plugin.config.GrailsCustomFoldersProjectService; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.List; + +public class AddCustomFolderAction extends DumbAwareAction { + + @Override + public void update(@NotNull AnActionEvent e) { Review Comment: Add and Configure show up in the context menu of every directory, in every pane, of every project, including non-Grails projects. The existing Grails actions first check `GrailsApplicationManager.getInstance(project).hasApplications()` (`GrailsActionGroup`, `GrailsRunCommandAction`). Here I'd require the selected directory to belong to an application. That also gives you the right base for the stored path. ########## plugin/src/main/resources/META-INF/plugin.xml: ########## @@ -318,6 +319,15 @@ <projectService serviceImplementation="org.apache.grails.intellij.plugin.lang.gsp.psi.gsp.impl.gtag.GspTagDescriptorService"/> + <applicationService serviceImplementation="org.apache.grails.intellij.plugin.config.GrailsCustomFoldersAppService"/> + <projectService serviceImplementation="org.apache.grails.intellij.plugin.config.GrailsCustomFoldersProjectService"/> + <projectConfigurable id="preferences.grails" groupId="tools" Review Comment: Could this go under **Languages & Frameworks ▸ Grails** (`groupId="language"`, display name `Grails`) instead of *Tools ▸ Grails IntelliJ Plugin*? Framework settings usually live there; CSS Stylesheets and SQL Dialects register the same way. I'd also avoid the empty parent page. Until there are more Grails settings, the Grails page itself could be the custom-folders page; otherwise, give the parent a short description. If custom folders stays a child page, *Custom Folders* is enough as a name. Since the page edits per-project folders, you probably also want `nonDefaultProject="true"`, as SQL Dialects does. ########## README.md: ########## @@ -8,6 +8,10 @@ integrations for i18n, coverage, Hibernate, Maven, and language injection. The Grails project view pane gives `grails-app/` dedicated **Stylesheets**, **Images**, **JavaScripts**, **Migrations**, **Translations** and **Utils** nodes, and lifts the test source roots out of `src` as **Tests:unit**, **Tests:integration** and **Tests:functional**. +Users can configure up to 3 custom folders — global defaults with a per-project override — +that appear as nodes below the test nodes in the Grails view. Custom folders can only Review Comment: This text, IMPROVEMENT-PLAN 2.8 and the PR description all say custom folders can only reference project directories, and that hidden dot-directories are filtered out. The code says otherwise: - the settings page says absolute paths are used as-is; - the provider resolves absolute paths through `LocalFileSystem`; - `testNodesCreatedForEachConfiguredFolder` uses `.github` (the default ignore list contains `.git`, not `.github`). Could you check in `runIde` what actually happens for an absolute path outside the project and for `.github`, and make the docs and settings text match? The fixed "up to 3" wording needs updating too, now that the cap is a setting. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/config/GrailsCustomFoldersProjectService.java: ########## @@ -0,0 +1,138 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.intellij.plugin.config; + +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.ModalityState; +import com.intellij.openapi.components.PersistentStateComponent; +import com.intellij.openapi.components.State; +import com.intellij.openapi.components.Storage; +import com.intellij.openapi.components.StoragePathMacros; +import com.intellij.openapi.project.Project; +import com.intellij.util.messages.Topic; +import org.apache.grails.intellij.plugin.projectView.GrailsProjectViewPanes; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +@State(name = "GrailsCustomFolders", storages = @Storage(StoragePathMacros.WORKSPACE_FILE)) +public final class GrailsCustomFoldersProjectService implements PersistentStateComponent<GrailsCustomFoldersProjectService.StateHolder> { + public static final int MAX_FOLDERS = 3; Review Comment: On your open question about the limit: let's make it a setting instead of a hard-coded number. Add a `maxFolders` value to the global state (`GrailsCustomFoldersAppService`) with a default of **6** and a range of about 1–20, and edit it with a spinner on the settings page. The live preview and the over-cap warning should use the value being edited, just as they already use the typed paths. Please apply the cap only when the view is built (`getEffectiveFolders`/`combine`), not when folders are saved. Today `clean()` and `parseFolders` throw away everything past the cap, so lowering the setting would permanently delete configured folders. Storing all folders and capping only at render time avoids that. This also simplifies the code: the four copies of the normalize/dedupe/cap loop (`clean()` in both services, `parseFolders`, `uniqueNormalized`) can become one dedupe helper in `FolderPaths`, and the two `MAX_FOLDERS` constants go away. Other places that need updating: - the "maximum reached" check in `AddCustomFolderAction.update` should read the configured value; - the "up to 3 folders" labels; - the README/IMPROVEMENT-PLAN text; - the tests that assert 3. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/actions/ConfigureCustomFoldersAction.java: ########## @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.intellij.plugin.actions; + +import com.intellij.ide.projectView.ProjectView; +import com.intellij.ide.projectView.impl.AbstractProjectViewPane; +import com.intellij.openapi.actionSystem.ActionUpdateThread; +import com.intellij.openapi.actionSystem.AnActionEvent; +import com.intellij.openapi.actionSystem.CommonDataKeys; +import com.intellij.openapi.options.ShowSettingsUtil; +import com.intellij.openapi.project.DumbAwareAction; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiDirectory; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiManager; +import com.intellij.util.ui.tree.TreeUtil; +import org.apache.grails.intellij.plugin.config.GrailsCustomFoldersConfigurable; +import org.apache.grails.intellij.plugin.projectView.nodes.GrailsCustomFolderNode; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.tree.TreePath; + +public class ConfigureCustomFoldersAction extends DumbAwareAction { + + @Override + public void update(@NotNull AnActionEvent e) { + Project project = e.getProject(); + boolean visible = false; + if (project != null) { + AbstractProjectViewPane pane = ProjectView.getInstance(project).getCurrentProjectViewPane(); + if (pane != null) { + // Visible when a custom folder node is selected (same as Remove) + if (isCustomFolderSelected(pane)) { + visible = true; + } + // Also visible when a directory is selected (same as Add) + else if (getSelectedDirectory(e) != null) { Review Comment: As with Add, this makes *Configure Custom Grails Folders* appear for any directory in any project. The page is already reachable from Settings, so I'd show this action only in the Grails pane, or only on custom folder nodes. ########## plugin/src/main/java/org/apache/grails/intellij/plugin/config/GrailsCustomFoldersConfigurable.java: ########## @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.intellij.plugin.config; + +import com.intellij.icons.AllIcons; +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.options.Configurable; +import com.intellij.openapi.options.ConfigurationException; +import com.intellij.openapi.project.Project; +import com.intellij.ui.DocumentAdapter; +import com.intellij.ui.components.JBLabel; +import com.intellij.ui.components.JBScrollPane; +import com.intellij.ui.components.JBTextArea; +import com.intellij.util.ui.FormBuilder; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.apache.grails.intellij.plugin.projectView.GrailsProjectViewPanes; + +import javax.swing.JComponent; +import javax.swing.JPanel; +import javax.swing.event.DocumentEvent; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * The single custom-folders settings page. It edits the global defaults (application service) and + * this project's own folders (project service) together and previews the folders the Grails view + * will actually show. The global defaults apply to every Grails project; this project's folders + * are added to them. + */ +public class GrailsCustomFoldersConfigurable implements Configurable { + + private final @NotNull GrailsCustomFoldersAppService appService; + private final @NotNull GrailsCustomFoldersProjectService projectService; + private final @NotNull Project project; + + private @Nullable JPanel component; + private @Nullable JBTextArea globalArea; + private @Nullable JBTextArea overrideArea; + private @Nullable JBTextArea effectiveArea; + private @Nullable JBLabel sourceLabel; + private @Nullable JBLabel warningLabel; + + public GrailsCustomFoldersConfigurable(@NotNull Project project) { + this.project = project; + this.appService = ApplicationManager.getApplication().getService(GrailsCustomFoldersAppService.class); + this.projectService = project.getService(GrailsCustomFoldersProjectService.class); + } + + @Override + public @NotNull String getDisplayName() { + return "Custom Folders Configuration"; + } + + @Override + public @NotNull JComponent createComponent() { + if (component == null) { + globalArea = foldersArea(true); + overrideArea = foldersArea(true); + effectiveArea = foldersArea(false); + + DocumentAdapter refreshPreview = new DocumentAdapter() { + @Override + protected void textChanged(@NotNull DocumentEvent e) { + updateEffectivePreview(); + } + }; + globalArea.getDocument().addDocumentListener(refreshPreview); + overrideArea.getDocument().addDocumentListener(refreshPreview); + + sourceLabel = new JBLabel(); + warningLabel = new JBLabel(); + warningLabel.setIcon(AllIcons.General.Warning); + + JBLabel description = new JBLabel( + "<html>Relative paths are resolved against the project root; absolute paths are used as-is.<br>" + + "The Grails view renders at most " + GrailsCustomFoldersProjectService.MAX_FOLDERS + + " folders in total: this project's folders come first, then the global defaults.</html>"); + + component = FormBuilder.createFormBuilder() + .addComponent(description) + .addComponent(warningLabel) + .addLabeledComponent(new JBLabel("Global defaults (up to " + + GrailsCustomFoldersProjectService.MAX_FOLDERS + " folders):"), + new JBScrollPane(globalArea), 1, false) + .addLabeledComponent(new JBLabel("This project's folders (up to " + + GrailsCustomFoldersProjectService.MAX_FOLDERS + " folders):"), + new JBScrollPane(overrideArea), 1, false) + .addComponent(sourceLabel) + .addLabeledComponent(new JBLabel("Effective folders in the Grails view:"), + new JBScrollPane(effectiveArea), 1, false) + .getPanel(); + + reset(); + } + return component; + } + + @Override + public boolean isModified() { + if (globalArea != null && !parseFolders(globalArea.getText()).equals(appService.getFolders())) return true; + return overrideArea != null && !parseFolders(overrideArea.getText()).equals(projectService.getFolders()); + } + + @Override + public void apply() throws ConfigurationException { + // The global defaults have no change event, so repaint here: editing them changes what every + // Grails project renders, and the view would otherwise keep the old folders until something + // else triggered a refresh. + if (globalArea != null) appService.setFolders(parseFolders(globalArea.getText())); + if (overrideArea != null) projectService.setFolders(parseFolders(overrideArea.getText())); + GrailsProjectViewPanes.refresh(project); Review Comment: The comment above says editing the global defaults changes what every Grails project renders, but only this project's pane is refreshed. Any other open Grails project keeps the old folders until something else triggers a refresh. Either refresh every open project, or publish an application-level topic that each pane listens to. ########## plugin/src/test/java/org/apache/grails/intellij/plugin/actions/CustomFoldersActionTest.java: ########## @@ -0,0 +1,236 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.intellij.plugin.actions; + +import com.intellij.openapi.actionSystem.ActionManager; +import com.intellij.openapi.actionSystem.ActionPlaces; +import com.intellij.openapi.actionSystem.AnActionEvent; +import com.intellij.openapi.actionSystem.CommonDataKeys; +import com.intellij.openapi.actionSystem.DataContext; +import com.intellij.openapi.actionSystem.DataKey; +import com.intellij.openapi.actionSystem.Presentation; +import com.intellij.openapi.vfs.VfsUtilCore; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiDirectory; +import com.intellij.psi.PsiManager; +import org.apache.grails.intellij.lib.testFramework.GrailsTestCase; +import org.apache.grails.intellij.plugin.config.GrailsCustomFoldersProjectService; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +public class CustomFoldersActionTest extends GrailsTestCase { + + public void testAddPathAppendsToProjectFolders() { Review Comment: This test can't catch the base-dir mismatch. In the light fixture, `getProject().getBaseDir()` is a real `/private/var/…/unitTest_*` directory, not an ancestor of `temp:///src`. So `getRelativePath` returns null, the action stores the absolute `/src/custom`, and `expected` is computed with the same formula. To cover the round trip, assert the exact stored value, then assert that the provider resolves it to an expandable node. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
