Skip to content

[NETBEANS-3362] Show inherited members on the navigator pane - #3296

Merged
junichi11 merged 2 commits into
apache:masterfrom
junichi11:netbeans-3362-cls-navigation-inherited-filter
Nov 20, 2021
Merged

[NETBEANS-3362] Show inherited members on the navigator pane#3296
junichi11 merged 2 commits into
apache:masterfrom
junichi11:netbeans-3362-cls-navigation-inherited-filter

Conversation

@junichi11

Copy link
Copy Markdown
Member

netbeans-3362-show-inherited-members-on-navigator-pane

@junichi11 junichi11 added API Change [ci] enable extra API related tests PHP [ci] enable extra PHP tests (php/php.editor) CSL [ci] enable web job labels Nov 2, 2021
@junichi11 junichi11 added this to the NB13 milestone Nov 2, 2021
@junichi11
junichi11 requested a review from sdedic November 2, 2021 15:16
@junichi11

junichi11 commented Nov 2, 2021

Copy link
Copy Markdown
Member Author

@sdedic Could you please take a look at the CSL part (the first commit)?

@junichi11
junichi11 force-pushed the netbeans-3362-cls-navigation-inherited-filter branch from 1c912ab to aee7893 Compare November 3, 2021 03:10
@junichi11

junichi11 commented Nov 4, 2021

Copy link
Copy Markdown
Member Author

Umm, there are problems with other features(overriding method, bread crumb). Will investigate it...

@junichi11
junichi11 marked this pull request as draft November 4, 2021 11:21
@junichi11
junichi11 force-pushed the netbeans-3362-cls-navigation-inherited-filter branch from aee7893 to 7f9aced Compare November 5, 2021 00:25
@junichi11
junichi11 marked this pull request as ready for review November 5, 2021 00:57
@junichi11

junichi11 commented Nov 5, 2021

Copy link
Copy Markdown
Member Author

@junichi11
junichi11 force-pushed the netbeans-3362-cls-navigation-inherited-filter branch from 7f9aced to 28c4692 Compare November 5, 2021 02:54
while (!todo.isEmpty()) {
StructureItem i = todo.remove(0);
if (i instanceof StructureItem.InheritedItem
&& ((StructureItem.InheritedItem) i).isInherited()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rather than spreading these instance checks all over the place, there could be a helper method:

public static boolean isInherited(StructureItem i) {
}

it could actually be in StructureItem interface itself now, when it is possible to have static methods in interfaces.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you for your review! Will do that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@JaroslavTulach JaroslavTulach left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fair use of extending interfaces.

@sdedic

sdedic commented Nov 12, 2021

Copy link
Copy Markdown
Member

Sigh. I had a PR review pending, but unpublished. @JaroslavTulach review's notification made me look & discover the draft. Sorry for the delay :(

Comment thread ide/csl.api/src/org/netbeans/modules/csl/api/StructureItem.java
Comment thread ide/csl.api/src/org/netbeans/modules/csl/navigation/base/FiltersManager.java Outdated
@junichi11

Copy link
Copy Markdown
Member Author

@sdedic Don't worry about it :) I guessed you are so busy. Thank you for your review!

@junichi11
junichi11 force-pushed the netbeans-3362-cls-navigation-inherited-filter branch from 28c4692 to 413de55 Compare November 12, 2021 12:03
@junichi11

Copy link
Copy Markdown
Member Author

@JaroslavTulach @sdedic I've fixed the points that were pointed out. Please let me know if there are any problems. Thanks!

@junichi11
junichi11 requested a review from tmysik November 16, 2021 00:07
@junichi11

Copy link
Copy Markdown
Member Author

@tmysik Could you please review the PHP part?

@tmysik tmysik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The PHP part looks OK to me. Thanks a lot for your great work, @junichi11!

@junichi11

Copy link
Copy Markdown
Member Author

@tmysik Thank you for your review!

@junichi11
junichi11 force-pushed the netbeans-3362-cls-navigation-inherited-filter branch from 413de55 to 89b415b Compare November 18, 2021 03:10
* @return the declaring element handle of the inherited item
*/
@NonNull
ElementHandle getDeclaringElement();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Generally, I suppose this doesn't return null.

@junichi11

Copy link
Copy Markdown
Member Author

Will merge this. Thanks!

@junichi11
junichi11 merged commit 07639d1 into apache:master Nov 20, 2021
@junichi11
junichi11 deleted the netbeans-3362-cls-navigation-inherited-filter branch November 20, 2021 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Change [ci] enable extra API related tests CSL [ci] enable web job PHP [ci] enable extra PHP tests (php/php.editor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants