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

Centralize general-use AST modules in common #342

Merged
merged 6 commits into from
Aug 25, 2023
Merged

Centralize general-use AST modules in common #342

merged 6 commits into from
Aug 25, 2023

Conversation

zachallaun
Copy link
Collaborator

This PR is an attempt to extract generally useful existing AST-handling code into common so that it can be more easily found and used for other features.

I believe this will be most easily reviewed commit-by-commit. Where the changes are not self-evident, I've added explanation to the commit.

Completions are currently built on top of two behaviours, `Environment` and
`Builder`. The first describes a behaviour for inspecting the contextual
environment of a given position so that you can filter completions to only
those that would make sense in that context. The second describes a behaviour
for turning those completions into something that the LSP understands.

The `Environment` behaviour and implementation is generally useful outside of
the context of completions. The completions feature implemented both behaviours
in a single `Lexical.Server.CodeIntelligence.Completion.Env`. This commit
decouples the env and the builder implementations, moving the
environment-related code (and struct) to `Lexical.Ast.Env` and putting the
builder implementation in `Lexical.Server.CodeIntelligence.Completion.Builder`.
`Lexical.RemoteControl.CodeMod.Ast` contained generally-useful functions for
parsing and creating zippers from documents. While it was located in
`RemoteControl`, it wasn't doing anything that required running on the remote
node.

Similarly, `Lexical.RemoteControl.CodeMod.Ast.Aliases` contains code for
resolving the available aliases at a given position and also has no
requirements to run on the remote node. It's been moved to
`Lexical.Ast.Aliases`.
This module provided only a relatively thin wrapper over `Aliases.at/2`. It has
been moved into `Lexical.Ast`.
This was a comment explaining why position line and character were being
incremented when they used to be zero-based. They are now one-based, so
this comment is no longer needed.
@scohen scohen merged commit 2ab02e2 into lexical-lsp:main Aug 25, 2023
@zachallaun zachallaun deleted the za-lexical-ast branch August 25, 2023 16:00
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.

3 participants