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

Remove the global AUXILIARY_EVENT_TX and use tracing #80

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

DavisVaughan
Copy link
Collaborator

@DavisVaughan DavisVaughan commented Dec 3, 2024

Closes #79

This is a first pass to completely remove AUXILIARY_EVENT_TX in favor of passing around state to access auxiliary_event_tx, owned by GlobalState.

Currently logging is tied up in auxiliary events, which results in a net loss for ergonomics when you want to log something, as you need to pass auxiliary_event_tx around to call its log_error() methods.

#93 is a follow up to this PR that splits the log specific events out of this, instead hooking up a tracing subscriber for logging so you can use tracing::error!() as a free function anywhere in the LSP

crates/lsp/src/config.rs Show resolved Hide resolved
Comment on lines 157 to +160
pub(crate) fn did_close(
params: DidCloseTextDocumentParams,
state: &mut WorldState,
auxiliary_event_tx: &AuxiliaryEventSender,
Copy link
Collaborator Author

@DavisVaughan DavisVaughan Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any LSP request/notification that needs to spawn a blocking task should add a auxiliary_event_tx argument that is passed through from the top level handle_event()

It can then call things like auxiliary_event_tx.spawn_blocking_task()

crates/lsp/src/main_loop.rs Outdated Show resolved Hide resolved
@DavisVaughan DavisVaughan requested a review from lionel- December 3, 2024 17:42
lionel-

This comment was marked as outdated.

@DavisVaughan

This comment was marked as outdated.

Copy link
Collaborator

@lionel- lionel- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what kind of objects we'll send around to spawn tasks but we can figure this out when we get there.

@DavisVaughan DavisVaughan changed the title Remove the global AUXILIARY_EVENT_TX Remove the global AUXILIARY_EVENT_TX and use tracing Dec 13, 2024
That way all you have to do to spawn a blocking task from a request handler is pass `auxiliary_event_tx` to it
@DavisVaughan DavisVaughan merged commit 9d308f1 into main Dec 13, 2024
4 checks passed
@DavisVaughan DavisVaughan deleted the fix/aux-global branch December 13, 2024 19:01
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.

test_format_minimal_diff is not reliable
2 participants