lsp: Actually implement workDoneProgress #234
Conversation
|
Seemed to work and doesn't have performance issue but I would probably try to change to reuse the buffer later on like I mentioned. |
Sorry but I'm not fully sure which buffer are you talking about, could you point it out to me perhaps? |
helix-term/src/application.rs
Outdated
| @@ -73,6 +76,8 @@ impl Application { | |||
| editor, | |||
|
|
|||
| callbacks: FuturesUnordered::new(), | |||
|
|
|||
| lsp_progress: HashMap::new(), | |||
There was a problem hiding this comment.
Having this on Application is problematic because two different LSPs could write to the same progress token. I know this is kind of hard, but we need to set these on the LSP client :/
helix-term/src/application.rs
Outdated
| (Some(title), None, None) => title.to_string(), | ||
| (None, Some(message), None) => message.to_string(), | ||
| (None, None, Some(percentage)) => format!("{}%", percentage), | ||
| (None, None, None) => "".into(), | ||
| }; | ||
|
|
||
| if let lsp::WorkDoneProgress::End(_) = work { | ||
| self.lsp_progress.remove(&token); | ||
| } | ||
|
|
||
| msg | ||
| }; | ||
| let token = match token { | ||
| lsp::NumberOrString::Number(n) => n.to_string(), | ||
| lsp::NumberOrString::String(s) => s, |
There was a problem hiding this comment.
Sorry but I'm not fully sure which buffer are you talking about, could you point it out to me perhaps?
This part. String is being allocated and deallocated for every event.
There was a problem hiding this comment.
I think I optimized it somewhat, let me know if you think we can improve it further. Currently the status variable is moved to set_editor so we still have to allocate it on each event.
|
I submitted my work in progress, very much incomplete so nothing to see here for now. Will work on it some more tomorrow. |
|
Can you please mark the pull request as draft? |
vv9k
force-pushed
the
workdone
branch
2 times, most recently
from
ca3cf83 to
fe388e5
Compare
vv9k
force-pushed
the
workdone
branch
2 times, most recently
from
ea2a78a to
1f2f970
Compare
|
Okay, one last thing, can we add a config option to disable the progress reports in the prompt? |
Done, had to rebase to master so that I can reuse the |
This PR reenables workDoneProgress and adds handling for workDoneProgress/create requests from lsp server.