jdaugherty commented on code in PR #432:
URL:
https://github.com/apache/grails-intellij-plugin/pull/432#discussion_r4197979594
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/nodes/GrailsPsiDirectoryNode.java:
##########
@@ -83,6 +100,27 @@ public int getNodeWeight() {
protected void updateImpl(@NotNull PresentationData data) {
super.updateImpl(data);
if (nodeIcon != null) data.setIcon(nodeIcon);
- if (nodeTitle != null) data.setPresentableText(nodeTitle);
+ if (nodeLocation != null) data.setLocationString(nodeLocation);
+ }
+
+ /**
+ * The title is written as a coloured fragment here, not with {@code
setPresentableText} in
+ * {@code updateImpl}, because the renderer draws {@code PresentationData}'s
fragment list and
+ * {@code PsiDirectoryNode} fills that list with the directory name — the
qualified path, such as
Review Comment:
The mechanism described here (and in the AGENTS.md "Project view gotchas"
section and the `GrailsPsiDirectoryNodeTest` javadoc) isn't what the platform
does. In `PsiDirectoryNode.updateImpl` on 262.10315.125, coloured fragments are
added **only** when the directory is a module content root
(`ProjectRootsUtil.isModuleContentRoot`). For every other directory it calls
`setPresentableText(name)` and adds no fragments, which is why the existing
Views/Configuration/Initialization titles always rendered without this hook.
The qualified `grails-app.i18n` label comes from
`ProjectViewDirectoryHelper.getNodeName` via `setPresentableText`, not from a
fragment.
The reason `Tests:unit` didn't render is that Gradle's per-source-set
modules make `src/test` a module content root, so the content-root branch ran
and wrote `test [app.test]` as fragments, which the renderer prefers over
`presentableText`.
The fix is right for both cases, and I checked that `NodeRenderer` still
merges the error-highlight `textAttributesKey` into the first fragment, so
problem files beneath a titled node still colour it. Two asks:
- State the content-root cause here, in AGENTS.md and in the test javadoc,
so the next reader doesn't go looking for fragments on nested directories.
- `testTitleReplacesTheDirectoryNameFragments` uses `grails-app/i18n`, where
no fragments exist before `postprocess`, so it doesn't exercise the case that
failed in the IDE. Registering the directory as a content entry
(`ModuleRootModificationUtil.updateModel(module, m -> m.addContentEntry(dir))`)
would make the platform write the fragments first and prove they get replaced.
##########
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:
The named constants are in (`INTERCEPTORS_FOLDER`, `INIT_FOLDER`) and the
value-pinning test covers them, thanks. One leftover: the class javadoc still
says tied weights "fall through to the platform comparator".
`GrailsNodeComparator` returns `leftDir.getNodeWeight() -
rightDir.getNodeWeight()` and never reaches its delegate for two
`GrailsPsiDirectoryNode`s, so a tie compares as 0 and the order is unspecified.
The comment you added in `Grails3NodeProvider` says exactly that, so the two
currently disagree. Could you align this javadoc with it?
##########
AGENTS.md:
##########
@@ -222,6 +258,8 @@ Special packaging: `plugin/standardDsls/` sits outside the
resource roots and is
| Wrong JDK / build fails to configure | `sdk env` (JDK pinned in `.sdkmanrc`,
no toolchain) |
| RAT failure on a new file | Add the Apache license header; excludes need a
justification |
| A feature "missing" after switching plugin builds | Rebuild before judging —
hit the Gradle refresh icon (or `./gradlew buildPlugin`) so the sandbox picks
up the new classes. A stale build can make working code look broken, and the
Grails project view pane is the usual tell because it is only added once an
application is detected |
+| A project-view node shows the wrong label, or a filter change has no visible
effect | Read "Project view gotchas" above before changing anything in
`projectView/` — the renderer reads `PresentationData`'s coloured fragments,
not `presentableText`, and the light fixture cannot see this class of bug |
+| `sdk env` leaves Gradle on the wrong JDK, or `instrumentCode` fails with
`UnsupportedClassVersionError` | `.sdkmanrc` pins a JDK version that may not be
installed (e.g. `25.0.3-librca`); `sdk env` then silently keeps the old
default. Set `JAVA_HOME` to a matching JDK explicitly |
Review Comment:
This row isn't related to the PR and the claim isn't accurate: when the
pinned version is missing, `sdk env` prints `Stop! java <version> is not
installed.` in red, suggests `sdk env install`, and returns 1, so nothing is
silent. Please drop it.
--
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]
