lexical-lsp / lexical

Lexical is a next-generation elixir language server
874 stars 80 forks source link

Feature request: Include def or defp in document symbols #701

Closed crbelaus closed 5 months ago

crbelaus commented 5 months ago

The document symbols provider is not showing any information that allows you to identify whether a function is public or private.

The following is a screenshot from VSCode with Lexical showing all the symbols in a particular module. Only changeset/2 and url/2 are public functions, while the rest are private.

image

For comparison, this is the same module with ElixirLS in this case. Notice the def and defp before the function names.

image

I'm just opening this issue as a potential feature request open for discussion. I personally find this functionality very useful as it allows me to easily see the public interface of a particular module without having to perform a def search.

scohen commented 5 months ago

@scottming suggested using different icons for public and private functions, but they're actually meant to distinguish between methods and functions and don't have any intrinsic meaning.

scohen commented 5 months ago

@crbelaus How about if we put a little lock icon in front of private functions? Like this:
Screenshot 2024-04-17 at 2 33 56 PM

scohen commented 5 months ago

I tried making public functions function kinds and private functions method kinds, but in emacs at least, they use the same icon.

Terbium-135 commented 5 months ago

Personally I'd prefer written text def/defp instead of another symbol in front. Second symbol looks overloaded to me. But this is UI and everyone to his taste. Anything will do a long as there is something at all

scottming commented 5 months ago

@crbelaus How about if we put a little lock icon in front of private functions? Like this:

I favor the small lock icon: : \uea75; it is very effective for quick visual recognition, and it will be easy to remove or modify when the official LSP adds more symbol tags in the future. Although 'def' and 'defp' are somewhat helpful in searches, they are a distraction when viewing symbols.(And I think we should rarely use prefixes def/defp to search for document symbols; more often, we need quick visual recognition)

crbelaus commented 5 months ago

The proposed small lock icon sounds good 👍

I mentioned def and defp on the issue because that is what ElixirLS does but any other solution that helps identifying public and private functions is great.

zachallaun commented 5 months ago

I'd prefer def/defp as it naturally extends to including defmacro and defmacrop as well. It would also be helpful to distinguish between functions and macros, and the lock does not give any of that info. (Workspace symbols doesn't currently include macro definitions but I expect that it will in short order.)

scohen commented 5 months ago

macros will have a different icon

scohen commented 5 months ago

I worry about defmacro/defmacrop + the function name + the arguments will be a lot of noise in the sidebar.

zachallaun commented 5 months ago

I worry about defmacro/defmacrop + the function name + the arguments will be a lot of noise in the sidebar.

@scohen Devil's advocate: I like that types, for instance, include @type before them. If I'm looking at symbols and only care about types, I can quickly type @type to narrow down to only those. The same could be useful for defs/defmacros.

scohen commented 5 months ago

That's fair and this stuff is super easy to change. Emacs just displays these things, i can't narrow them down at all, so that reasoning wasn't obvious to me.