search

eclipse / lsp4e

Language Server Protocol support in Eclipse IDE
Eclipse Public License 2.0
60 stars 53 forks source link

The default label for multiple declarations is not useful #973

Closed ylussaud closed 3 months ago

ylussaud commented 3 months ago

The computed label for multiple declarations is the name and the path of the resource with is not really useful as an end user.

It would be better to display the line declaring the symbol so the used can select the relevant entry. This could be done at least for IFile from the workspace since we can have the encoding and the stream to read its content.

mickaelistria commented 3 months ago

I'm not sure I understand what you're talking about. Can you please submit some screenshot of the particular UI element being discussed?

rubenporras commented 3 months ago

Yes, an screenshot with and without your PR would be very helpful. I know how it currently looks like, but I am not sure about how it would look like after

ylussaud commented 3 months ago

Before the patch: image After the patch: image I updated my commit...

rubenporras commented 3 months ago

I am unsure on this change. I see how in some cases the declaration line might be useful, but the URI might be as well quite relevant. Would the problem be solved if the dropdown menu that shows the URI would be wider?

mickaelistria commented 3 months ago

Showing the line content can be helpful, but it IMO usually seems more important to show the target location as well (and not only its content). Also not that in some cases, the first line of the declaration might not be so interesting. It could for example be the start of a comment. The line content might be a nice addition as a suffix, but we'd need more space. I think we can maybe save some space by replacing the "Open" by an arrow emoji ➡️ , Also we might be able to only show some relative paths when they're shorter; then we could have more room to append line content.

ylussaud commented 3 months ago

In my case all declarations are in the same file so there is no way to distinguish a declaration from its URI. Maybe this choice should be done by the language implementation via an extension point ? This extension point could include other labels as well.

mickaelistria commented 3 months ago

choice should be done by the language implementation via an extension point ?

I fear it's a dangerous to have extension points for it, as at such pace, we'd start maintaining dozens of extension points for everything. How does VSCode behave in this case? Maybe adding a "label" to LocationLink in the spec would be the best way forward.

ylussaud commented 3 months ago

I understand, in the mean time could:

Declaration - <resource name> - <line content>
Declaration - body.mtl - [template public generateClassifier(cls : ecore::EClassifier)]

be an option ?

rubenporras commented 3 months ago

At least for our use cases, the file name would not be enough, the folders are also relevant.

I wonder, for the cases where the URI is not enough, should one not use type hierarchy? Would that work for you?

ylussaud commented 3 months ago

No those are declarations not related to types in the language. I'll try to find a workaround...

ylussaud commented 3 months ago

I provided my own org.eclipse.ui.workbench.texteditor.hyperlinkDetectors as a workaround. I still have the hyperlink detector from LSP4E but I can live with that for the moment. For more details one can check this commit.