Skip to content

Add a generic progress start and update delay #549

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.

Sign up for GitHub

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

Jump to bottom
Closed
michaelpj opened this issue Feb 3, 2024 · 5 comments
Closed

Add a generic progress start and update delay #549

michaelpj opened this issue Feb 3, 2024 · 5 comments
Assignees
@michaelpj
Labels
good first issue

Comments

@michaelpj
Copy link
Collaborator

At the moment we send progress start messages as soon as we can, and updates whenever we get them.

This means that:

  • We send progress messages even for extremely short-lived operations
  • We send very frequent progress updates in some cases

We could potentially handle this generically: only send progress messages after a fixed interval has passed (should be configurable). In particular, if the operation terminates before we hit the first interval, we could send nothing.

This would let people declare most handlers to support progress and automatically get nice behaviour where progress is shown only if it takes a noticeable amount of time.
@michaelpj
Copy link
Collaborator Author

This should let us get rid of a decent chunk of https://github.com/haskell/haskell-language-server/blob/b91c9076367123e983087ed305d183288a23f494/ghcide/src/Development/IDE/Core/ProgressReporting.hs#L108

michaelpj added a commit to haskell/haskell-language-server that referenced this issue Feb 5, 2024
@michaelpj
Bump lsp versions
98accef 
Broadly:
- A few places where we need to pipe `ProgressToken`s around.
   - I also just removed the progress reporting from resolve commands,
     since it's going to often be costly to do progress reporting on
     something that short. Possibly we could revisit after haskell/lsp#549
- Some changes to the registration options we infer
- A few places where we need to adapt to ignoring registrations or not
michaelpj added a commit to haskell/haskell-language-server that referenced this issue Feb 5, 2024
@michaelpj
Bump lsp versions
7020465 
Broadly:
- A few places where we need to pipe `ProgressToken`s around.
   - I also just removed the progress reporting from resolve commands,
     since it's going to often be costly to do progress reporting on
     something that short. Possibly we could revisit after haskell/lsp#549
- Some changes to the registration options we infer
- A few places where we need to adapt to ignoring registrations or not
- Adapting to use the ghcide verison of `getCompletionPrefix` everywhere
michaelpj added a commit to haskell/haskell-language-server that referenced this issue Feb 5, 2024
@michaelpj
Bump lsp versions
30f7e40 
Broadly:
- A few places where we need to pipe `ProgressToken`s around.
   - I also just removed the progress reporting from resolve commands,
     since it's going to often be costly to do progress reporting on
     something that short. Possibly we could revisit after haskell/lsp#549
- Some changes to the registration options we infer
- A few places where we need to adapt to ignoring registrations or not
- Adapting to use the ghcide verison of `getCompletionPrefix` everywhere
- Adapting to use the new mixed rope format
@michaelpj michaelpj mentioned this issue Feb 5, 2024
Merged
michaelpj added a commit to haskell/haskell-language-server that referenced this issue Feb 5, 2024
@michaelpj
Bump lsp versions
0cb5bf9 
Broadly:
- A few places where we need to pipe `ProgressToken`s around.
   - I also just removed the progress reporting from resolve commands,
     since it's going to often be costly to do progress reporting on
     something that short. Possibly we could revisit after haskell/lsp#549
- Some changes to the registration options we infer
- A few places where we need to adapt to ignoring registrations or not
- Adapting to use the ghcide verison of `getCompletionPrefix` everywhere
- Adapting to use the new mixed rope format
michaelpj added a commit to haskell/haskell-language-server that referenced this issue Feb 8, 2024
@michaelpj
Bump lsp versions
d01b872 
Broadly:
- A few places where we need to pipe `ProgressToken`s around.
   - I also just removed the progress reporting from resolve commands,
     since it's going to often be costly to do progress reporting on
     something that short. Possibly we could revisit after haskell/lsp#549
- Some changes to the registration options we infer
- A few places where we need to adapt to ignoring registrations or not
- Adapting to use the ghcide verison of `getCompletionPrefix` everywhere
- Adapting to use the new mixed rope format
michaelpj added a commit to haskell/haskell-language-server that referenced this issue Feb 8, 2024
@michaelpj
Bump lsp versions
b6f0047 
Broadly:
- A few places where we need to pipe `ProgressToken`s around.
   - I also just removed the progress reporting from resolve commands,
     since it's going to often be costly to do progress reporting on
     something that short. Possibly we could revisit after haskell/lsp#549
- Some changes to the registration options we infer
- A few places where we need to adapt to ignoring registrations or not
- Adapting to use the ghcide verison of `getCompletionPrefix` everywhere
- Adapting to use the new mixed rope format
michaelpj added a commit to haskell/haskell-language-server that referenced this issue Feb 8, 2024
@michaelpj
Bump lsp versions (#4052)
0b0eee3 
* Bump lsp versions

Broadly:
- A few places where we need to pipe `ProgressToken`s around.
   - I also just removed the progress reporting from resolve commands,
     since it's going to often be costly to do progress reporting on
     something that short. Possibly we could revisit after haskell/lsp#549
- Some changes to the registration options we infer
- A few places where we need to adapt to ignoring registrations or not
- Adapting to use the ghcide verison of `getCompletionPrefix` everywhere
- Adapting to use the new mixed rope format

* stack

* More fixes
@michaelpj michaelpj mentioned this issue May 4, 2024
Closed
@michaelpj
Copy link
Collaborator Author

Okay, I'm going to work on this for a little.

@soulomoon
Copy link
Collaborator

Sounds good

@michaelpj
Copy link
Collaborator Author

This is slightly annoying with the way it's currently done. In particular, at the moment we only do things when we get an update from the updater function, but once there are delays in the picture we probably need to wake up and send the last update that we saw every now and again, otherwise we might get quite bad behaviour. Which I think necessitates another thread handling that. So I need to refactor a bit.

@michaelpj michaelpj self-assigned this May 9, 2024
@michaelpj
Copy link
Collaborator Author

I did this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelpj michaelpj

Labels
good first issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@michaelpj @soulomoon