Complete switch constructor #415
Conversation
| Complete tests/src/Completion.res 362:8 | ||
| posCursor:[362:8] posNoWhite:[362:7] Found expr:[360:8->365:3] | ||
| posCursor:[362:8] posNoWhite:[362:7] Found expr:[361:2->365:3] | ||
| XXX Pexp_match with 1 cases not handled |
There was a problem hiding this comment.
This shows that parser recovery leads to a single case being considered in the switch.
| Complete tests/src/Completion.res 367:30 | ||
| posCursor:[367:30] posNoWhite:[367:29] Found expr:[367:11->367:30] | ||
| posCursor:[367:30] posNoWhite:[367:29] Found expr:[367:16->367:30] | ||
| XXX Pexp_match with 1 cases not handled |
There was a problem hiding this comment.
This shows that parser recovery also gives 1 case when no more cases others follow.
| Complete tests/src/Completion.res 372:8 | ||
| posCursor:[372:8] posNoWhite:[372:7] Found expr:[370:8->376:3] | ||
| posCursor:[372:8] posNoWhite:[372:7] Found expr:[371:2->376:3] | ||
| XXX Pexp_match with 2 cases not handled |
There was a problem hiding this comment.
This example confirms that the first 2 cases are squashed into one.
| XXX Pexp_match with 1 cases not handled | ||
| XXX first case pattern:[362:7->364:5] expression:[364:9->364:10] | ||
| posCursor:[362:8] posNoWhite:[362:7] Found pattern:[362:7->364:5] | ||
| posCursor:[362:8] posNoWhite:[362:7] Found pattern:[362:7->362:8] |
There was a problem hiding this comment.
From the location, this could be the pattern T.
| XXX first case pattern:[362:7->364:5] expression:[364:9->364:10] | ||
| posCursor:[362:8] posNoWhite:[362:7] Found pattern:[362:7->364:5] | ||
| posCursor:[362:8] posNoWhite:[362:7] Found pattern:[362:7->362:8] | ||
| XXX Ppat_construct T:[362:7->362:8] |
There was a problem hiding this comment.
Indeed here's the T pattern.
| posCursor:[373:21] posNoWhite:[373:20] Found expr:[372:2->376:3] | ||
| posCursor:[373:21] posNoWhite:[373:20] Found pattern:[373:7->375:5] | ||
| posCursor:[373:21] posNoWhite:[373:20] Found pattern:[373:7->373:21] | ||
| XXX Ppat_construct AndThatOther.T:[373:7->373:21] |
There was a problem hiding this comment.
Looks like we can catch more complex paths too.
| Printf.printf "XXX Ppat_construct %s:%s\n" | ||
| (lidPath |> String.concat ".") | ||
| (Loc.toString lid.loc); | ||
| setResult (Cpath (CPId (lidPath, Value))) |
There was a problem hiding this comment.
Triggering completion as a Value. There's no need for specific completions for Constructor because of the upper case. Lower-case ids are normal values, and upper-case values are constructors.
| "detail": "That\n\ntype v = This | That", | ||
| "documentation": null | ||
| }, { | ||
| "label": "This", |
There was a problem hiding this comment.
This and That are found as expected.
| "detail": "This\n\ntype v = This | That", | ||
| "documentation": null | ||
| }, { | ||
| "label": "TableclothMap", |
There was a problem hiding this comment.
This is a module starting with T which happens to be in scope.
This is accurate, as the user might be in the process of writing ThatSpecificModule.SomeConstructor.
There was a problem hiding this comment.
I disagree with this, except in the rare case where the type of the switch variable is not known (because it might be used to disambiguate, as you suggested). That should only happen once, or in many cases never.
My understanding is that once the type is known the compiler no longer needs disambiguation to find constructors. At that point seeing all scope modules in the autocomplete list is just noise.
I'm about to start looking at how to remove modules from the autocomplete list in this scenario, but now I realise it was intentional... can we discuss whether removing them is a good idea?
There was a problem hiding this comment.
I think you're interested in type contexts, here: #494
Not this PR.
There was a problem hiding this comment.
I am, but then I found this PR and it looked like completion in switch statements already included all the valid constructors.
There was a problem hiding this comment.
This is an ongoing process. Not everything is at the level it could be.
In the process, some techniques have been used to aid autocompletion where the alternative would be to get nothing.
| @@ -1030,6 +1030,7 @@ Completable: Cpath Value[SomeLocal] | |||
| }] | |||
|
|
|||
| Complete tests/src/Completion.res 271:20 | |||
| posCursor:[271:20] posNoWhite:[271:19] Found pattern:[271:7->274:3] | |||
There was a problem hiding this comment.
This, and other similar lines indicate that this affects a bit existing test cases, but does not lead to a different outcome.
It just means that the relevant type, found below, happens to be inside a pattern containing the cursor.
No description provided.