Skip to content
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

Use StructureProvider to populate navigator #7483

Merged
merged 6 commits into from
Jun 19, 2024

Conversation

jtulach
Copy link
Contributor

@jtulach jtulach commented Jun 14, 2024

We at Enso are finally adding VSCode outline view into Enso VSCode extension:

We had to write our own EnsoStructure provider. As the snapshots in the pull request demonstrate it is working fine in VSCode.

However: I also want it to work in NetBeans and IGV!

I see little point writing own navigator implementation and registering it in the Enso NBM. Especially when my original vision of the LSP API saw it as a way to hook into VSCode as well as NetBeans by writing one implementation and using it in both systems. This PR is my attempt to contribute to that vision.

  • Existing lsp.client module now checks for presence of StructureProvider
  • When found, it extracts the StructureElement hierarchy and builds nodes for the elements

The functionality is very close to the already existing functionality that works via LSP4J interfaces. Maybe there is a way to share the navigators - either by abstracting the functionality to common interfaces or by bridging (now deprecated) org.eclipse.lsp4j.SymbolInformation to StructureElement - what do you think? Abstracted common functionality in 47d411b and using it in edf081f

In any case, I'd be thankful if NetBeans recognized various LSP API providers and used them in the classical UI where appropriate. This is how NetBeans look with this PR and Enso4Igv NBM version v1.36.114:

Enso Structure in NetBeans 22+

@jtulach jtulach added LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests needs:discussion Requires discussion on the NetBeans dev mailing list labels Jun 14, 2024
@jtulach jtulach self-assigned this Jun 14, 2024
@jtulach jtulach added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Jun 14, 2024
@jtulach jtulach removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Jun 17, 2024
@ppisl
Copy link
Member

ppisl commented Jun 18, 2024

Good idea. At first glance, it seems good.

Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

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

I am not much opposed to reusing the StructureProvider, but won't this mean we will get the dynamic LSP panel for all MIME types that have StructurePovider, in addition to their existing navigator panels?

@jtulach
Copy link
Contributor Author

jtulach commented Jun 19, 2024

I am not much opposed to reusing the StructureProvider,

Great. I am glad you find the new dependency on lsp.api OK.

but won't this mean we will get the dynamic LSP panel for all MIME types that have StructureProvider,
in addition to their existing navigator panels?

That was the case. Maybe it is not that bad, especially when the StructureProvider is listed as the last option. But for now, I am disabling this StructureProvider view when there is normal Navigator/Panels/mime/type registration.

Since 7ef43e0 we display StructureProvider navigator only when there is no better navigator showing.

@jtulach jtulach removed the needs:discussion Requires discussion on the NetBeans dev mailing list label Jun 19, 2024
@jtulach jtulach merged commit d0dc68a into apache:master Jun 19, 2024
33 checks passed
@mbien mbien added this to the NB23 milestone Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants