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.

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

Another attempt at using the lsp API for some progress reporting #4218

Merged
merged 10 commits into from
May 19, 2024
Merged

Another attempt at using the lsp API for some progress reporting #4218

merged 10 commits into from
May 19, 2024

Conversation

michaelpj
Copy link
Collaborator

No description provided.

@michaelpj
Copy link
Collaborator Author

Okay, I think the tests should pass now. I had to get rid of some of the functional tests because they test the real HLS executable, so that now includes a delay on starting progress. So we can't observe it unless we can make things take long enough, and we can't make the delay shorter because we're using the real HLS (unlike the ghcide tests which use a special executable with different options). I was able to arrange for one of the tests to have a long enough delay, so that might have to do for now.

As far as I can tell, this seems to work reliably. I would also like to rewrite the other special progress reporter, not sure if I'll do that in this PR or not.

@michaelpj
Copy link
Collaborator Author

Okay, many tests rely on waiting for progress notifications and are now somewhat broken. I shall continue on it.

@michaelpj
Copy link
Collaborator Author

This will fix #381, in that we won't show progress for fast operations, which will include most formatting operations.

It should also fix #3577, or at least mitigate it, in that updates will be throttled and won't be sent for short-lived "processing" operations.

@michaelpj michaelpj mentioned this pull request May 9, 2024
Merged
@michaelpj michaelpj marked this pull request as ready for review May 10, 2024 14:00
@michaelpj
Copy link
Collaborator Author

Ready for a look, perhaps @soulomoon ?

soulomoon
soulomoon approved these changes May 10, 2024
View reviewed changes
Copy link
Collaborator

@soulomoon soulomoon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good !
Just want to ask one thing, so we lose some of the notification tests since we now have 1 s delay for notification ? I wonder if we can set the shorter delay for testing and gain those test back?

@soulomoon soulomoon mentioned this pull request May 10, 2024
Closed
@michaelpj
Copy link
Collaborator Author

Yes, so we have some special arguments for the ghcide executable that get used during testing that set the delay to 0. But for the HLS functional tests we don't call the main function of HLS we actually run the built executable, so I'm not sure how to make that nicely configurable...

@soulomoon
Copy link
Collaborator

soulomoon commented May 11, 2024 •
edited
Loading

Yes, so we have some special arguments for the ghcide executable that get used during testing that set the delay to 0.

So it is using

hlsLspCommand :: String
hlsLspCommand = hlsExeCommand ++ " --lsp -d -j4"

Maybe we can generalize the special arguments for the ghcide to the hls-exe ?

@michaelpj
Copy link
Collaborator Author

Yeah, or maybe I can use an environment variable or something.

konn
konn approved these changes May 17, 2024
View reviewed changes
Copy link
Collaborator

@konn konn left a comment

Choose a reason for hiding this comment

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

LGTM as for Splice Plugin! 👍

@michaelpj michaelpj force-pushed the mpj/progress-reporting-experiments branch from 34766bb to 0e0a492 Compare May 19, 2024 13:43
@michaelpj michaelpj requested a review from wz1000 as a code owner May 19, 2024 13:43
@michaelpj
Copy link
Collaborator Author

We already had a --test argument to HLS that turns on the testing stuff, which I had already made change the delay. So I just needed to pass that in the test invocation and it works again.

