-
Notifications
You must be signed in to change notification settings - Fork 874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update OutputUtils to support stacktrace links leading to JDK files. #5091
Conversation
java/maven/src/org/netbeans/modules/maven/api/output/OutputUtils.java
Outdated
Show resolved
Hide resolved
e274d20
to
f5a25b0
Compare
f5a25b0
to
216bfd1
Compare
java/maven/src/org/netbeans/modules/maven/api/output/OutputUtils.java
Outdated
Show resolved
Hide resolved
java/maven/src/org/netbeans/modules/maven/classpath/ClassPathProviderImpl.java
Outdated
Show resolved
Hide resolved
java/maven/src/org/netbeans/modules/maven/classpath/ClassPathProviderImpl.java
Show resolved
Hide resolved
fe628f7
to
28f67de
Compare
java/maven/src/org/netbeans/modules/maven/classpath/ClassPathProviderImpl.java
Show resolved
Hide resolved
28f67de
to
36e8d1d
Compare
rebased to latest master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks sane to me and a quick test on JDK 8 and 11 showed, that it works both with and without modules.
One thing I'm not really sure about is the order of classpaths:
return createProxyClassPath(
Stream.concat(
Stream.of(prov.getProjectClassPaths(ClassPath.BOOT)),
Stream.of(prov.getProjectClassPaths(ClassPath.EXECUTE)))
);
consider nbjavac. When it is declared as a dependency it overrides the built-in javac although normal classloading behavior would be to prefer the boot classpath. So my assumption would be, that people having dependencies clashing with JDK classes would ensure that the classloaders are setup to prefer these. But that is a niche use-case, so feel free to disagree and merge as is.
- add active JDK to class path - fix the packageName parsing of the line action - open class file as fallback -> user may want to download source
- prefer switch over long if chain - removed dead code - simplified loops - other minor improvements
36e8d1d
to
7624b4d
Compare
@matthiasblaesing That is a good point. I thought about this before and wasn't sure either. But I think having project-cp before boot cp might be indeed the better default for stack trace linking since it covers more cases than the other way around (i think). Swapped it now and rebased to latest master for a fresh build. |
all green. merging. |
fixes #4516
second commit is for cleanup, first has the changes.
test snippet
devbuild link (expires in 7 days):
https://github.com/apache/netbeans/suites/10002051531/artifacts/485721202