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

fix: jump to def/decl/type-def don't stop working after the first jump #901

Closed
wants to merge 1 commit into from

Conversation

ulugbekna
Copy link
Collaborator

Fixes #898

It seems like we shouldn't reuse a pipeline if a pipeline has been constructed for another file.

@rgrinberg
Copy link
Member

It seems like we shouldn't reuse a pipeline if a pipeline has been constructed for another file.

But how are we reusing a pipeline used for another file? We create a pipeline for every document.

@rgrinberg
Copy link
Member

In any case, it does seem sketchy what we're doing. We can't stop reusing pipelines altogether, but how about we only save the last used pipeline and reuse it for subsequent requests to the same document?

@ulugbekna
Copy link
Collaborator Author

But how are we reusing a pipeline used for another file? We create a pipeline for every document.

AFAIU, a pipeline creation invalidates all previously created pipelines, due the creation manipulating global variables such as Location.input_name in setup_reader_config fn in mocaml.ml.

how about we only save the last used pipeline and reuse it for subsequent requests to the same document?

Yes, that's what I intend to do.

@rgrinberg
Copy link
Member

Okay, carry on then. I misunderstood the PR's description then.

@ulugbekna
Copy link
Collaborator Author

On second thought, we have Mpipeline.with_pipeline that implies that there should be more than one pipeline existing. That function invokes Mocaml.with_state that allows to set a set of global refs to a snapshot captured for this particular pipeline. Env.Current_unit_name.t and Location.input_name mutable vars do not seem to be snapshotted.

Also, there's query field in Mconfig.t, which allows specifying file and its directory, so could a pipeline be reused for different files?

I'll try looking at how emacs/vim use merlin's API tomorrow.

@ulugbekna
Copy link
Collaborator Author

I'll make another PR when I figure things out.

@ulugbekna ulugbekna closed this Nov 4, 2022
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.

Locate failing with "not in environment"
2 participants