@michaelpj michaelpj enabled auto-merge (squash) May 19, 2024 13:44
@michaelpj michaelpj disabled auto-merge May 19, 2024 15:06
@michaelpj michaelpj merged commit 0651c5c into master May 19, 2024
37 of 38 checks passed
@michaelpj michaelpj mentioned this pull request May 19, 2024
Merged
fendor
fendor reviewed May 19, 2024
View reviewed changes
ghcide/src/Development/IDE/Main.hs
{ LSP.optCompletionTriggerCharacters = Just "."
-- Generally people start to notice that something is taking a while at about 1s, so
-- that's when we start reporting progress
, LSP.optProgressStartDelay = 1_00_000
Copy link
Collaborator

@fendor fendor May 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What a weird number. Shouldn't this be 1_000_000?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤦

@BrewTestBot BrewTestBot mentioned this pull request Jun 19, 2024
Closed
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 31, 2025
@depressed-pho
devel/haskell-language-server: update to haskell-language-server-2.9.0.0
30ab57a 
## 2.9.0.0

- Bindists for GHC 9.10.1 by @wz1000, @jhrcek, @michaelpj
- More hls-graph reliability improvements by @soulomoon
- Refactoring of test suite runners by @soulomoon
- Fixes in multiple home units support by @wz1000

### Pull Requests

- Fix quadratic memory usage in GetLocatedImports
  ([#4318](haskell/haskell-language-server#4318)) by @mpickering
- Bump stack configs + CI to 9.6.5 and 9.8.2
  ([#4316](haskell/haskell-language-server#4316)) by @jhrcek
- Add support for Fourmolu 0.16
  ([#4314](haskell/haskell-language-server#4314)) by @ brandonchinn178
- Code action to remove redundant record field import (fixes #4220)
  ([#4308](haskell/haskell-language-server#4308)) by @battermann
- Use restricted monad for plugins (#4057)
  ([#4304](haskell/haskell-language-server#4304)) by @awjchen
- 4301 we need to implement utility to wait for all runnning keys in hls graph done
  ([#4302](haskell/haskell-language-server#4302)) by @soulomoon
- Call useWithStale instead of useWithStaleFast when calling ParseCabalFields
  ([#4294](haskell/haskell-language-server#4294)) by @VeryMilkyJoe
- test: add test documenting #806
  ([#4292](haskell/haskell-language-server#4292)) by @develop7
- ghcide: drop ghc-check and ghc-paths dependency
  ([#4291](haskell/haskell-language-server#4291)) by @wz1000
- Limit number of valid hole fits to 10
  ([#4288](haskell/haskell-language-server#4288)) by @akshaymankar
- Add common stanza to completion data
  ([#4286](haskell/haskell-language-server#4286)) by @VeryMilkyJoe
- FindImports: ThisPkg means some home unit, not "this" unit
  ([#4284](haskell/haskell-language-server#4284)) by @wz1000
- Remove redudant absolutization in session loader
  ([#4280](haskell/haskell-language-server#4280)) by @soulomoon
- Bump to new lsp versions
  ([#4279](haskell/haskell-language-server#4279)) by @michaelpj
- Put more test code into pre-commit
  ([#4275](haskell/haskell-language-server#4275)) by @soulomoon
- Delete library ghcide test utils
  ([#4274](haskell/haskell-language-server#4274)) by @soulomoon
- Delete testUtil from ghcide-tests
  ([#4272](haskell/haskell-language-server#4272)) by @soulomoon
- CI change, only run bench on performance label
  ([#4271](haskell/haskell-language-server#4271)) by @soulomoon
- Migrate WatchedFileTests
  ([#4269](haskell/haskell-language-server#4269)) by @soulomoon
- Migrate UnitTests
  ([#4268](haskell/haskell-language-server#4268)) by @soulomoon
- Migrate SafeTests
  ([#4267](haskell/haskell-language-server#4267)) by @soulomoon
- Migrate SymlinkTests
  ([#4266](haskell/haskell-language-server#4266)) by @soulomoon
- Remove unused and outdated CHANGELOG files
  ([#4264](haskell/haskell-language-server#4264)) by @fendor
- Enable cabal flaky test
  ([#4263](haskell/haskell-language-server#4263)) by @soulomoon
- Migrate RootUriTests
  ([#4261](haskell/haskell-language-server#4261)) by @soulomoon
- Migrate PreprocessorTests
  ([#4260](haskell/haskell-language-server#4260)) by @soulomoon
- Migrate PluginSimpleTests
  ([#4259](haskell/haskell-language-server#4259)) by @soulomoon
- Migrate ClientSettingsTests
  ([#4258](haskell/haskell-language-server#4258)) by @soulomoon
- Unify critical session running in hls
  ([#4256](haskell/haskell-language-server#4256)) by @soulomoon
- Bump cachix/cachix-action from 14 to 15
  ([#4255](haskell/haskell-language-server#4255)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.7.2 to 2.7.3
  ([#4254](haskell/haskell-language-server#4254)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.7.2 to 2.7.3 in /.github/actions/setup-build
  ([#4253](haskell/haskell-language-server#4253)) by @dependabot[bot]
- Shorter file names completion
  ([#4252](haskell/haskell-language-server#4252)) by @VenInf
- Fix progress start delay
  ([#4249](haskell/haskell-language-server#4249)) by @michaelpj
- Bump cachix/install-nix-action from 26 to 27
  ([#4245](haskell/haskell-language-server#4245)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.7.1 to 2.7.2
  ([#4244](haskell/haskell-language-server#4244)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.7.1 to 2.7.2 in /.github/actions/setup-build
  ([#4243](haskell/haskell-language-server#4243)) by @dependabot[bot]
- Enable test for #717
  ([#4241](haskell/haskell-language-server#4241)) by @soulomoon
- Remove Pepe from CODEOWNERS
  ([#4239](haskell/haskell-language-server#4239)) by @michaelpj
- Fix resultBuilt(dirty mechanism) in hls-graph
  ([#4238](haskell/haskell-language-server#4238)) by @soulomoon
- Support for 9.10
  ([#4233](haskell/haskell-language-server#4233)) by @wz1000
- Refactor hls-test-util and reduce getCurrentDirectory after initilization
  ([#4231](haskell/haskell-language-server#4231)) by @soulomoon
- [Migrate BootTests] part of #4173 Migrate ghcide tests to hls test utils
  ([#4227](haskell/haskell-language-server#4227)) by @soulomoon
- Actually enable pedantic flag in ci flags job
  ([#4224](haskell/haskell-language-server#4224)) by @jhrcek
- Cleanup cabal files, ghc compat code, fix ghc warnings
  ([#4222](haskell/haskell-language-server#4222)) by @jhrcek
- Another attempt at using the lsp API for some progress reporting
  ([#4218](haskell/haskell-language-server#4218)) by @michaelpj
- [Migrate diagnosticTests] part of #4173 Migrate ghcide tests to hls test utils
  ([#4207](haskell/haskell-language-server#4207)) by @soulomoon
- Prepare release 2.8.0.0
  ([#4191](haskell/haskell-language-server#4191)) by @wz1000
- Stabilize the build system by correctly house keeping the dirtykeys and rule values [flaky test #4185 #4093]
  ([#4190](haskell/haskell-language-server#4190)) by @soulomoon
- hls-cabal-plugin: refactor context search to use `readFields`
  ([#4186](haskell/haskell-language-server#4186)) by @fendor
- 3944 extend the properties api to better support nested configuration
  ([#3952](haskell/haskell-language-server#3952)) by @soulomoon

## 2.8.0.0

- Bindists for GHC 9.6.5
- New hls-notes plugin (#4126, @jvanbruegge)
- Floskell, hlint and stylish-haskell plugins enabled for GHC 9.8
- Improvements for hls-graph increasing robustness (#4087, @soulomoon)
- Improvements to multi-component support (#4096, #4109, #4179, @wz1000, @fendor)

### Pull Requests

- Bump haskell-actions/setup from 2.7.0 to 2.7.1
  ([#4189](haskell/haskell-language-server#4189)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.7.0 to 2.7.1 in /.github/actions/setup-build
  ([#4188](haskell/haskell-language-server#4188)) by @dependabot[bot]
- Fix ghcdie-tests CI
  ([#4184](haskell/haskell-language-server#4184)) by @soulomoon
- Fix ghc and hlint warnings, fix formatting
  ([#4181](haskell/haskell-language-server#4181)) by @jhrcek
- Allow users to specify whether to use `cabal`'s multi-repl feature
  ([#4179](haskell/haskell-language-server#4179)) by @fendor
- Improve parsing of import suggestions extending multiple multiline imports (fixes #4175)
  ([#4177](haskell/haskell-language-server#4177)) by @jhrcek
- move ghcide-tests to haskell-language-server.cabal and make it depend on hls-test-utils
  ([#4176](haskell/haskell-language-server#4176)) by @soulomoon
- enable ThreadId for when testing
  ([#4174](haskell/haskell-language-server#4174)) by @soulomoon
- Drop Legacy Logger from Codebase
  ([#4171](haskell/haskell-language-server#4171)) by @fendor
- get rid of the `unsafeInterleaveIO` at start up
  ([#4167](haskell/haskell-language-server#4167)) by @soulomoon
- Remove EKG
  ([#4163](haskell/haskell-language-server#4163)) by @michaelpj
- Mark plugins as not buildable if the flag is disabled
  ([#4160](haskell/haskell-language-server#4160)) by @michaelpj
- Fix references to old CPP names in tests, update tests
  ([#4159](haskell/haskell-language-server#4159)) by @jhrcek
- Bump haskell-actions/setup from 2.6.3 to 2.7.0
  ([#4158](haskell/haskell-language-server#4158)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.6.3 to 2.7.0 in /.github/actions/setup-build
  ([#4157](haskell/haskell-language-server#4157)) by @dependabot[bot]
- Remove dead code in ghcide and hls-graph for priority
  ([#4151](haskell/haskell-language-server#4151)) by @soulomoon
- Bump haskell-actions/setup from 2.6.2 to 2.6.3 in /.github/actions/setup-build
  ([#4150](haskell/haskell-language-server#4150)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.6.2 to 2.6.3
  ([#4149](haskell/haskell-language-server#4149)) by @dependabot[bot]
- Run ExceptionTests in temporary directory
  ([#4146](haskell/haskell-language-server#4146)) by @fendor
- hls-eval-plugin: Replicate #4139
  ([#4140](haskell/haskell-language-server#4140)) by @mattapet
- Update comment in refactor tests
  ([#4138](haskell/haskell-language-server#4138)) by @jhrcek
- Update contact info in docs
  ([#4137](haskell/haskell-language-server#4137)) by @jhrcek
- hls-notes-plugin: Do not error if no note is under the cursor
  ([#4136](haskell/haskell-language-server#4136)) by @jvanbruegge
- improve logging in semantic tokens rule
  ([#4135](haskell/haskell-language-server#4135)) by @soulomoon
- Bump softprops/action-gh-release from 1 to 2
  ([#4133](haskell/haskell-language-server#4133)) by @dependabot[bot]
- Bump cachix/install-nix-action from 25 to 26
  ([#4132](haskell/haskell-language-server#4132)) by @dependabot[bot]
- Use Set.member instead of Foldable.elem
  ([#4128](haskell/haskell-language-server#4128)) by @jhrcek
- hls-notes-plugin: Initial implementation
  ([#4126](haskell/haskell-language-server#4126)) by @jvanbruegge
- Enable floskell and hlint plugins for ghc 9.8
  ([#4125](haskell/haskell-language-server#4125)) by @jhrcek
- Integrate stylish-haskell into hls executable with ghc 9.8
  ([#4124](haskell/haskell-language-server#4124)) by @jhrcek
- Reduce usage of partial functions
  ([#4123](haskell/haskell-language-server#4123)) by @jhrcek
- Benchmark: Enable 9.6, 9.8
  ([#4118](haskell/haskell-language-server#4118)) by @soulomoon
- Bump haskell-actions/setup from 2.6.1 to 2.6.2 in /.github/actions/setup-build
  ([#4116](haskell/haskell-language-server#4116)) by @dependabot[bot]
- Bump haskell-actions/setup from 2.6.1 to 2.6.2
  ([#4115](haskell/haskell-language-server#4115)) by @dependabot[bot]
- eval: more robust way to extract comments from ParsedModule
  ([#4113](haskell/haskell-language-server#4113)) by @jhrcek
- Improve isolation of build artefacts of test runs
  ([#4112](haskell/haskell-language-server#4112)) by @fendor
- Improve handling of nonsense rename attempts
  ([#4111](haskell/haskell-language-server#4111)) by @jhrcek
- Exit with non-zero exitcode if wrapper fails to launch
  ([#4110](haskell/haskell-language-server#4110)) by @fendor
- Replace checkHomeUnitsClosed with a faster implementation
  ([#4109](haskell/haskell-language-server#4109)) by @wz1000
- Don't distribute gifs or plugin readmes
  ([#4107](haskell/haskell-language-server#4107)) by @fendor
- Remove locale workaround for Module name that conatins non-ascii characters
  ([#4106](haskell/haskell-language-server#4106)) by @fendor
- Track extra-source-files of plugins more accurately
  ([#4105](haskell/haskell-language-server#4105)) by @fendor
- remove non-ascii name
  ([#4103](haskell/haskell-language-server#4103)) by @soulomoon
- Add cabal-gild as a cabal file formatter plugin
  ([#4101](haskell/haskell-language-server#4101)) by @fendor
- Remove more workarounds for GHCs < 9.2 (#4092)
  ([#4098](haskell/haskell-language-server#4098)) by @jhrcek
- session-loader: Don't loop forever when we don't find a file in any multi component
  ([#4096](haskell/haskell-language-server#4096)) by @wz1000
- Prepare release 2.7.0.0
  ([#4095](haskell/haskell-language-server#4095)) by @fendor
- Remove more workarounds for GHCs < 9.0
  ([#4092](haskell/haskell-language-server#4092)) by @jhrcek
- Fix hls-graph: phantom dependencies invoke in branching deps (resolve #3423)
  ([#4087](haskell/haskell-language-server#4087)) by @soulomoon
- Rename only if the current module compiles (#3799)
  ([#3848](haskell/haskell-language-server#3848)) by @sgillespie
- Reintroduce ghc-lib flag for hlint plugin
  ([#3757](haskell/haskell-language-server#3757)) by @RaoulHC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fendor fendor fendor left review comments

@konn konn konn approved these changes

@soulomoon soulomoon soulomoon approved these changes

@jvanbruegge jvanbruegge Awaiting requested review from jvanbruegge jvanbruegge is a code owner

@eddiemundo eddiemundo Awaiting requested review from eddiemundo eddiemundo is a code owner

@pepeiborra pepeiborra Awaiting requested review from pepeiborra

@wz1000 wz1000 Awaiting requested review from wz1000 wz1000 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@michaelpj @soulomoon @konn @fendor