haskell
Limit the number of valid hole fits
@pepeiborra commented in the original pr
I think the limit (be it 5 or 15) should be moved to the GHC flags that we set in GHC.Compat, and make sure we don't override a user value.
I think we should take in account the ghc flag in some way, at least to honour it if the user has set it on purpose. I would be a little bit surprised if i set 20 and only see 10 in the editor.
So i think that maybe we should follow the recommendation and set the flag auto for the specific ghc session for hls if users didnt set it explicitly.
@expipiplus1 Tests from https://github.com/haskell/haskell-language-server/blob/b970e250217d970060a211022852d863e34060aa/ghcide/test/exe/Main.hs#L2433-L2458 use filtered actions, so these keep failing.
hmm, the tests don't run for me locally. Would it be possible for someone else to take over this PR, please?
❯ cabal run test:ghcide-tests -- --pattern "postfix hole"
Up to date
ghcide
code actions
fill typed holes
postfix hole uses postfix notation of infix operator: FAIL
Exception: ghcide: createProcess: runInteractiveProcess: exec: does not exist (No such file or directory)
Your CI appears to be blocked by https://github.com/haskell/haskell-language-server/issues/2016
tests failing:
Test suite ghcide-tests: RUNNING...
ghcide
code actions
fill typed holes
postfix hole uses postfix notation of infix operator: FAIL (1.82s)
test/exe/Main.hs:5546:
Found no matching actions for "replace _ with (+)" in ["replace _ with test","replace _ with (-)","replace _ with (*)","replace _ with asTypeOf","replace _ with const","replace _ with pure","replace _ with (Some hole fits suppressed; use -fmax-valid-hole-fits=N or -fno-max-valid-hole-fits)","replace _ with curry _","replace _ with ($) _","replace _ with ($!) _","replace _ with const _","replace _ with flip _","replace _ with id _"]
Use -p '/postfix hole uses postfix notation of infix operator/' to rerun this test only.
filling infix type hole uses infix operator: FAIL (1.83s)
test/exe/Main.hs:5546:
Found no matching actions for "replace _ with (+)" in ["replace _ with test","replace _ with (-)","replace _ with (*)","replace _ with asTypeOf","replace _ with const","replace _ with pure","replace _ with (Some hole fits suppressed; use -fmax-valid-hole-fits=N or -fno-max-valid-hole-fits)","replace _ with curry _","replace _ with ($) _","replace _ with ($!) _","replace _ with const _","replace _ with flip _","replace _ with id _"]
Use -p '/filling infix type hole uses infix operator/' to rerun this test only.
cradle
dependencies
Note! Valid hole-fits are a lot faster now on HEAD, so this might not be required anymore.
That's good news but my gripe was that when there are too many to choose from at a glance then one may as well just type it. The speed at which they appear wasn't really an issue for me here.
On Wed, 6 Oct 2021, 8:11 am Matthías Páll Gissurarson, < @.***> wrote:
Note! Valid hole-fits are a lot faster now on HEAD, so this might not be required anymore.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/haskell/haskell-language-server/pull/1914#issuecomment-935122270, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGRJXA3VBGKM7UHEIZTPSLUFOH2NANCNFSM46TIJRFQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
Hole fitting solving would always be costly for the foreseeable futuristic future.
I consider 10 as a sane default (which would terminate mostly in the sane time & not clutter much, which I would prefer).
Lets not bikeshed - don't let the ideal be the enemy of the good.
@expipiplus1 would you be kind to push it over the last bump? :heart:
Sorry, I won't be able to get to this in a timely manner. Happy for someone else to take over though.
Yep. Looked into commits.
Rebasing for this diff was a bit excessive :rofl:
Ok then somebody from us would finish this.
This continues to be desirable but this implementation seems dead. Does anyone want to pick it up?
