Support infix completions #1267
Conversation
|
Looking good. I think as time passes we are going to end up with ever more tightly focused contexts, so the completion gets more reliable. |
| item ^. label `shouldBe` "filter" | ||
| item ^. kind `shouldBe` Just CiFunction | ||
| item ^. insertTextFormat `shouldBe` Just Snippet | ||
| item ^. insertText `shouldBe` Just "filter`" |
There was a problem hiding this comment.
Can we add some assertions about the placeholders the function is suggesting after completing? It would be painful if the placeholders we suggest contain the left hand side of the function
There was a problem hiding this comment.
The placeholders are the expected type? e.g. in the case of filter: filter' {2:[a]}?
There was a problem hiding this comment.
With backticks the placeholders should just drop the first element right? Because we can assume that the left hand side will supply the first argument
There was a problem hiding this comment.
I dont know. Is this really wanted? I find myself often writing such functions in exisiting expressions, normally I dont want these placeholders...
There was a problem hiding this comment.
This is something that has crossed my mind too, but I think it needs a more general solution, to include in the context if there is anything to the right already, and in that case not offering placeholders
There was a problem hiding this comment.
Maybe I am also misunderstanding the question. This particular completion omits the placeholders completely at the moment.
There was a problem hiding this comment.
So you are proposing that we keep it like that? Which could work. If anyone feels strongly about it we can add them again. I do think that infix operations tend to be binary, so the missing placeholder(s) is probable not an issue.
There was a problem hiding this comment.
Yes, I think it would be better and we can add it anytime.
|
I think we should wait for the more complete tests before merging this. |
Yes, with some workaround. First I dont use stack to get the hie executable, I just use the one on the path and then I can use |
|
Ok, LGTM, will merge after the pending monthly release. |
closes #1259
[ ] More Documentation