Support for custom struct tags and runtime context#942
Open
samlown wants to merge 4 commits intoexpr-lang:masterfrom
Open
Support for custom struct tags and runtime context#942samlown wants to merge 4 commits intoexpr-lang:masterfrom
samlown wants to merge 4 commits intoexpr-lang:masterfrom
Conversation
Member
|
Unfortunately, this approach will not work. |
Author
|
@antonmedv sorry, I'd noticed that after submitting. Small fix for an inefficient context assignment seems to have corrected the stats for most calls. |
Author
|
While testing locally, I discovered an issue with tag handling and extras after the commas. Latest commit fixes those. |
Member
|
What if instead of context, we just gonna use some global variable? |
Author
That's certainly the easiest option. If however multiple packages need to make use of expr independently, they could have different expectations of the tag to use, very hard to debug and fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR tries to solve the issue of defining a custom struct tag to use when determining the name of fields. This is especially useful when dealing with a large and complex set of Go structs that already have a
jsontag set.Part of the implementation required the presence of a context to store the Tag being used, and ensure the struct caches are correctly associated with the context, given that different tags may be used with different compiler blocks. This adds a lot more complication. The runtime package also required a clearer context for some of the functions, it might be interesting to move all them to the
runtime.Contextfor consistency, although not required.The
builtin.Functionstruct now has aFuncWithContextfield, that always expects a function with a context argument and is required for some of the default methods that depend on the runtime context to be able to correctly determine some of the field details. I've used a generic context here and added support for aRunWithContextmethod to the vm that may be useful in some additional circumstances.Fix: #234
Replaces: #829
Disclaimer: Claude Code was used to do much of the heavy lifting.