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

Document best practice setup for labelOpticDeriver #4

Open
fonghou opened this issue Feb 19, 2023 · 4 comments
Open

Document best practice setup for labelOpticDeriver #4

fonghou opened this issue Feb 19, 2023 · 4 comments

Comments

@fonghou
Copy link

fonghou commented Feb 19, 2023

Hi @nikita-volkov,

I found this part of README document is not sufficient for labelOpticDeriver.

declare (Just (False, True)) (stdDeriver <> labelOpticDeriver)

  1. When the second flag in declare (Just (False, False)) is set to False, hasFieldDeriver in stdDeriver produces "Illegal instance declaration for GHC.Records.HasField ... already has a field ...", which labelOpticDeriver also uses. Removing hasFieldDeriver fixes the compile error.
  2. stdDeriver includes constructorIsLabelDeriver, mapperIsLabelDeriver, accessorIsLabelDeriver, though they don't produce any compile error they seem reductant to optics review, over, set operators.
  3. Documentation only mentioned DuplicateRecordFields (typoed as DuplicateRecords somewhere). I found that, in order to use non-prefix record fields declare (Just (False, False)), it's best to enable these GHC extentions:
  •   DuplicateRecordFields (always)
    
  •   NoFieldSelectors (always GHC 9.2+), and optionally OverloadedRecordDot (caveat partial HasField for sum types)
    
  •   NamedFieldPuns, and/or RecordWildCards (ie. concise field names in pattern match but no record accessor functions)
    

Just want to share them first, and discuss where the best of places to put them (documentation or TemplateHaskell codegen).

Cheers!

@nikita-volkov
Copy link
Owner

Thanks! I agree, the docs need improvement. I think it would be best to cover such details in the Haddocks of the according derivers.

Not sure what you meant by placing the docs in the TemplateHaskell codegen.

@nikita-volkov
Copy link
Owner

It's worth mentioning that OverloadedRecordDot was not a thing when the docs here were written, so the compatibility with it was not mentioned. It'd be great to have the instructions on how to use "domain" with OverloadedRecordDot covered.

@fonghou
Copy link
Author

fonghou commented Feb 19, 2023

Thanks! I agree, the docs need improvement. I think it would be best to cover such details in the Haddocks of the according derivers.

Not sure what you meant by placing the docs in the TemplateHaskell codegen.

I meant define/export another top level stdDeriver without hasField/isLabelDeriver in DomainOptics module (name it stdOpticsDeriver?). Currently it must be exported from a separate module in project just to make TemplateHaskell stages happy. Yes, I agree Haddock is the best place, maybe put all of above under stdOpticsDeriver.

@fonghou
Copy link
Author

fonghou commented Feb 19, 2023

It's worth mentioning that OverloadedRecordDot was not a thing when the docs here were written, so the compatibility with it was not mentioned. It'd be great to have the instructions on how to use "domain" with OverloadedRecordDot covered.

Understood. I'm actually on the fence about it. It works with the setting I gave above. Pros is it looks nicer (no %^~# chars :-) if all you have are records. Cons is that it can't handle Affine fields for sum types (ie. runtime errors).

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

No branches or pull requests

2 participants