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

Extend FS25 codefix to allow generating match cases from scratch #1309

Merged
merged 5 commits into from
Sep 4, 2024

Conversation

gbtb
Copy link
Contributor

@gbtb gbtb commented Jul 7, 2024

Hello 👋

I've made an addition to existing FS25 "Generate union pattern match case". Basically, it allows applying this codefix to an empty match ... with, so user isn't required to write first case branch for the codefix to work. I think that this codefix might be useful in slightly different contexts. Current implementation is more suitable for cases when already existing Union type gets extended, and therefore already existing match expression becomes non-exhaustive. But in my practice, when I'm writing small programs from scratch, I usually have a different situation cause I'm writing completely new match expressions.

I tried to develop it in backward-compatible way, so that previous logic for the codefix stays intact, and the code only falls back to a new logic if the original one is unable to produce results.

Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Looks good to me, @baronfel did you see this one yet?

@@ -211,6 +211,19 @@ let private tryFindPatternMatchExprInParsedInput (pos: Position) (parsedInput: P
| _ -> None
else
None)
|> Option.orElseWith (fun () ->
if synMatchClauseList.IsEmpty then
match debugPoint with
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you went with this extra check for the debug point.
I don't think it matters, checking if synMatchClauseList.IsEmpty probably is enough.

Copy link
Contributor Author

@gbtb gbtb Aug 19, 2024

Choose a reason for hiding this comment

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

I think I copied this check from preceding "branch" of orElseWith call chain. Honestly, I don't know what DebugPointAtBinding is here for 😅

@baronfel
Copy link
Contributor

baronfel commented Sep 4, 2024

Wow, I'm so sorry I missed this. I've kick-started the checks, will take a look once those report back. This looks awesome!

@baronfel
Copy link
Contributor

baronfel commented Sep 4, 2024

Checks are green! Thanks for adding the test and a cool feature overall. I'll merge and we can work on a new release.

@baronfel baronfel merged commit fbb6eb0 into ionide:main Sep 4, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants