Bug with lsp imenu processing (maybe golang only?)

jtl5770 commented 2 years ago

Thank you for the bug report

[X] I am using the latest version of lsp-mode related packages.
[X] I checked FAQ and Troubleshooting sections
[x] You may also try reproduce the issue using clean environment using the following command
```
emacs -q -l lsp-start-plain.el
```
where lsp-start-plain.el can be downloaded here.

Bug description

Using lsp-ui-imenu, while editing golang files, I encountered the following problem: Please note how the Sensor struct is printed before the "Structs" header corrupting the display.

Screenshot-lsp-ui-imenu

This happens whenever the custom option lsp-imenu-index-function is different from it's default value lsp-imenu-create-uncategorized-index (in this case: lsp-imenu-create-top-level-categorized-index, but similar looking with lsp-imenu-categorized-index). Note the following image where it is set to this default value:

Screenshot-lsp-ui-imenu-index-function:uncategorized

Here it is essentially correct, but ofc missing lots of structural information.

Comparing this to lsp-treemacs-symbols, it seems that this ignores the value of the lsp-imenu-index-function, it always looks like this:

Screenshot-lsp-treemacs-symbols

essentially the same as the information of the default lsp-ui-imenu display. Interestingly, trying to use the main lsp-treemacs display to click down into the file structure also reveals some weird display and behavior:

Screenshot-lsp-treemacs-main

one can see, that here the Sensor and Trigger structs are inserted as part of a "Functions" parent, which clearly is wrong. Maybe this is related to the same root cause of not being able to process tree of symbols correctly.

Steps to reproduce

I have attached a small self contained main package go file that I have used to try out some stuff while learning the language :-) it shows the same problem when setting lsp-imenu-index-function to lsp-imenu-categorized-index

Just open it with gopls server installed and use lsp-ui-imenu.

main.zip

Expected behavior

The expected behavior of lsp-ui-imenu is to construct the symbol tree correctly even when setting lsp-imenu-index-function to e.g. lsp-imenu-create-categorized-index.

Even better: lsp-treemacs-symbols respects that variable (and constructs a correct tree), and lsp-treemacs main display would not generate weird parent/child relations.

In the long run, a good solution would also group the methods belonging to one struct together with/below this struct, instead of lumping together all functions and methods.

Which Language Server did you use?

gopls

OS

Linux

Error callstack

No response

Anything else?

No response

jtl5770 commented 2 years ago

I have now reproduced the problem also with the clean environment (see screenshot below), this time I used two structs with methods and two "normal" functions, and the setting of "lsp-imenu-create-categorized-index" for lsp-imenu-index-function.

Screenshot-vanilla-emacs

jtl5770 commented 2 years ago

Some other things I found out: When setting the placement of "Lsp Ui Imenu Kind Position" from top to left, the following picture is shown

Screenshot-lsp-ui-imenu-left

This surely reduces the problem to two aspects only:

The bug is where I made the red circle - this gap should have been closed. A Fields header could be used in the left area, too to be consistent (in Front of the blue "Bar struct(...)" beginning it's own section). This would be a problem with lsp-ui most probably, as treemacs is getting it somehow right under certain conditions)
The next one is maybe more a missing feature than a bug: I would rather like to see the simply the structs opening up and showing all theirs field PLUS all the methods that belong to them, instead of listing just all the methods together.

Now, in the first comment I was showing what seems to be an inconsistent display of fields and methods in the main treemacs display. In the supplied screenshot it looks already weird, but what drove me crazy is that it seemed to change what it was displaying over time - sometimes it showed a "type" and "func" node, then again it was "Functions", and sometimes even "methods" and "fields". And next time all was back to just "func" and "Type"

It turned out that it depends on if lsp-imenu has been called or not on the file in question. It seems, that the initial way for treemacs is to display "func" and "type" nodes. After lsp-ui-imenu has been called with its default options, that is replaced with a "Functions" node where the structs are put wrongly below. But, if I call lsp-ui-imenu once on a buffer with lsp-imenu-index-function set to lsp-imenu-create-categorized-index, this shown in lsp-treemacs:

Screenshot-lsp-treemacs-better

...which looks rather nice, although the problem of the methods being grouped together is still there. The best actually for me would be to combine all of methods, fields and structs under one "Type" node which could be expanded to show both the fields and the methods of the Type (in the case of a struct type). Interfaces could be put there, too.

jtl5770 commented 2 years ago

Now, the other problem that remains is: How to get lsp-treemacs to directly display it's inline symbols in the way they look without having to call lsp-ui-imenu with the correct options set? I did add a call into go-mode-hook for lsp-imenu-create-categorized-index. This will bring the desired results after the file is loaded the first time. Unfortunately this means that browsing around the tree into files that have not been loaded will still show just the basic "func" and "type" nodes, not the categorized view. Anybody knows of a way to make treemacs aware of using lsp-imenu-create-categorized-index right from the start?

michaelpj commented 2 years ago

I think this might be the same as an odd issue I noticed with Haskell: https://github.com/haskell/haskell-language-server/issues/2103

I couldn't reproduce it reliably, but as you say, maybe it depends on whether certain functions have been called yet or not?

jtl5770 commented 2 years ago

Well, I just realized I wouldn't even be able to decide if a screenshot of haskell code related stuff looks funky or OK :-) But maybe you can follow what I did and play around with the different custom options in lsp-imenu and lsp-ui-imenu group. Maybe that shows (or disproofs) a connection to my problem.

yyoncho commented 2 years ago

I don't see sensor struct in main.go. Please provide matching screenshots and file to reproduce the issue. If the issue is the visuals of lsp-ui (mismatched location) then it should be moved in lsp-ui package.

yyoncho commented 2 years ago

FTR treemacs will show different stuff if lsp is not initialized in a file because it does not initialize the lsp-mode's imenu function.

jtl5770 commented 2 years ago

Hi @yyoncho - sorry for not using the right file for the first comment. But all the screenshots in the following comments (done with a "clean" emacs as explained) are showing the main.go file as attached.

And yes, it's about the visual representation. How can this report be moved to lsp-ui then? Who/where would be the right place to ask about the feature to sort methods with their respective structs? This most probably isn't lsp-ui related

About the treemacs part of the question: is there any way to have it use the lsp supplied imenu builder right from the start?

emacs-lsp / lsp-ui