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

support explicit specification of the optic target #55

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

aplavin
Copy link
Member

@aplavin aplavin commented Jun 11, 2022

The main common usecase is using @(re)set with an array element, as in @set $(x[i]).a.b = 123 and @reset $(x[i]).a.b = 123.
Currently, these require a temporary variable:

old = x[i]
new = @set old.a.b = 123

and

old = x[i]
x[i] = @set old.a.b = 123

test/test_core.jl Outdated Show resolved Hide resolved
@aplavin
Copy link
Member Author

aplavin commented Apr 4, 2023

@jw3126 thanks for raising those concerns with @reset and potentially surprising mutability. Indeed, avoiding mutations unless explicitly asked for is best.

Still, from time to time I wish there was this functionality for @set, would make stuff more convenient. So, in the new commit $... works in @set, but not @reset.

Also, the same commit fixes the rare but nasty @reset bug unrelated to $:

julia> @reset last(x, n) = [1, 2, 3]
last (generic function with 1 method)

- it redefined the function!

@aplavin
Copy link
Member Author

aplavin commented Jan 4, 2024

Bump @jw3126 :) Would be nice to have this feature, now that there's no mutation anywhere (as a sideeffect, this fixes the reset bug mentioned above).
Typical usecases look like this:

@set f($x, 5).a = 3
@insert f(5, $x).a[2] = 3
@set f($(xs[i]).a) = 3
...

Now, these require explicit set(@optic ) call or a temporary variable.

@jw3126
Copy link
Member

jw3126 commented Jan 5, 2024

Agreed, this is nice.

@aplavin aplavin merged commit 48bb220 into JuliaObjects:master Jan 5, 2024
8 checks passed
@aplavin aplavin deleted the patch-4 branch January 5, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants