Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: #1690 - Infix typed holes are now filled using infix notation #1708

Merged
merged 5 commits into from
Apr 13, 2021

Conversation

OliverMadine
Copy link
Collaborator

@OliverMadine OliverMadine commented Apr 11, 2021

fix: #1690

In follow up to my response to @July541's PR (#1694), I have written a different solution to cover the cases of filling holes with infix operators.

applying the "replace _ with (+)" code action to

test :: Int -> Int -> Int
test a1 a2 = a1 `_` a2

now results in

test :: Int -> Int -> Int
test a1 a2 = a1 + a2

rather than

test :: Int -> Int -> Int
test a1 a2 = a1 `(+)` a2

Note: This solution depends on processHoleSuggestions returning infix functions in their prefix form (e.g. (+)) to determine if a function infix/prefix. I imagine there might be a nicer way of doing this.

Edit: In some commit messages I have written "postfix" by mistake instead of "prefix"

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix, tests cases looks great, many thanks!

@jneira
Copy link
Member

jneira commented Apr 12, 2021

This solution depends on processHoleSuggestions returning infix functions in their postfix form (e.g. (+)) to determine if a function infix/postfix. I imagine there might be a nicer way of doing this.

The unique caveat i can think of is somebody is using already postfix notation and we turn it in infix one, is there some test case to check we will not do it?

@OliverMadine
Copy link
Collaborator Author

OliverMadine commented Apr 12, 2021

This solution depends on processHoleSuggestions returning infix functions in their postfix form (e.g. (+)) to determine if a function infix/postfix. I imagine there might be a nicer way of doing this.

The unique caveat i can think of is somebody is using already postfix notation and we turn it in infix one, is there some test case to check we will not do it?

Do you mean that we should check that infix form is not used when prefix is required?

So we should make sure

foo :: Int -> Int -> Int
foo = _ a1 a2

is filled to

foo :: Int -> Int -> Int 
foo a1 a2 = (+) a1 a2

?

I've added this as a test case.

@jneira
Copy link
Member

jneira commented Apr 12, 2021

yeah, that was my idea, many thanks

@jneira jneira added the merge me Label to trigger pull request merge label Apr 13, 2021
@mergify mergify bot merged commit 02d5c66 into haskell:master Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infix backticked holes get their backticks deleted once filled
3 participants