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

Find References #405

Merged
merged 7 commits into from Oct 31, 2023
Merged

Find References #405

merged 7 commits into from Oct 31, 2023

Conversation

scohen
Copy link
Collaborator

@scohen scohen commented Oct 11, 2023

Added initial implementation of find references. It presently works for any module-like thing that can be recognized by Entity.resolve, like Modules, aliased modules, __MODULE__ and structs.

Note: This will not work reliably until indexing is re-enabled, so if you want to test locally, you'll need to revert a170acb

@zachallaun
Copy link
Collaborator

Shouldn't we be able to start the store for these tests?

@scottming
Copy link
Collaborator

I just found the indexer breaks the multiple nodes: if we enable the indexer, we can't use multiple editors to open the same project concurrently.

20:43:50.374 [error] Child LXical.Document.Store of Supervisor LXical.Server.Supervisor terminated
** (exit) killed
Pid: #PID<0.135.0>
Start Call: LXical.Document.Store.start_link([])
Restart: :permanent
Shutdown: 5000
Type: :worker
20:43:50.374 [error] Child LXical.Document.Store of Supervisor LXical.Server.Supervisor failed to start
** (exit) already started: #PID<23232.135.0>
Pid: #PID<0.135.0>
Start Call: LXical.Document.Store.start_link([])
Restart: :permanent
Shutdown: 5000
Type: :worker

@scohen
Copy link
Collaborator Author

scohen commented Oct 17, 2023

I just found the indexer breaks the multiple nodes

This should be an easy fix, we just need to run the indexer only on the node with the store's leader.

@scohen
Copy link
Collaborator Author

scohen commented Oct 17, 2023

@scottming is that what's going on? I need more information. It looks like there should only be one indexer running at a given time across nodes on a given machine.

It looks like you can't get two document stores running at the same time

@scottming
Copy link
Collaborator

scottming commented Oct 18, 2023

@scottming is that what's going on? I need more information. It looks like there should only be one indexer running at a given time across nodes on a given machine.

Yes, that should be the issue. However, I don't quite understand the logic related to the leader of Ets module .

This is the log in project.log

2023-10-18T09:27:00.233815+08:00 info: backend reports stale
2023-10-18T09:27:00.304509+08:00 info: Loaded []
2023-10-18T09:27:00.376849+08:00 info: Compile completed with status ok Produced 0 diagnostics []
2023-10-18T09:27:00.380429+08:00 info: Plugins found 0 results in 0.008 ms
2023-10-18T09:27:08.407430+08:00 info: global: Name conflict terminating {'Elixir.LXical.Document.Store',<22661.135.0>}

@scohen
Copy link
Collaborator Author

scohen commented Oct 18, 2023

This is very interesting, I would think it impossible to launch two editors so quickly that they conflict for the global name. How are you doing this?

@scottming
Copy link
Collaborator

I just have this problem after revert indexer commit packaged, opening a very small project with Nvim, wait the compiling finished, and then opening the same project with VScode.

@scohen
Copy link
Collaborator Author

scohen commented Oct 18, 2023

I just have this problem after revert indexer commit packaged, opening a very small project with Nvim, wait the compiling finished, and then opening the same project with VScode

I don't think this issue is related to find references per se; It's related to having different editors connect to an index store node. This effectively flattens out the namespace, and things that were globally registered in the context of an editor and its lsp, will now be seen by other editors.

This does mean that using global for the document store's registration isn't going to work any more. 🤔

@scohen
Copy link
Collaborator Author

scohen commented Oct 18, 2023

@scottming I added #425 to track this issue.

@scohen scohen force-pushed the find-references branch 3 times, most recently from 397d77b to 5713f55 Compare October 24, 2023 21:29
Added initial implementation of find references. It presently works
for any module-like thing that can be recognized by Entity.resolve,
like Modules, aliased modules, __MODULE__ and structs.
In order to enable indexing, you must do
`ENABLE_INDEXING=true mix package`
@scottming
Copy link
Collaborator

scottming commented Oct 30, 2023

I think we should ignore the results from _build/... and deps/... folders.

image

@scohen
Copy link
Collaborator Author

scohen commented Oct 30, 2023

I think we should ignore the results from _build/... and deps/... folders.

I'm on the fence here with deps. We shouldn't see anything from the _build directory since those are compiled artifacts and we're indexing source code. Were you seeing stuff from _build?

@scottming
Copy link
Collaborator

In another production project, I often see many occurrences of both deps and build, but only a few occurrences in lexical.

CleanShot 2023-10-31 at 20 47 26@2x

@scohen
Copy link
Collaborator Author

scohen commented Oct 31, 2023

ah, i see. Yes, we should exempt _build from indexing. (This is an indexing problem, and not a find references problem)

@scohen
Copy link
Collaborator Author

scohen commented Oct 31, 2023

@scottming I'll make an issue for this.

@scohen scohen merged commit a6dd101 into main Oct 31, 2023
7 checks passed
@scohen scohen deleted the find-references branch October 31, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants