atom-haskell-archive / haskell-ghc-mod

haskell-ghc-mod atom package
https://atom.io/packages/haskell-ghc-mod
MIT License
69 stars 20 forks source link

Add support for filling holes using ghc-mod's auto command #174

Closed chrismwendt closed 7 years ago

chrismwendt commented 7 years ago

This adds support for filling holes using ghc-mod's auto command.

output

lierdakil commented 7 years ago

Thank you for your contribution!

I have to ask though, SuggestionListView seems almost identical to ImportListView. Do you think we could unite those to reduce code duplication? Also, I'm not a huge fan of your use of pre there, any particular reason you're using it?

Also, I think this functionality intersects with getCompletionsForHole in completion backend somewhat. Would you happen to have any thoughts on that?

chrismwendt commented 7 years ago

Oh yes, it would be good to merge the *ListViews. Maybe we could rename one to ListView and delete the other. I used pre because sometimes the output spans multiple lines (e.g. case suggestions) and the monospace font aligned it better, but I'll happily defer to your judgement on that one.

I actually didn't know about hole completions in autocomplete-haskell! I would actually prefer auto's output in the autocomplete list, rather than in a ListView. It operates differently, though, so there are cases where one is better than the other. For example, for this function:

a :: Maybe Bool
a = _

ghc-mod's auto suggests:

while autocomplete-haskell suggests:

In that case, auto had better suggestions, but auto seems to hang for other examples (e.g. https://gist.github.com/chrismwendt/59eff2b95e1f35b56c7d65907cd1057a) and doesn't even provide suggestions for some seemingly obvious cases (e.g. not _ should suggest True and False, but suggests nothing at all).

That case where ghc-mod seemingly hung scares me - I wouldn't want my editor to hang for that long. Maybe it's best to hold off merging this until that is addressed?

lierdakil commented 7 years ago

https://gist.github.com/chrismwendt/59eff2b95e1f35b56c7d65907cd1057a

Couldn't build/test your example. Lack of cabal file makes this unnecessarily hard ._. We can always run ghc-mod auto non-interactively and kill it by timeout if it's a concern.

while autocomplete-haskell suggests

Yes, ac-h works differently here, more in the vein of how Hoogle does it: it tries to not only match immediately obvious options, but also things like polymorphic functions etc by hole type. Of course this is pure heuristics, so it's not necessarily 'correct'. Also some Prelude identifiers are "strange", which includes primitive types, True/False, seq, error and undefined, since those are treated more like keywords and as such don't have a type annotation. Those end up in every completion list. I should probably fix it eventually.

So anyway, I was thinking that maybe it'd be a better option to enhance ac-h output with auto rather than add a separate function. I don't think we should replace existing hole autocompletion with auto of course, exactly for reasons you mention: auto doesn't necessarily provide the best options (or any options at all for that matter)

Also while on topic, probably could leverage refine to make ac-h suggestions a little bit better. Although I'm afraid straightforward implementation could end up being punishingly slow.

I used pre because sometimes the output spans multiple lines (e.g. case suggestions) and the monospace font aligned it better

I would argue that this should be solved with css (in a stylesheet!) rather than introducing new elements into tree. Something like white-space: pre; font-family: monospace; should do it I think? Reason being this leaves an option for customization user-side open, while using pre makes this... well, harder, for no good reason IMO. If we'll decide to move auto to autocompletion side, this discussion is purely academic though.

chrismwendt commented 7 years ago

Oh, of course I should have included the cabal and stack files, sorry about that! I updated https://gist.github.com/chrismwendt/59eff2b95e1f35b56c7d65907cd1057a with both as well as a screenshot of my system monitor during execution.

I like the approach that autocomplete-haskell takes to figuring out what should go in a hole, and :+1: to enhancing its output with auto instead of adding a new command. refine would be a nice extension as well.

Ah, good call on using a stylesheet instead. I'm not particularly well versed in HTML/CSS design, and pre was the quickest way I knew. Anyway, let's ditch this PR and think about moving it to autocomplete-haskell.

lierdakil commented 7 years ago

Closing in favor of #175, see if it makes sense to you. Also, it'll be easier for everyone if we have a shared branch, so I've added you as a collaborator.