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.

Sign up for GitHub

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

Jump to bottom

over' and iover' pretend to work with incompatible optics #473

Open
treeowl opened this issue Dec 27, 2022 · 0 comments · May be fixed by #475
Open

over' and iover' pretend to work with incompatible optics #473

treeowl opened this issue Dec 27, 2022 · 0 comments · May be fixed by #475

Comments

@treeowl
Copy link
Contributor

treeowl commented Dec 27, 2022 

ghci> length $ over' mapped id [undefined, undefined]
2

Oops. I'm currently working on adding over', iover', and relatives to lens. The choice I've made over there is not to add a Settable Solo instance, to avoid this very problem. I don't know how these optics work, but the uses of unwrapIdentity' in the Mapping (Star Identity') and Mapping (IxStar Identity') instances look really fishy. What goes wrong if those instances are removed, and over' and iover' are declared to need traversals rather than setters?

Additional surprising uses:

failover' and ifailover' both do things like

if visited
  then Just (unwrapIdentity' t)
  else Nothing

Should those be like this?

if | visited
   , Identity' () v <- t
   -> Just v
   | otherwise
   -> Nothing

or (using Solo)

if | visited
   , Solo v <- t
   = Just v
   | otherwise
   = Nothing

While you're in the ballpark, Identity' is quite an awkward type, and there's really no need for such complication.

Identity'  ~=  Solo -- Best Solo source is OneTuple, which handles all the compat mess
unwrapIdentity' ~= getSolo
wrapIdentity' ~= (Solo $!)
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
@treeowl
Restrict over', iover', and set' to traversals
9ef85c8 
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
@treeowl treeowl linked a pull request Dec 28, 2022 that will close this issue
Open
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
@treeowl
Restrict over', iover', and set' to traversals
dd788cb 
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
@treeowl
Restrict over', iover', and set' to traversals
7a7585d 
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
@treeowl
Restrict over', iover', and set' to traversals
4d41f41 
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
@treeowl
Restrict over', iover', and set' to traversals
40a38d2 
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Restrict over', iover', and set' to traversals treeowl/optics
1 participant
@treeowl