Show documentation on hover for symbols defined in the same module #691
Conversation
There was a problem hiding this comment.
I had this in my list of things to fix. Thank you @wz1000 !
| @@ -50,6 +50,9 @@ type instance RuleResult GetDependencies = TransitiveDependencies | |||
| -- that module. | |||
| data TcModuleResult = TcModuleResult | |||
| { tmrModule :: TypecheckedModule | |||
| -- ^ warning, the ModIface in the tm_checked_module_info of the | |||
| -- TypecheckedModule will always be Nothing, use the ModIface in the | |||
| -- HomeModInfo instead | |||
There was a problem hiding this comment.
This is very helpful, thanks!
|
(a test would be very nice) |
|
@pepeiborra the test already existed :) (good to see a CI failure because of |
|
In hls we are about to add hlint support including refactoring and it will be broken with this. Could be return both version of the parsed modules and make hlint (and other tools that needs the original parsed module version) use the existing one? I think @shayne_fletcher mentioned some wip to make compatible the parsing with haddock options though. |
|
@jneira I'm confused, this patch is already included in the branch of ghcide that hls is using. |
|
Yeah, but as we have to make it optional for daml hlint i was proposing another way to do it: make two versions available at runtime instead only one or another using cpp. But it could be added afterwards, if it is really needed, i suppose. |
|
@wz1000 I think we need a configuration option before this can be merged to avoid a breaking change, unless @cocreature no longer needs it |
|
Ok, I can do that. However, I think @jneira has hlint working in the presence of this patch. |
When parsing a module, if parsing haddocks succeeds, then use them Previously, even though we were parsing modules twice, with and without haddocks, we were just returning the result of parsing without haddocks. The reason for this was that Opt_KeepRawTokenStream and Opt_Haddock do not interact nicely. We decided that for now it was better to fix an actual issue and then solve the problem when hlint requires a module with Opt_KeepRawTokenStream.
|
@pepeiborra done and rebased. |
|
@wz1000 i've not seen any error using the hlint plugin + GetParsedModule in hls so far, using a ghcide version with this patch. |
|
They are not fully incompatible. But the haddock parsing options doesn’t include some stuff, e.g., extensions iirc so HLint won’t show you the corresponding hints if you use haddock parsing. That said, I don’t care all that much about those hints (neither here nor in DAML) so it’s probably reasonable to go for this approach for now. Hopefully, this will be addressed on GHC’s side at some point. |
|
Is it not possible to manually tell hlint about extensions etc. by extracting them from the dynflags? /cc @ndmitchell |
|
You can tell HLint that the code should be parsed with the OverloadedStrings or GADTs extension just fine. So HLint will still be able to parse the file. You can't tell HLint that the |
|
Hlint also supports ANN pragmas for annotations which presumably need full tokens |
…askell/ghcide#691) * Show documentation on hover for symbols defined in the same module When parsing a module, if parsing haddocks succeeds, then use them Previously, even though we were parsing modules twice, with and without haddocks, we were just returning the result of parsing without haddocks. The reason for this was that Opt_KeepRawTokenStream and Opt_Haddock do not interact nicely. We decided that for now it was better to fix an actual issue and then solve the problem when hlint requires a module with Opt_KeepRawTokenStream. * Add option to decide which ParsedModule to return
…askell/ghcide#691) * Show documentation on hover for symbols defined in the same module When parsing a module, if parsing haddocks succeeds, then use them Previously, even though we were parsing modules twice, with and without haddocks, we were just returning the result of parsing without haddocks. The reason for this was that Opt_KeepRawTokenStream and Opt_Haddock do not interact nicely. We decided that for now it was better to fix an actual issue and then solve the problem when hlint requires a module with Opt_KeepRawTokenStream. * Add option to decide which ParsedModule to return
…askell/ghcide#691) * Show documentation on hover for symbols defined in the same module When parsing a module, if parsing haddocks succeeds, then use them Previously, even though we were parsing modules twice, with and without haddocks, we were just returning the result of parsing without haddocks. The reason for this was that Opt_KeepRawTokenStream and Opt_Haddock do not interact nicely. We decided that for now it was better to fix an actual issue and then solve the problem when hlint requires a module with Opt_KeepRawTokenStream. * Add option to decide which ParsedModule to return
When parsing a module, if parsing haddocks succeeds, then use them
Previously, even though we were parsing modules twice, with and without
haddocks, we were just returning the result of parsing without haddocks.
The reason for this was that Opt_KeepRawTokenStream and Opt_Haddock do
not interact nicely. We decided that for now it was better to fix an
actual issue and then solve the problem when hlint requires a module
with Opt_KeepRawTokenStream.