Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add code completion for chained value factories #11
Add code completion for chained value factories #11
Changes from 2 commits
38e1d64
fd499f9
568fcb6
904f093
6047401
d59b830
fab2d86
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
💭 Given that with a big enough schema this might be an expensive operation, I would suggest finding a way to build the index once (or maybe precompute/memoize some parts of it) and invalidate it on schema changes.
For instance, some precomputed or memoized state could include:
layout.settingsDirectory
starting from that type (along with the type oflayout.settingsDirectory
)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've addressed this, please take a look.
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.
💭 Currently, in addition to the top-level type, you can use the members of the nested containers. For instance, in
foo { bar { baz = qux.quux() } }
qux
could be a member of:bar
's receiverfoo
's receiverWe should probably discuss if we want to keep it that way, but if we do, these nested receivers should also contribute the value factory groups to completion.
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.
As it works now,
qux
will be found regardless which of those receivers it's on and will be proposed as a potential value for properties with the same type as the return type of thequux()
function.I think the real problem we should think about is that
qux.quux()
will be proposed as a potential value forbaz
even ifqux
comes from a totally unrelated block, which is part of the top-level receiver, but is unrelated tofoo
(thus by extension unrelated tobar
andbaz
too).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.
Added a TODO.