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

decoder: Refactor newExpression() & plumb PathContext #189

Merged
merged 1 commit into from
Jan 17, 2023

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Jan 16, 2023

This is primarily to make it easier to "hand-over" completion/hover/etc. between different expression types (which do not /need to/ have access to all of PathDecoder) and reduce diff conflicts between upcoming PRs where each involves different expression type.

It is expected that all complex types (list, set, tuple, map, object) may contain either Reference (hence need ReferenceTargets) or AnyExpression (hence need future FunctionsMap and possibly ReferenceTargets too).

I have briefly considered creating a separate type, e.g. ExpressionContext or pass these things via context.Context, or use the static newExpression() in all places where (*PathDecoder) newExpression() is now used, but concluded that it would all be premature optimization at this point.

@radeksimko radeksimko added enhancement New feature or request technical-debt labels Jan 16, 2023
@radeksimko radeksimko changed the title decoder: Refactor newExpression & plumb PathContext decoder: Refactor newExpression() & plumb PathContext Jan 16, 2023
@radeksimko radeksimko marked this pull request as ready for review January 16, 2023 20:08
@radeksimko radeksimko requested a review from a team as a code owner January 16, 2023 20:08
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@radeksimko radeksimko merged commit f9adfd8 into main Jan 17, 2023
@radeksimko radeksimko deleted the f-refactor-newExpression branch January 17, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request technical-debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants