Merge ghcide repository (replacing the submodule) #702
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Add some troubleshooting notes. * Update README with link to docker-ghcide-neovim instructions. * Update README
- retrieve runtime version from ghc executable, not from pkg db (ghc-check 0.3.0.0) - Do not error when unable to retrieve runtime version
* [#518] Build ghcide with GHC 8.10.1 Resolves #518 * Move CPP logic to the Compat module * Revert changes to mkHieFile * Add local fork of HieAst for 8.10.1 The fix for mkHieFile didn't make it into 8.10.1, so the override is still needed * Ignore hlint in src-ghc810/HieAst.hs * Whitelist CPP for Development.IDE.GHC.Orphans * [#518] Build ghcide with GHC 8.10.1 Resolves #518 * Move CPP logic to the Compat module * Revert changes to mkHieFile * Add local fork of HieAst for 8.10.1 The fix for mkHieFile didn't make it into 8.10.1, so the override is still needed * Ignore hlint in src-ghc810/HieAst.hs * Whitelist CPP for Development.IDE.GHC.Orphans * Plugin tests known broken in 8.10.1 (#556) * Bump up ghc-check version Co-authored-by: Pepe Iborra <[email protected]> Co-authored-by: pepe iborra <[email protected]>
* Extend nix explanations in README * Correct ghcide-nix url Co-authored-by: Domen Kožar <[email protected]> Co-authored-by: Domen Kožar <[email protected]>
Replace openDoc' with createDoc which sends out workspace/didChangedWatchedFiles notifications
By collecting the fieldOcc names in the data con args
* Track dependencies when using qAddDependentFile Closes #492 * Add test for qAddDependentFile * Update test/exe/Main.hs Co-authored-by: Moritz Kiefer <[email protected]> * Update test/exe/Main.hs Co-authored-by: Moritz Kiefer <[email protected]> Co-authored-by: Moritz Kiefer <[email protected]>
* Testsuite: Only run with --test if necessary * Add (failing) test to check GotoHover.hs file compiles * Fix compilation of GotoHover.hs
* Rats: Fix space leak in withProgress Eta-expanding the function means GHC no longer allocates a function closure every time `withProgress` is called (which is a lot). See: https://www.joachim-breitner.de/blog/763-Faster_Winter_5__Eta-Expanding_ReaderT * Rats: Share computation of position mapping Ensure that PositionMappings are shared between versions There was a quadratic space leak as the tails of the position maps were not shared with each other. Now the space usage is linear which is produces more acceptable levels of residency after 3000 modifications. * Rats: Eta-expand modification function See: https://www.joachim-breitner.de/blog/763-Faster_Winter_5__Eta-Expanding_ReaderT * Add a comment warning about eta-reducing * Distinguish between a Delta and a Mapping in PositionMapping A Delta is a change between two versions A Mapping is a change from the current version to a specific older version. Fix hlint Fix hlint
* Refactor rawDependencyInformation There are two reasons why this patch is good: 1. We remove the special case of the initial module from the dependency search. It is now treated uniformly like the rest of the modules. 2. rawDependencyInformation can now take a list of files and create dependency information for all of them. This isn't currently used but on my fork we have a rule which gets the dependency information for the whole project in order to create a module graph. It seemed simplest to upstream this part first, which is already a strict improvement to make the overal patch easier to review. * Make indentation not depend on identifier length Co-authored-by: Moritz Kiefer <[email protected]>
Follow up from #557. We definitely want the progress state to be fully evaluated, so demand that with evaluating functions like evaluate and $!, rather than relying on the compiler to get it right. My guess is the `$!` is unnecessary now we have `evaluate`, but it's also not harmful, so belt and braces approach.
* Drop interface loading diagnostics * No reason to skip the --test flag anymore
* Update to hie-bios 0.5.0 * Fix test-cases due to changes in the direct cradle * Update test/exe/Main.hs comment Co-authored-by: Moritz Kiefer <[email protected]> Co-authored-by: Moritz Kiefer <[email protected]>
In 0.18.4 deprioritise was renamed reschedule, so follow the new name.
* Add kakoune installation instructions * Add additional files to roots field
* Multi component support In this commit we add support for loading multiple components into one ghcide session. The current behaviour is that each component is loaded lazily into the session. When a file from an unrecognised component is loaded, the cradle is consulted again to get a new set of options for the new component. This will cause all the currently loaded files to be reloaded into a new HscEnv which is shared by all the currently known components. The result of this is that functions such as go-to definition work between components if they have been loaded into the same session but you have to open at least one file from each component before it will work. Only minimal changes are needed to the internals to ghcide to make the file searching logic look in include directories for all currently loaded components. The main changes are in exe/Main.hs which has been heavily rewritten to avoid shake indirections. A global map is created which maps a filepath to the HscEnv which should be used to compile it. When a new component is created this map is completely refreshed so each path maps to a new Which paths belong to a componenent is determined by the targets listed by the cradle. Therefore it is important that each cradle also lists all the targets for the cradle. There are some other choices here as well which are less accurate such as mapping via include directories which is the aproach that I implemented in haskell-ide-engine. The commit has been tested so far with cabal and hadrian. Also deleted the .ghci file which was causing errors during testing and seemed broken anyway. Co-authored-by: Alan Zimmerman <[email protected]> Co-authored-by: fendor <[email protected]> * Final tweaks? * Fix 8.4 build * Add multi-component test * Fix hlint * Add cabal to CI images * Modify path * Set PATH in the right place (hopefully) * Always generate interface files and hie files * Use correct DynFlags in mkImportDirs You have to use the DynFlags for the file we are currently compiling to get the right packages in the package db so that lookupPackage doesn't always fail. * Revert "Always generate interface files and hie files" This reverts commit 820aa241890c4498c566e29b0823a803fb2fd297. * remove traces * Another test * lint * Unset env vars set my stack * Fix extra-source-files As usual, stack doesn’t understand Cabal properly and doesn’t seem to like ** wildcards so I’ve enumerated it manually. * Unset env locally Co-authored-by: Alan Zimmerman <[email protected]> Co-authored-by: fendor <[email protected]> Co-authored-by: Moritz Kiefer <[email protected]>
* Prepare release of ghcide 0.2.0 * Fix year in copyright notices * Credit chshersh for the 8.10 support
* Initial benchmark suite, reusing ideas from Neil's post https://neilmitchell.blogspot.com/2020/05/fixing-space-leaks-in-ghcide.html * Add an experiment for code actions without edit * formatting * fix code actions bench script * error handling + options + how to run * extract Positions and clean up imports (Neil's review feedback) * replace with Extra.duration * allow ImplicitParams * add bench to the cradle * applied @mpickering review feedback * clean up after benchmark * remove TODO
* ShakeSession and shakeRunGently Currently we start a new Shake session for every interaction with the Shake database, including type checking, hovers, code actions, completions, etc. Since only one Shake session can ever exist, we abort the active session if any in order to execute the new command in a responsive manner. This is suboptimal in many, many ways: - A hover in module M aborts the typechecking of module M, only to start over! - Read-only commands (hover, code action, completion) need to typecheck all the modules! (or rather, ask Shake to check that the typechecks are current) - There is no way to run non-interfering commands concurrently This is an experiment inspired by the 'ShakeQueue' of @mpickering, and the follow-up discussion in mpickering/ghcide#7 We introduce the concept of the 'ShakeSession' as part of the IDE state. The 'ShakeSession' is initialized by a call to 'shakeRun', and survives until the next call to 'shakeRun'. It is important that the session is restarted as soon as the filesystem changes, to ensure that the database is current. The 'ShakeSession' enables a new command 'shakeRunGently', which appends work to the existing 'ShakeSession'. This command can be called in parallel without any restriction. * Simplify by assuming there is always a ShakeSession * Improved naming and docs * Define runActionSync on top of shakeEnqueue shakeRun is not correct as it never returns anymore * Drive progress reporting from newSession The previous approach reused the shakeProgress thread, which doesn't work anymore as ShakeSession keeps the ShakeDatabase open until the next edit * Deterministic progress messages in tests Dropping the 0.1s sleep to ensure that progress messages during tests are deterministic * Make kick explicit This is required for progress reporting to work, see notes in shakeRun As to whether this is the right thing to do: 1. Less magic, more explicit 2. There's only 2 places where kick is actually used * apply Neil's feedback * avoid a deadlock when the enqueued action throws * Simplify runAction + comments * use a Barrier for clarity A Barrier is a smaller abstraction than an MVar, and the next version of the extra package will come with a suitably small implementation: ndmitchell/extra@98c2a83 * Log timings for code actions, hovers and completions * Rename shakeRun to shakeRestart The action returned by shakeRun now blocks until another call to shakeRun is made, which is a change in behaviour,. but all the current uses of shakeRun ignore this action. Since the new behaviour is not useful, this change simplifies and updates the docs and name accordingly * delete runActionSync as it's just runAction * restart shake session on new component created * requeue pending actions on session restart * hlint * Bumped the delay from 5 to 6 * Add a test for the non-lsp command line * Update exe/Main.hs Co-authored-by: Moritz Kiefer <[email protected]>
The benchmark script uses git worktree. The upstream branch contains a ghcide submodule, which is not well supported by worktree. Once this PR has been merged and the upstream branch no longer contains a git submodule, we can reenable it in the bench config
I missed these previously
These tests were underspecified and broke with the recent improvements to ghcide diagnostics in haskell/ghcide#959 and included in this merge. Fixed by waiting specifically for the typecheck diagnostics and by being less prescriptive in the number and order of code actions
The ghcide merge includes haskell/ghcide#948 which removes the language extension code actions This makes the associated func-test fail, because the HLS plugin does not pass the test (only the ghcide code action did). This is because the HLS plugin uses commands, and the tests do not wait for the command edit to be applied. The fix is to change the HLS plugin to return a code action with edits and no commands
With so many github actions (>60) we cannot afford to run on every push
Reminder that ghcide requires at least 2 capabilities
This is needed to build with Cabal v1 if ghc is built with DYNAMIC_GHC_PROGRAMS=NO which is the case e.g. in Windows
```
Error: While constructing the build
plan, the following exceptions were
encountered:
In the dependencies
for shake-bench-0.1.0.0:
Chart-diagrams needed, but the stack
configuration has no
specified version
(latest matching
version is 1.9.3)
diagrams needed, but the stack
configuration has no
specified version (latest
matching version is 1.4)
diagrams-svg needed, but the stack
configuration has no
specified version
(latest matching
version is 1.4.3)
needed since shake-bench is a build
target.
```
Error:
Error: While constructing the build
plan, the following exceptions were
encountered:
In the dependencies
for diagrams-postscript-1.4.1:
hashable-1.3.0.0 from stack
configuration does
not
match >=1.1 && <1.3
(latest matching
version is 1.2.7.0)
lens-4.18 from stack configuration
does not
match >=4.0 && <4.18
(latest matching version
is 4.17.1)
needed due to shake-bench-0.1.0.0
-> diagrams-postscript-1.4.1
|
@jneira this is going to be ready tonight. What's the status of the 8.10.3 release? |
|
@jneira this is ready to merge. Is there any reason not to do the 8.10.3 release from a branch? |
|
mmm really no, I would prefer to release from master, to not have to keep the branch forever (if you want the exact history of the release, to bisect or whatever) but I think the save of time for this "little maneavour" could worth it |
|
we can try to make the release on top of this and fallback to the branch if there are problems |
|
@jneira it is standard practice to make point releases from release branches. I don't think that keeping the branch forever has any real downsides. If the 8.10.3 release is made from master it will include the changes in this PR (a ghcide version bump) as well as all the other changes in master: the reworked Eval plugin, the new Class plugin, etc. |
pepeiborra
added a commit
that referenced
this pull request
Dec 29, 2020
|
Is the plan to still keep |
|
Well, create release branches usually is related with a workflow where you are willing to create bug fix releases on them, and that is not our workflow for now. And consistency matters, if you can trace all releases on master you will be surprised by that one. But I am not strongly against, let's merge it and I will link in the release the branch used one |
|
@mpickering you can now get a slimmed down version of hls, with no plugins or formatters: and you can add plugins one by one with as described in #164 (comment) |
pepeiborra
added a commit
to pepeiborra/ide
that referenced
this pull request
Dec 29, 2020
pepeiborra
added a commit
to pepeiborra/ide
that referenced
this pull request
Dec 29, 2020
But you know that HLS now has cabal flags to select which plugins get linked, so why not use that? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The merge was performed using the command:
I have extended the Github CI to run the ghcide fmt, test and benchmark scripts.
The next step will be to transfer all the GitHub Issues