Skip to content

Bugfix: Go To source broken for nested classes.#3189

Merged
lkishalmi merged 1 commit intoapache:masterfrom
ratcashdev:bugfix/jira-6041
Feb 19, 2022
Merged

Bugfix: Go To source broken for nested classes.#3189
lkishalmi merged 1 commit intoapache:masterfrom
ratcashdev:bugfix/jira-6041

Conversation

@ratcashdev
Copy link
Contributor

This PR is part of the fixes needed for https://issues.apache.org/jira/browse/NETBEANS-6041
This is a fix for gradle projects, but since it's also broken for MAVEN (and possible standard netbeans projects) might need to be refactored to a different common module to fix it everywhere at once. Not sure what would be the best place.

@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:bug Bug report or fix labels Jan 25, 2022
@mbien mbien added this to the NB14 milestone Jan 25, 2022
@mbien
Copy link
Member

mbien commented Jan 25, 2022

added milestone NB14 to keep this on the radar.

@ratcashdev
Copy link
Contributor Author

NB14? ...been waiting for 4 months. Could have been included already in 12.6

@mbien
Copy link
Member

mbien commented Jan 25, 2022

NB14? ...been waiting for 4 months. Could have been included already in 12.6

probably, but nobody reviewed it yet unfortunately. NB14 is the next realistic milestone, NB13 would be still possible in theory but this would have to get reviewed quickly and rebased on top of the delivery branch since NB13 is in the rc phase.

If you really want to get a PR in and see no activity here, dropping a mail to the dev list can sometimes help to get some eyes on it.

Thanks for sticking around.

@mbien mbien requested a review from lkishalmi January 25, 2022 13:30
@ratcashdev
Copy link
Contributor Author

will do next time. Tried to make it clear in the jira ticket that there's a PR, but that, apparently, was left unattended too.

@lkishalmi
Copy link
Contributor

I'm sorry, I've totally missed this one.

@mbien mbien added the Gradle [ci] enable "build tools" tests label Jan 25, 2022
@lkishalmi
Copy link
Contributor

Well, updated this one from master, documented the API changes. Let's see what CI is telling about this one.

*/
public static Location parseCanonicalLocation(String loc) {
if (loc == null) {
return new Location(null, null, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we expecting null as an argument?
wouldn't be better to throw an NPE, or just do an assert loc != null;
Or return a null instead of a Location.

if ((first != null) && (first instanceof GradleTestMethodNode)) {
GradleTestMethodNode node = (GradleTestMethodNode) first;
Location loc = new Location(node.getTestLocation().getFileName());
Location loc = node.getTestLocation().withNoTarget();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this with tab characters? I hope you are not using tabs to indent the source code.

Copy link
Contributor Author

@ratcashdev ratcashdev Feb 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, yes. With today's merge tools and their ability to hide whitespace, formatting became much less of a concern for me.
Thanks for putting it into shape and merging. So this is... for 14. Do you think it is compatible with 13 too?

@lkishalmi lkishalmi merged commit 7ae4452 into apache:master Feb 19, 2022
@ratcashdev
Copy link
Contributor Author

@lkishalmi do you think this can be built out of the current master and dropped into 13.0 as a jar?

@lkishalmi
Copy link
Contributor

Not really. Though you can try to build nbm-s out of the gradle,java and gradle.junit modules, though you may need to build the gradle main module as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gradle [ci] enable "build tools" tests Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:bug Bug report or fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants