-
Notifications
You must be signed in to change notification settings - Fork 59
Explore type based autocomplete #493
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
Changes from 3 commits
7db9012
4362bdf
87508b3
5ed8bb4
d121e73
57e7cce
b125207
431ab48
bdd2e11
bea78fe
5ebe602
b85c4ae
dd8333e
ffa06f4
37e4f9b
c2e2adf
d1d86e8
90ccf19
d54b562
2b29291
27532b0
e87ad5a
f79a38d
b260d38
54e596c
8c49fa5
c8f112a
49e45d8
7c8a1ad
b34c35d
7b1b8cd
f79f21b
0df2b0c
33cd645
f21a06e
44698e4
48b7bec
295be6b
5a2097e
da23ca8
39a35ec
f9515d1
a268d24
9f511cd
ef427d2
555110f
cf8b472
3a6a6e5
339417c
928e84e
d134b8b
0d06fc6
d2ef230
013680e
303eb57
858be8f
d5ced33
1679769
f262a50
28dddca
8a98793
5647a1e
2c1b721
5d69adb
f061119
82796f9
6a0a884
f98db04
5558995
5320d9e
36832cc
eb3eb80
859c3fd
e67f63c
64f4f77
40e3ec1
e330b68
fc48ca8
cd2161a
0522d01
16a6f0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
type someVariant = One | Two | Three | Four | ||
|
||
let someVariantToString = (~someVariant) => | ||
switch someVariant { | ||
| One => "One" | ||
| Two => "Two" | ||
| Three => "Three" | ||
| Four => "Four" | ||
} | ||
|
||
// let x = someVariantToString(~someVariant= | ||
// ^com | ||
|
||
module SomeComponent = { | ||
@react.component | ||
let make = (~whatever) => { | ||
someVariantToString(~someVariant=whatever)->React.string | ||
} | ||
} | ||
|
||
// let jsx = <SomeComponent whatever= | ||
// ^com |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
DCE src/Dce.res | ||
issues:235 | ||
issues:241 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
Complete src/TypeContextCompletion.res 10:44 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's see what's going on here first. But if you look at |
||
posCursor:[10:44] posNoWhite:[10:43] Found expr:[10:11->18:1] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now, since the cursor in autocomplete is always after what you want to complete, we need to go back to find anything interesting. And eat up any white space. In this case, just one character back to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have hit an AST expression which goes from line Why so many lines? That's parser error recovery, which apparently is taking in also the module definition. |
||
Pexp_apply ...[10:11->10:30] (~someVariant10:32->10:43=...[13:0->17:3]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Digging inside, we found a sub-expression that is an application. |
||
Completable: CnamedArg(Value[someVariantToString], "", [someVariant]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is where it gets a bit hairy. Not sure why it decides it's a named arg completion given that it's in the expression assigned to the label. It could be because that expression is exactly where parser recovery happened and some heuristic is needed because of the uncertainty that parser recovery brings. Anyhow the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK looking at the code, I think we're hitting this case: | [] ->
if posAfterFunExpr <= posBeforeCursor && posBeforeCursor < endPos then
Some (CnamedArg (contextPath, "", allNames)) So we went through the entire application and did not find anything interesting. So we'll just default to named arg completion. To see why this case is needed one can just turn it off and see what tests fail. |
||
Found type for function (~someVariant: someVariant) => string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to check where this is coming from: presumably named arg completion finding the type of the function. |
||
[] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As predicted, no result. |
||
|
||
Complete src/TypeContextCompletion.res 20:37 | ||
posCursor:[20:37] posNoWhite:[20:36] Found expr:[20:14->20:37] | ||
JSX <SomeComponent:[20:14->20:27] whatever[20:28->20:36]=...__ghost__[0:-1->0:-1]> _children:None | ||
[] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should get rid of these differences. This is testing several things at once and it's not necessary.