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

Implement support for client initiated $/progress #380

Closed
1 task done
ebkalderon opened this issue Mar 7, 2023 · 1 comment · Fixed by #385
Closed
1 task done

Implement support for client initiated $/progress #380

ebkalderon opened this issue Mar 7, 2023 · 1 comment · Fixed by #385
Assignees
Labels
enhancement New feature or request

Comments

@ebkalderon
Copy link
Owner

ebkalderon commented Mar 7, 2023

Background

This task was split from #176.

According to the relevant section in the specification, client initiated progress is defined as follows:

  1. The client includes a .work_done_progress_params.work_done_token field in the "params" given to the server.
  2. The server sends back $/progress begin/report/end notifications to the client using this work_done_token value.

Implementation

To support this type of $/progress, we should do the following:

  • Introduce a Client::progress() method resembling the draft proposal below. This lets the server send a stream of $/progress notifications to the client using a client-issued WorkDoneToken.

Draft API

impl Client {
    pub fn progress<T>(&self, token: ProgressToken, title: T) -> Progress
    where
        T: Into<String>,
}

enum Bounded {}
enum Unbounded {}
enum Cancellable {}
enum NotCancellable {}

pub struct Progress<B = Unbounded, C = NotCancellable> { ... }

impl<C> Progress<Unbounded, C> {
    pub fn bounded(self, start_percentage: u32) -> Progress<Bounded, C>;
}

impl<B> Progress<B, NotCancellable> {
    pub fn cancellable(self, show_cancel_btn: bool) -> Progress<B, Cancellable>;
}

impl<B, C> Progress<B, C> {
    pub fn with_message<M>(self, message: M) -> Self
    where
        M: Into<String>;

    pub async fn begin(self) -> OngoingProgress<B, C>;
}

pub struct OngoingProgress<B, C> { ... }

impl OngoingProgress<Unbounded, NotCancellable> {
    pub async fn report<M>(&self, message: M)
    where
        M: Into<Option<String>>;
}

impl OngoingProgress<Unbounded, Cancellable> {
    pub async fn report<M>(&self, message: M, show_cancel_btn: bool)
    where
        M: Into<Option<String>>;
}

impl OngoingProgress<Bounded, NotCancellable> {
    pub async fn report<M>(&self, percentage: u32, message: M)
    where
        M: Into<Option<String>>;
}

impl OngoingProgress<Bounded, Cancellable> {
    pub async fn report<M>(&self, percentage: u32, message: M, show_cancel_btn: bool)
    where
        M: Into<Option<String>>;
}

impl<C> OngoingProgress<Bounded, C> {
    // This is explicitly allowed by the official specification. Clients will treat all
    // subsequent calls to `report()` as unbounded progress.
    pub fn discard_bound(self) -> OngoingProgress<Unbounded, C>;
}

impl<B, C> OngoingProgress<B, C> {
    pub fn token(&self) -> &ProgressToken;

    pub async fn finish<M>(self, message: M)
    where
        M: Into<Option<String>>;
}

Usage

impl LanguageServer for MyServer {
    async fn completion(&self, params: CompletionParams) -> Result<Option<CompletionResponse>> {
        let token = params.work_done_progress_params.work_done_token.unwrap();

        let progress = self
            .client
            .progress(token, "retrieving completions")
            .bounded(0)
            .with_message("starting work...")
            .begin()
            .await;

        for x in something {
            // Do some work...
            let percentage = ...
            progress.report(percentage, "reticulating splines".to_owned()).await;
        }

        progress.finish("done!".to_owned()).await;

        // ...
    }

    // ...
}

Other Ideas

  • Consider dropping the message: M where M: Into<Option<String>> argument in favor of distinct .foo() and .foo_with_message() methods.
  • Consider separate methods for bounded/unbounded × cancellable/not-cancellable instead of a typed builder.
    • Seems like this would get messy quickly, but will still list it here as an option.
  • Consider the implications of the Bounded, Unbounded, Cancellable, NotCancellable enums being public vs. private.
  • For bounded progress reports: consider requiring users to pass an increment value instead of a percentage. This ensures the percentage values sent to the client are monotonic.
  • For bounded progress reports: consider how percentage values above 100% should be handled. Panic? Log a warning? Something else?
  • For cancellable progress reports: consider alternatives to the show_cancel_btn: bool argument:
    • Use an enum with explicit states instead of a plain bool to improve readability at the callsite?
    • Store the bool as a struct field and expose getters and setters instead of providing the value with every call to report()?
  • Nice to have: include an API for wrapping an arbitrary std::iter::Iterator and/or futures::Stream and emit a stream of progress. See indicatif::ProgressBarIter for prior art.
@ebkalderon
Copy link
Owner Author

I've opened #385 implementing a streamlined version of the API above. Public feedback and testing is most welcome! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant