Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

std.mergePatch should create resulting object fields at Normal visibility, not Unhide #255

Merged

Conversation

JoshRosen
Copy link
Contributor

This PR changes the default visibility of fields in objects created by std.mergePatch: previously they were at Unhide visibility, equivalent to the ::: operator, but as of this PR they are now at Normal standard visibility.

Concretely, previously

{a:: 0} + std.mergePatch({a: 1}, {})

would return {a: 1} but after this patch it returns {a:: 1), i.e. it preserves the hidden field.

It turns out that v0.20.0 of google/jsonnet and google/go-jsonnet also differ in their behavior here. That, in turn, is due to a behavior difference in the default visibility of fields in object comprehension results: jsonnet marks object comprehension fields as forced-visible, while go-jsonnet marks them as inherited visibility; see google/jsonnet#1111.

jsonnet accepted google/jsonnet#1140 to match go-jsonnet's behavior.

Note that sjsonnet already matches go-jsonnet's behavior for object comprehensions: we only have a difference in std.mergePatch because our current implementation explicitly hardcodes Unhide visibility.

This PR changes that mergePatch-specific behavior to match the current go-jsonnet (and future jsonnet) behavior.

@JoshRosen JoshRosen force-pushed the std-mergepatch-non-unhide branch from e54095c to 9b40266 Compare January 3, 2025 08:36
@stephenamar-db stephenamar-db merged commit 7d75fd7 into databricks:master Jan 3, 2025
6 checks passed
@JoshRosen JoshRosen deleted the std-mergepatch-non-unhide branch January 23, 2025 03:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants