jdaugherty commented on code in PR #432:
URL:
https://github.com/apache/grails-intellij-plugin/pull/432#discussion_r4178183710
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/nodes/OtherGrailsAppSourcesNode.java:
##########
@@ -96,7 +97,12 @@ public OtherGrailsAppSourcesNode(@NotNull PsiDirectory
directory, @NotNull ViewS
for (VirtualFile dir : otherDirs) {
PsiDirectory directory = manager.findDirectory(dir);
if (directory != null) {
- result.add(new PsiDirectoryNode(project, directory, getSettings()));
+ if (GrailsViewItems.ASSETS_DIR.equals(dir.getName())) {
+ result.add(new PsiDirectoryNode(project, directory, getSettings(),
+ item ->
!GrailsViewItems.isAssetSubfolder(item.getName())));
Review Comment:
This filter matches by name, and `PsiDirectoryNode` passes its filter down
to every child directory node (`ProjectViewDirectoryHelper` creates each
subdirectory node with the same filter). So any folder or file named `images`,
`stylesheets` or `javascripts` anywhere under `assets/` disappears, not just
the three top-level ones. A common vendor layout such as
`grails-app/assets/vendor/jquery-ui/images/` vanishes from the tree.
`contains()` still returns `true` for
`assets/vendor/jquery-ui/images/ui-icons.png`, because
`isUnderHiddenChildDirectory` only looks at the direct children of `assets`. So
Reveal in Project View expands Other sources and dead-ends on the hidden
folder, which is the case this PR sets out to prevent.
I confirmed both with a probe test: the `jquery-ui` node's children came
back as `[jquery-ui.js]` only. Matching only the direct children of `assets`
fixes it, and the existing tests still pass:
```java
item -> {
VirtualFile file = item.getVirtualFile();
return file == null || !dir.equals(file.getParent()) ||
!GrailsViewItems.isAssetSubfolder(item.getName());
}
```
Please add a regression test for a nested `images/` folder as well.
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/impl/Grails3NodeProvider.java:
##########
@@ -36,13 +42,22 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
+import javax.swing.Icon;
public class Grails3NodeProvider implements GrailsViewNodeProvider {
private static final List<String> SPECIAL_FILES = List.of("build.gradle",
"settings.gradle", "gradle.properties");
private static final List<String> SPECIAL_DIRS = List.of("src/main/scripts",
"src/main/webapp");
+ /**
+ * Grails 7 kebab-case test source roots under {@code src/}. The Grails 6
camelCase roots
+ * ({@code integrationTest}, {@code functionalTest}) deliberately stay
inside the {@code src} node.
+ */
+ private static final Set<String> TEST_SOURCE_DIRS = Set.of("test",
"integration-test", "functional-test");
Review Comment:
On 8.0.x, test phases aren't limited to these three names.
`TestPhase.sourceFolderName` defaults to `src/<kebab-case phase name>` for any
phase declared in `testPhases { }`, and `GrailsCliArtifactGradlePlugin` adds
`src/integration-test-cli`. Those would stay buried under `src`.
Deriving the lifted folders from the module's test source roots would cover
them. For example, lift a child of `src` whose source roots are in test content
(`ProjectFileIndex.isInTestSourceContent`, which the Gradle import already sets
up). That's Community Edition API. This is fine as a follow-up if you'd rather
keep the PR's scope as it is.
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/impl/GrailsViewItems.java:
##########
@@ -47,16 +46,34 @@ public record SpecialFolder(@NotNull Icon icon, int weight,
@NotNull String titl
public static final Map<String, SpecialFolder> SPECIAL_GRAILS_APP_FOLDERS =
specialFolders();
+ /** Subfolders of {@code grails-app/assets} shown as dedicated top-level
nodes. */
+ public static final Map<String, SpecialFolder> SPECIAL_ASSET_FOLDERS =
assetFolders();
+
+ /** Name of the {@code grails-app} directory whose subfolders are shown as
dedicated nodes. */
+ public static final String ASSETS_DIR = "assets";
+
private GrailsViewItems() {
}
+ // Declaration order below is not the rendering order: GrailsNodeComparator
sorts these nodes by
+ // their NodeWeights weight, so the two registries can be read in any order
without affecting the tree.
private static Map<String, SpecialFolder> specialFolders() {
- // LinkedHashMap because the project view renders these in declaration
order.
- Map<String, SpecialFolder> result = new LinkedHashMap<>();
- result.put("conf", new SpecialFolder(AllIcons.Nodes.ConfigFolder,
NodeWeights.CONFIG_FOLDER, "Configuration"));
- result.put("views", new SpecialFolder(GroovyMvcIcons.Gsp_logo,
NodeWeights.VIEWS_FOLDER, "Views"));
- result.put("init", new SpecialFolder(AllIcons.Nodes.ConfigFolder,
NodeWeights.CONFIG_FOLDER - 1, "Initialization"));
- return Map.copyOf(result);
+ return Map.of(
+ "conf", new SpecialFolder(AllIcons.Nodes.ConfigFolder,
NodeWeights.CONFIG_FOLDER, "Configuration"),
+ "views", new SpecialFolder(GroovyMvcIcons.Gsp_logo,
NodeWeights.VIEWS_FOLDER, "Views"),
+ "init", new SpecialFolder(AllIcons.Nodes.ConfigFolder,
NodeWeights.CONFIG_FOLDER - 1, "Initialization"),
+ "i18n", new SpecialFolder(AllIcons.FileTypes.Properties,
NodeWeights.TRANSLATIONS_FOLDER, "Translations"),
+ // utils is generated in the Grails 7 app skeleton; it holds user Codec
classes.
Review Comment:
grails-forge (start.grails.org) doesn't create `grails-app/utils`. On 8.0.x,
`GrailsBase` only adds `src/main/groovy`, `src/test/groovy`,
`src/integration-test/groovy`, `services`, `domain` and `taglib`. The
`utils/.gitkeep` comes from the legacy `grails-profiles/web/skeleton`.
Recognising `utils` is still worthwhile, but this comment gives the wrong
reason.
##########
plugin/src/test/java/org/apache/grails/intellij/plugin/projectView/impl/GrailsAppNodeProviderTest.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.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.projectView.impl.nodes.PsiFileSystemItemFilter;
+import com.intellij.ide.util.treeView.AbstractTreeNode;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiDirectory;
+import com.intellij.psi.PsiManager;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import
org.apache.grails.intellij.plugin.artefact.api.GrailsDisplayableArtefactHandler;
+import
org.apache.grails.intellij.plugin.artefact.impl.ControllerArtefactHandler;
+import org.apache.grails.intellij.plugin.artefact.impl.DomainArtefactHandler;
+import
org.apache.grails.intellij.plugin.artefact.impl.InterceptorArtefactHandler;
+import org.apache.grails.intellij.plugin.artefact.impl.ServiceArtefactHandler;
+import org.apache.grails.intellij.plugin.artefact.impl.TaglibArtefactHandler;
+import org.apache.grails.intellij.plugin.projectView.NodeWeights;
+import
org.apache.grails.intellij.plugin.projectView.nodes.GrailsApplicationNode;
+import
org.apache.grails.intellij.plugin.projectView.nodes.GrailsArtefactHandlerNode;
+import
org.apache.grails.intellij.plugin.projectView.nodes.GrailsPsiDirectoryNode;
+import
org.apache.grails.intellij.plugin.projectView.nodes.OtherGrailsAppSourcesNode;
+import org.apache.grails.intellij.plugin.structure.GrailsApplication;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+import javax.swing.Icon;
+
+public class GrailsAppNodeProviderTest extends GrailsNodeProviderTestSupport {
+
+ public void testRendersTranslationsAndAssetSubfolderNodes() {
+ addAssetAndTranslationFixture();
+
+ Collection<AbstractTreeNode<?>> nodes = new
GrailsAppNodeProvider().createNodes(testApplication(true),
ViewSettings.DEFAULT);
+
+ GrailsPsiDirectoryNode translations = findNode(nodes, "i18n");
+ assertNotNull("Translations node must be present", translations);
+ assertEquals(NodeWeights.TRANSLATIONS_FOLDER,
translations.getNodeWeight());
+ assertSame(AllIcons.FileTypes.Properties, translations.getNodeIcon());
+
+ GrailsPsiDirectoryNode stylesheets = findNode(nodes, "stylesheets");
+ assertNotNull("Stylesheets node must be present", stylesheets);
+ assertEquals(NodeWeights.STYLESHEETS_FOLDER, stylesheets.getNodeWeight());
+ assertSame(AllIcons.FileTypes.Css, stylesheets.getNodeIcon());
+
+ GrailsPsiDirectoryNode images = findNode(nodes, "images");
+ assertNotNull("Images node must be present", images);
+ assertEquals(NodeWeights.IMAGES_FOLDER, images.getNodeWeight());
+ assertSame(AllIcons.FileTypes.Image, images.getNodeIcon());
+
+ GrailsPsiDirectoryNode javascripts = findNode(nodes, "javascripts");
+ assertNotNull("JavaScripts node must be present", javascripts);
+ assertEquals(NodeWeights.JAVASCRIPTS_FOLDER, javascripts.getNodeWeight());
+ assertSame(AllIcons.FileTypes.JavaScript, javascripts.getNodeIcon());
+
+ GrailsPsiDirectoryNode utils = findNode(nodes, "utils");
+ assertNotNull("Utils node must be present", utils);
+ assertEquals(NodeWeights.UTILS_FOLDER, utils.getNodeWeight());
+ assertSame(AllIcons.Nodes.Class, utils.getNodeIcon());
+
+ GrailsPsiDirectoryNode migrations = findNode(nodes, "migrations");
+ assertNotNull("Migrations node must be present", migrations);
+ assertEquals(NodeWeights.MIGRATIONS_FOLDER, migrations.getNodeWeight());
+ assertSame(AllIcons.Nodes.DataSchema, migrations.getNodeIcon());
+ }
+
+ public void testRendersTitlesForNewNodes() {
+ addAssetAndTranslationFixture();
+
+ Collection<AbstractTreeNode<?>> nodes = new
GrailsAppNodeProvider().createNodes(testApplication(true),
ViewSettings.DEFAULT);
+
+ assertPresentation(findNode(nodes, "i18n"), "Translations",
AllIcons.FileTypes.Properties);
+ assertPresentation(findNode(nodes, "utils"), "Utils",
AllIcons.Nodes.Class);
+ assertPresentation(findNode(nodes, "migrations"), "Migrations",
AllIcons.Nodes.DataSchema);
+ assertPresentation(findNode(nodes, "images"), "Images",
AllIcons.FileTypes.Image);
+ assertPresentation(findNode(nodes, "javascripts"), "JavaScripts",
AllIcons.FileTypes.JavaScript);
+ assertPresentation(findNode(nodes, "stylesheets"), "Stylesheets",
AllIcons.FileTypes.Css);
+ }
+
+ public void testMissingAssetSubfolderProducesNoNode() {
+ myFixture.addFileToProject("grails-app/i18n/messages.properties", "a=b");
+ myFixture.addFileToProject("grails-app/assets/stylesheets/app.css", "h1
{}");
+
+ Collection<AbstractTreeNode<?>> nodes = new
GrailsAppNodeProvider().createNodes(testApplication(true),
ViewSettings.DEFAULT);
+
+ assertNotNull("Stylesheets node must be present", findNode(nodes,
"stylesheets"));
+ assertNull("no Images node when assets/images is absent (FR7)",
findNode(nodes, "images"));
Review Comment:
Nit: `FR4`/`FR7`/`FR8` here and further down refer to a requirements doc
that isn't in the repo. Please drop them from the assertion messages.
##########
plugin/src/test/java/org/apache/grails/intellij/plugin/projectView/impl/GrailsAppNodeProviderTest.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.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.projectView.impl.nodes.PsiFileSystemItemFilter;
+import com.intellij.ide.util.treeView.AbstractTreeNode;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiDirectory;
+import com.intellij.psi.PsiManager;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import
org.apache.grails.intellij.plugin.artefact.api.GrailsDisplayableArtefactHandler;
+import
org.apache.grails.intellij.plugin.artefact.impl.ControllerArtefactHandler;
+import org.apache.grails.intellij.plugin.artefact.impl.DomainArtefactHandler;
+import
org.apache.grails.intellij.plugin.artefact.impl.InterceptorArtefactHandler;
+import org.apache.grails.intellij.plugin.artefact.impl.ServiceArtefactHandler;
+import org.apache.grails.intellij.plugin.artefact.impl.TaglibArtefactHandler;
+import org.apache.grails.intellij.plugin.projectView.NodeWeights;
+import
org.apache.grails.intellij.plugin.projectView.nodes.GrailsApplicationNode;
+import
org.apache.grails.intellij.plugin.projectView.nodes.GrailsArtefactHandlerNode;
+import
org.apache.grails.intellij.plugin.projectView.nodes.GrailsPsiDirectoryNode;
+import
org.apache.grails.intellij.plugin.projectView.nodes.OtherGrailsAppSourcesNode;
+import org.apache.grails.intellij.plugin.structure.GrailsApplication;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+import javax.swing.Icon;
+
+public class GrailsAppNodeProviderTest extends GrailsNodeProviderTestSupport {
+
+ public void testRendersTranslationsAndAssetSubfolderNodes() {
+ addAssetAndTranslationFixture();
+
+ Collection<AbstractTreeNode<?>> nodes = new
GrailsAppNodeProvider().createNodes(testApplication(true),
ViewSettings.DEFAULT);
+
+ GrailsPsiDirectoryNode translations = findNode(nodes, "i18n");
+ assertNotNull("Translations node must be present", translations);
+ assertEquals(NodeWeights.TRANSLATIONS_FOLDER,
translations.getNodeWeight());
+ assertSame(AllIcons.FileTypes.Properties, translations.getNodeIcon());
+
+ GrailsPsiDirectoryNode stylesheets = findNode(nodes, "stylesheets");
+ assertNotNull("Stylesheets node must be present", stylesheets);
+ assertEquals(NodeWeights.STYLESHEETS_FOLDER, stylesheets.getNodeWeight());
+ assertSame(AllIcons.FileTypes.Css, stylesheets.getNodeIcon());
+
+ GrailsPsiDirectoryNode images = findNode(nodes, "images");
+ assertNotNull("Images node must be present", images);
+ assertEquals(NodeWeights.IMAGES_FOLDER, images.getNodeWeight());
+ assertSame(AllIcons.FileTypes.Image, images.getNodeIcon());
+
+ GrailsPsiDirectoryNode javascripts = findNode(nodes, "javascripts");
+ assertNotNull("JavaScripts node must be present", javascripts);
+ assertEquals(NodeWeights.JAVASCRIPTS_FOLDER, javascripts.getNodeWeight());
+ assertSame(AllIcons.FileTypes.JavaScript, javascripts.getNodeIcon());
+
+ GrailsPsiDirectoryNode utils = findNode(nodes, "utils");
+ assertNotNull("Utils node must be present", utils);
+ assertEquals(NodeWeights.UTILS_FOLDER, utils.getNodeWeight());
+ assertSame(AllIcons.Nodes.Class, utils.getNodeIcon());
+
+ GrailsPsiDirectoryNode migrations = findNode(nodes, "migrations");
+ assertNotNull("Migrations node must be present", migrations);
+ assertEquals(NodeWeights.MIGRATIONS_FOLDER, migrations.getNodeWeight());
+ assertSame(AllIcons.Nodes.DataSchema, migrations.getNodeIcon());
+ }
+
+ public void testRendersTitlesForNewNodes() {
+ addAssetAndTranslationFixture();
+
+ Collection<AbstractTreeNode<?>> nodes = new
GrailsAppNodeProvider().createNodes(testApplication(true),
ViewSettings.DEFAULT);
+
+ assertPresentation(findNode(nodes, "i18n"), "Translations",
AllIcons.FileTypes.Properties);
+ assertPresentation(findNode(nodes, "utils"), "Utils",
AllIcons.Nodes.Class);
+ assertPresentation(findNode(nodes, "migrations"), "Migrations",
AllIcons.Nodes.DataSchema);
+ assertPresentation(findNode(nodes, "images"), "Images",
AllIcons.FileTypes.Image);
+ assertPresentation(findNode(nodes, "javascripts"), "JavaScripts",
AllIcons.FileTypes.JavaScript);
+ assertPresentation(findNode(nodes, "stylesheets"), "Stylesheets",
AllIcons.FileTypes.Css);
+ }
+
+ public void testMissingAssetSubfolderProducesNoNode() {
+ myFixture.addFileToProject("grails-app/i18n/messages.properties", "a=b");
+ myFixture.addFileToProject("grails-app/assets/stylesheets/app.css", "h1
{}");
+
+ Collection<AbstractTreeNode<?>> nodes = new
GrailsAppNodeProvider().createNodes(testApplication(true),
ViewSettings.DEFAULT);
+
+ assertNotNull("Stylesheets node must be present", findNode(nodes,
"stylesheets"));
+ assertNull("no Images node when assets/images is absent (FR7)",
findNode(nodes, "images"));
+ assertNull("no JavaScripts node when assets/javascripts is absent (FR7)",
findNode(nodes, "javascripts"));
+ assertNull("no fake fonts node", findNode(nodes, "fonts"));
+ assertNull("no Migrations node when grails-app/migrations is absent",
+ findNode(nodes, "migrations"));
+ }
+
+ public void
testOtherGrailsAppSourcesNodeExcludesTranslationsAndAssetSubfolders() {
+ addAssetAndTranslationFixture();
+
+ GrailsApplication application = testApplication(true);
+ PsiDirectory appRoot =
PsiManager.getInstance(application.getProject()).findDirectory(application.getAppRoot());
+ assertNotNull(appRoot);
+
+ OtherGrailsAppSourcesNode node = new OtherGrailsAppSourcesNode(appRoot,
ViewSettings.DEFAULT);
+ node.setParent(new GrailsApplicationNode(application,
ViewSettings.DEFAULT));
+
+ Collection<AbstractTreeNode<?>> children = node.getChildrenImpl();
+ Set<String> childNames = children.stream()
+ .map(child -> child.getValue() instanceof PsiDirectory directory
+ ? directory.getName() : String.valueOf(child.getValue()))
+ .collect(Collectors.toSet());
+ assertFalse("i18n must not duplicate under Other sources",
childNames.contains("i18n"));
+ assertFalse("utils must not duplicate under Other sources",
childNames.contains("utils"));
+ assertFalse("migrations must not duplicate under Other sources",
childNames.contains("migrations"));
+
+ AbstractTreeNode<?> assetsChild = children.stream()
+ .filter(child -> child instanceof PsiDirectoryNode)
+ .map(child -> (PsiDirectoryNode)child)
+ .filter(child -> "assets".equals(child.getValue().getName()))
+ .findFirst()
+ .orElse(null);
+ assertNotNull("assets must remain under Other sources for stray files",
assetsChild);
+
+ PsiFileSystemItemFilter filter =
((PsiDirectoryNode)assetsChild).getFilter();
+ assertNotNull("assets node must carry a filter hiding the dedicated
subfolders", filter);
+ PsiDirectory assetsDir = ((PsiDirectoryNode)assetsChild).getValue();
+ assertFalse("stylesheets subfolder hidden (FR4)",
filter.shouldShow(assetsDir.findSubdirectory("stylesheets")));
+ assertFalse("images subfolder hidden (FR4)",
filter.shouldShow(assetsDir.findSubdirectory("images")));
+ assertFalse("javascripts subfolder hidden (FR4)",
filter.shouldShow(assetsDir.findSubdirectory("javascripts")));
+ assertTrue("stray file directly under assets stays visible (FR8)",
filter.shouldShow(assetsDir.findFile("extra.txt")));
+ }
+
+ public void testContainsExcludesHiddenAssetSubfoldersAndSpecialFolders() {
+ addAssetAndTranslationFixture();
+ myFixture.addFileToProject("grails-app/views/index.gsp", "<html/>");
+ myFixture.addFileToProject("grails-app/assets/fonts/webfont.woff",
"stray-font");
+
+ GrailsApplication application = testApplication(true);
+ PsiDirectory appRoot =
PsiManager.getInstance(application.getProject()).findDirectory(application.getAppRoot());
+ assertNotNull(appRoot);
+
+ OtherGrailsAppSourcesNode node = new OtherGrailsAppSourcesNode(appRoot,
ViewSettings.DEFAULT);
+ node.setParent(new GrailsApplicationNode(application,
ViewSettings.DEFAULT));
+
+ assertFalse("css under a hidden asset subfolder must not be claimed
(reveal dead-end)",
+
node.contains(myFixture.findFileInTempDir("grails-app/assets/stylesheets/app.css")));
+ assertFalse("messages under the extracted Translations folder must not be
claimed",
+
node.contains(myFixture.findFileInTempDir("grails-app/i18n/messages.properties")));
+ assertFalse("codecs under the extracted Utils folder must not be claimed",
+
node.contains(myFixture.findFileInTempDir("grails-app/utils/ShoutyCodec.groovy")));
+ assertFalse("changelogs under the extracted Migrations folder must not be
claimed",
+
node.contains(myFixture.findFileInTempDir("grails-app/migrations/changelog.groovy")));
+ assertFalse("views are rendered as a dedicated node, not here",
+
node.contains(myFixture.findFileInTempDir("grails-app/views/index.gsp")));
+ assertTrue("stray files directly under assets stay visible (FR8)",
+
node.contains(myFixture.findFileInTempDir("grails-app/assets/extra.txt")));
+ assertTrue("non-hidden asset subfolders stay visible",
+
node.contains(myFixture.findFileInTempDir("grails-app/assets/fonts/webfont.woff")));
+ }
+
+ public void testHandlerWeightOrderForComparator() {
+ Project project = getProject();
+ ViewSettings settings = ViewSettings.DEFAULT;
+ GrailsNodeComparator comparator = new GrailsNodeComparator(project,
"GrailsView");
+
+ GrailsArtefactHandlerNode domains = handlerNode(project,
DomainArtefactHandler.INSTANCE, settings);
+ GrailsArtefactHandlerNode services = handlerNode(project,
ServiceArtefactHandler.INSTANCE, settings);
+ GrailsArtefactHandlerNode controllers = handlerNode(project,
ControllerArtefactHandler.INSTANCE, settings);
+ GrailsArtefactHandlerNode interceptors = handlerNode(project,
InterceptorArtefactHandler.INSTANCE, settings);
+ GrailsArtefactHandlerNode taglibs = handlerNode(project,
TaglibArtefactHandler.INSTANCE, settings);
+
+ assertTrue("Domains must sort before Services",
comparator.compare(domains, services) < 0);
+ assertTrue("Services must sort before Controllers (reorder)",
comparator.compare(services, controllers) < 0);
+ assertTrue("Controllers must sort before Interceptors",
comparator.compare(controllers, interceptors) < 0);
+ assertTrue("Interceptors must sort before Tag Libraries",
comparator.compare(interceptors, taglibs) < 0);
+ assertTrue("Tag Libraries must sort last", comparator.compare(taglibs,
domains) > 0);
+ }
+
+ public void testDirectoryWeightOrderForComparator() {
Review Comment:
This test builds its nodes from the constants directly, so it checks the
order of the constants rather than what the providers produce, and it leaves
out Initialization. Sorting the output of `createNodes()` with
`GrailsNodeComparator` and asserting the resulting order would test what the
pane actually renders.
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/impl/Grails3NodeProvider.java:
##########
@@ -63,8 +78,21 @@ public class Grails3NodeProvider implements
GrailsViewNodeProvider {
PsiDirectory src = GrailsViewItems.findPsiDirectory(application, "src");
if (src != null) {
- PsiFileSystemItemFilter filter = item -> !specialDirs.contains(item) &&
GrailsViewItems.shouldShowItem(item);
+ List<PsiDirectory> testDirs = findTestSourceDirectories(src);
+ // Directories that get their own node, so src can refuse to claim them.
PsiDirectoryNode.contains()
+ // only applies a node's filter to the file and its immediate parent, so
hiding the directories from
Review Comment:
Small correction: `PsiDirectoryNode.contains()` applies the filter only to
the file itself (it calls `findFile(file)` and `findDirectory(file)` on the
same `VirtualFile`), not to its parent. The conclusion still holds: hiding a
directory doesn't stop the node from claiming what's under it, so the ancestor
check is the right fix.
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/impl/Grails3NodeProvider.java:
##########
@@ -36,13 +42,22 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
+import javax.swing.Icon;
public class Grails3NodeProvider implements GrailsViewNodeProvider {
private static final List<String> SPECIAL_FILES = List.of("build.gradle",
"settings.gradle", "gradle.properties");
private static final List<String> SPECIAL_DIRS = List.of("src/main/scripts",
"src/main/webapp");
+ /**
+ * Grails 7 kebab-case test source roots under {@code src/}. The Grails 6
camelCase roots
Review Comment:
Grails has never generated camelCase test roots.
`IntegrationTestGradlePlugin` has used `sourceFolderName =
'src/integration-test'` since Grails 3; it's the same in grails-gradle-plugin
5.3.1 (Grails 5/6) and on grails-core 8.0.x. So the kebab-case roots aren't
specific to Grails 7, and `src/integrationTest` isn't a Grails 6 convention.
The behaviour is right, because it applies to every Grails 3+ app. But this
javadoc, `testCamelCaseTestRootsStayUnderSrc` and the commit message describe a
layout no Grails version produces. Could you reword them? If the camelCase test
stays, it could be framed as "folders that aren't Grails test roots stay under
src".
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/impl/Grails3NodeProvider.java:
##########
@@ -63,8 +78,21 @@ public class Grails3NodeProvider implements
GrailsViewNodeProvider {
PsiDirectory src = GrailsViewItems.findPsiDirectory(application, "src");
if (src != null) {
- PsiFileSystemItemFilter filter = item -> !specialDirs.contains(item) &&
GrailsViewItems.shouldShowItem(item);
+ List<PsiDirectory> testDirs = findTestSourceDirectories(src);
+ // Directories that get their own node, so src can refuse to claim them.
PsiDirectoryNode.contains()
+ // only applies a node's filter to the file and its immediate parent, so
hiding the directories from
+ // src is not enough: src would still claim their contents, and Reveal
in Project View would expand
+ // src and dead-end. isAncestor(dir, dir, false) is true, so one check
covers the directories too.
+ Set<VirtualFile> lifted = liftedDirectories(specialDirs, testDirs);
+ PsiFileSystemItemFilter filter = item -> !isUnder(lifted,
item.getVirtualFile())
+ && GrailsViewItems.shouldShowItem(item);
result.add(new GrailsPsiDirectoryNode(src, settings,
NodeWeights.SRC_FOLDERS, filter));
+
+ for (PsiDirectory testDir : testDirs) {
+ Icon icon = "test".equals(testDir.getName()) ?
PlatformIcons.TEST_SOURCE_FOLDER : GroovyMvcIcons.Grails_test;
Review Comment:
These nodes have no title, so the pane shows `test`, `integration-test` and
`functional-test` as siblings of `src`, which reads like top-level project
folders. `OldGrailsNodeProvider` labels its equivalents `Tests:unit` and so on.
Consider a title or a `src/test` location hint.
##########
plugin/src/test/java/org/apache/grails/intellij/plugin/projectView/NodeWeightsTest.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+import org.junit.Test;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * GrailsNodeComparator orders directory nodes by subtracting their weights,
so two nodes sharing a
Review Comment:
`GrailsNodeComparator` returns `leftDir.getNodeWeight() -
rightDir.getNodeWeight()` directly, so tied nodes compare as 0; they don't fall
through to the platform comparator.
Two weights are also computed rather than declared in `NodeWeights`:
Interceptors (`CONTROLLERS_FOLDER + 1` = 31) and Initialization (`CONFIG_FOLDER
- 1` = 59). A new constant at 31 or 59 would tie with them without this test
noticing. Making them named constants (e.g. `INTERCEPTORS_FOLDER`,
`INIT_FOLDER`) would let the test cover them.
##########
AGENTS.md:
##########
@@ -118,7 +118,7 @@ is a pure aggregator — it owns only RAT and coverage
aggregation, no sources.
| Path | Gradle project | Description |
|------|----------------|-------------|
-| `plugin/` | `:plugin` | Main plugin: GSP language, Grails project support,
run configs. Compiles against the Community Edition API only |
+| `plugin/` | `:plugin` | Main plugin: GSP language, Grails project support,
run configs. Compiles against the Community Edition API only. Its Grails 3+
pane surfaces dedicated **Stylesheets**/**Images**/**JavaScripts**,
**Migrations**, **Translations**, **Utils** and separated Grails 7 test-root
nodes |
Review Comment:
This table describes the module layout for agents working on the build, so a
feature list doesn't belong here. I'd drop this change. The README mention is
fine, although the inserted clause makes that sentence hard to read.
##########
IMPROVEMENT-PLAN.md:
##########
@@ -195,6 +200,16 @@ Detection must work from Gradle dependency data (the
plugin already has a
init,utils,assets}` plus `src/main/groovy`, `src/test/groovy`,
`src/integration-test/groovy` (registered by `TestPhasesGradlePlugin`)
and
`src/functional-test/groovy` (functional/Geb phase) source sets.
+ **Ground truth (grails-core `8.0.x`, checked 2026-10-01):** the
generated web skeleton is
Review Comment:
This is backwards for the generator 0.2 uses. On 8.0.x, grails-forge's
`GrailsApplication` feature writes
`grails-app/init/{packagePath}/Application.groovy` and `BootStrap.groovy`, so
`init/` **is** generated, and `GrailsBase` creates no `grails-app/utils`. The
"`utils` but no `init`" picture comes from the legacy
`grails-profiles/web/skeleton`, and even there `grails-profiles/base/skeleton`
contributes `grails-app/init/.../Application.groovy`. Could you correct this
paragraph?
##########
IMPROVEMENT-PLAN.md:
##########
@@ -98,9 +98,14 @@ that Grails 3+ actually uses.
3. Grails **6.x** (legacy baseline, for the legacy plugin's regression record).
Walk this checklist per app and record works / broken / missing:
-- [ ] Project recognized as Grails; `grails-app/*` project view pane renders
-
controllers/domain/services/taglib/views/conf/i18n/**init**/**utils**/assets
- groups (init/ and utils/ are conventions the plugin may predate).
+- [x] Project recognized as Grails; `grails-app/*` project view pane renders
Review Comment:
Section 0.2 is a walkthrough done per app: generate 7.0.x, 8.0.0-SNAPSHOT
and 6.x apps and record what works, what's broken and what's missing. I'd leave
this box unticked until that's done. The "Shipped" note fits better under 2.2.
--
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]
