Roots (and directories) containing multi-file launcher sources should respond to ClassPath queries. #7733
Conversation
lahodaj
added
Java
|
I tested this with the following setup:
public class Main {
public static void main(String args[]) {
System.out.println("hello");
System.out.println(Runtime.version());
dev.test.Util.foo();
}
}
package dev.test;
public class Util {
public static void foo() {
System.out.println("foo called");
}
}and added this caused: although the error badges were sometimes not visible until I restarted NetBeans. |
I checked it and I think it does not work. Here is what I did:
The breakpoint in line 164 is hit with the home directory as I did not use the source file launcher, yet the file and the directory are now recognized. What is more I see similar reports as @mbien. I opened java files from a JDK checkout and see: |
lahodaj
force-pushed
the
multi-source-launcher-classpath-for-registered-roots
branch
from
3c0b8ba
to
ed150ca
Compare
|
I am not quite sure what exactly is (or better said was) the problem with the ClassPathProvider returning the ClassPath for the root or directory, except that debugger stopped at some line. The behavior before #7509 was I believe to return the ClassPath, and neither that PR nor the associated bug provide any clues what was the problem with that. Anyway, I've updated this patch to only return the ClassPath when the branding is set to register the roots. I hereby also apologize I didn't quite realize the full impact of #7509, that was my mistake. Regarding the: I am getting that even when running on master. Quite unsurprisingly: the file reports an error in the editor, but there are no errors in the index (since there's no index). |
|
Whether or not my finding is really a problem I did not investigate, but the hits on the breakpoints invalidated the claim that it is save because it is only relevant if multi-file launcher is used. Wouldn't it make sense to use the same approach as CPPLite? I.e. ask the user to create a "virtual" project? That way we could get rid of the hacks and use the "normal" NetBeans architecture. I'll be away from keyboard for the next week, I won't stay in the way of this, but it would be nice if new features (the problem exists since the introduction of support for source file launchers) would not break the IDE. |
could the main class not simply have a check box in the file properties to opt into a project-less compilation/execution model? I think at some point there must be some way to specify the classpath anyway, otherwise "java scripts" like https://github.com/apache/netbeans/blob/master/.github/scripts/BinariesListUpdates.java will never work properly in NB (since the jar dependencies are not necessarily in the same folder). The file properties already have JVM/program args and a JDK selector #7605 |
What I meant was that the root must first be discovered through a file. But I admit I wrote that ambiguously.
I don't recall any positive feedback on CPPLite (but plenty, plenty of negative). So no, forcing the users to create manually a project does not sound like a good idea, unless there are piles of positive feedback on CPPLite hiding somewhere. When I was doing this, I tried to create a project automatically, but a) it was extremely complicated; b) it wouldn't solve anything if the project was created automatically anyway.
While I am trying to not break things, sometimes I do, I am afraid. The only way how one can not introduce (new) breakages is to do nothing (and live with the existing breakages). |
From my POV CPPLite suffers from the problem, that you need to configure much more, than just the root of the project. You'll need the metadata file, that described compilation. I tried once to get small step improvements by allowing to use the language server without additional configuration, which was rejected. The same does not apply to this kind of java project. |
mbien
added
the
ci:dev-build
Hm, that should be doable, thanks! |
|
Here: |
There was a problem hiding this comment.
works really well!
error badges went away right after I checked the box (same test from #7733 (comment)), the property will also only show on source files and only outside of projects.
There are probably some UI tweaks we could do later, e.g grayed out checkbox to indicate that a file is already in a source root - but I think this has lower priority. (maybe even a visual indicator on the icon?)
The opt-in action is also an excellent place to add context sensitive warnings for situations when the user is about to index his/her entire home folder ;)
There was a problem hiding this comment.
In general this is looks like a good to me.
I don't think the following is in scope for this change, so I'll describe it, maybe one of the readers has an idea:
- I ran
ant trymewith a cleantestuserdir - in my
tmpfolder I placed a filescratch.java - I selected a JDK in the properties of the file
After that I observe indexing starting. I traced it into the APTUtils, which are queried by the JavacParser which is queried by the JavaNode. As a side effect indexing is invoked on attribute changes.
… respond to ClassPath queries.
lahodaj
force-pushed
the
multi-source-launcher-classpath-for-registered-roots
branch
from
ef4a2e5
to
38cf35a
Compare
|
Unless there are objections, I'll integrate in the next day or so. |
The multi-file source launcher support has been tweaked under:
#7509
to only respond to files that are source launcher files. But, that means the queries no longer respond when asked about directories, especially the root. But, the Java indexing, and other features, need the ClassPath to work also on the root directories. As a consequence, the multi-file source launcher is now broken.
The proposal herein is to respond not only for source launcher files as before, but also for enclosing directories up to the package hierarchy root. This will happen only if the root has already been recognized, so it should not alter cases where source file launcher is not used.