parsonsmatt / intero-neovim

A neovim plugin for Intero, forked from ghcmod-vim
218 stars 28 forks source link

Fix type_at for ghci backend #134

Closed gilligan closed 5 years ago

gilligan commented 6 years ago

This fixes https://github.com/parsonsmatt/intero-neovim/issues/133 for me - i hope there is no other ghci version that behaves differently ? At least for ghc 8.0.2 and 8.2.2 this fixes it.

parsonsmatt commented 6 years ago

Looks good to me. Pinging @owickstrom as the author of that bit who might know why it's like that.

owickstrom commented 6 years ago

I've tried this out with:

GHC versions: 8.0.2, 8.2.2 Custom backend commands: ghci, cabal new-repl

First, I cannot get type-at info for local identifiers without manually doing :set +c and then reloading. I think this is required by regular GHCi (non-intero) for it to load that module info, just wanted to note that.

Without applying the patch, i.e. running off master, I get type-at to work properly for any identifier (given :set +c), in all combinations of GHC versions and commands.

When I do apply this patch, I only get type info for imported identifiers from other modules, not from local identifiers. Basically the same as regular :type. Same for all combinations.

@gilligan Do you have some minimal example Haskell module/project to reproduce the behavior your observing? Including the backend configuration you are using.

gilligan commented 6 years ago

Alright - so I tested this again and my conclusion is: it only works for me with cabal repl and only if :set +c is set (it is in my .ghci for the tests below). I tested it in the terminal as well ...

[15:32:59] gilligan :: comonad  ➜  ~/Development/type-at-test » ghci
GHCi, version 8.0.2: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/gilligan/Development/type-at-test/.ghci
Prelude> :l Main

<no location info>: error: module ‘Main’ is a package module
Failed, modules loaded: none.
Prelude> :set -i./src
Prelude> :l Main
[1 of 1] Compiling Main             ( src/Main.hs, interpreted )
Ok, modules loaded: Main.
Collecting type info for 1 module(s) ...
*Main> :type-at "src/Main.hs" 3 1 3 5 it
Couldn't guess that module name. Does it exist?
*Main>
Leaving GHCi.
[15:33:36] gilligan :: comonad  ➜  ~/Development/type-at-test » cabal repl
Preprocessing executable 'type-at-test' for type-at-test-0.1.0.0..
GHCi, version 8.0.2: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/gilligan/Development/type-at-test/.ghci
[1 of 1] Compiling Main             ( src/Main.hs, interpreted )
Ok, modules loaded: Main.
Collecting type info for 1 module(s) ...
*Main> :type-at "src/Main.hs" 3 1 3 5 it
it :: a -> a

Am I maybe missing something obvious? Otherwise my conclusion is to use cabal repl instead of ghci

@owickstrom you said for you it worked with ghci as well?

@parsonsmatt I am starting to think it would be great if we had tests for the different backends but i will admit they might be somewhat painful to write ;-}

owickstrom commented 6 years ago

OK, I could reproduce your problem now, and it seems to come down to :set -i./src being the problem. If I do :set -isrc it works. So, probably a bug in GHCi with how it loads the required module info, or the :type-at command and file paths.

If you want to use plain ghci as a command, I suggest you add the include-flags to the command, and specify paths without ./ for now. At least to see if that fixes your issue. Thanks!

parsonsmatt commented 6 years ago

We have vader for pure-vim tests, but I have absolutely no idea how we'd write tests for the GHCi interaction :(

ghci is going to require :set +c and a reload. intero does that automatically. I wonder if there's a way we can change the initial command to have :set +c on by default?

owickstrom commented 6 years ago

@parsonsmatt I think +c might be something we should only inform about, saying that it's required for non-top-level type information without intero. I dont' think we should enable it by default. The GHCi manual doesn't say much about if there's any performance penalty of enabling it, but it just seems dodgy to "sneak it in." Just my two cents, and please tell me if I've misunderstood you.

parsonsmatt commented 6 years ago

There should be a flag that indicates to the user whether or not :type-at functionality works. For intero it woudl always be true, but for GHCi it would need to be enabled manually.

Doing :set +c at the beginning of the GHCi invocation would prevent the need for the plugin to know whether or not the setting is enabled.

parsonsmatt commented 6 years ago

I'm increasingly convinced that we should be doing some GHCi customization from the plugin. :set +c for GHCi and :set -fobject-code for both are probably good benefits.