Skip to content
This repository was archived by the owner on Mar 14, 2021. It is now read-only.

Conversation

@chrismwendt
Copy link
Collaborator

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

output

return Promise.resolve [] unless buffer.getUri()?
crange = Util.tabShiftForRange(buffer, crange)
@queueCmd 'typeinfo',
interactive: @caps?.interactiveCaseSplit ? false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's safe to use interactive: true here. interactiveCaseSplit is a workaround for a ghc-mod bug where case-split command crashed interactive mode, but I don't think it's the case with auto.

NB: sig-fill just crashed in most cases since ghc-mod-5.3, and was finally fixed in 5.6 IIRC, so the same workaround is applied there to avoid crashing interactive mode.

@lierdakil
Copy link
Contributor

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
Copy link
Collaborator Author

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:

  • Nothing
  • Just otherwise

while autocomplete-haskell suggests:

  • fmap
  • Bool
  • Double
  • False
  • ...

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
Copy link
Contributor

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
Copy link
Collaborator Author

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 👍 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
Copy link
Contributor

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.

@lierdakil lierdakil closed this Sep 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants