-
-
Notifications
You must be signed in to change notification settings - Fork 55
Add prisms to IsLabel instance #69
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
Conversation
First of all, thanks for such a well-documented PR! I always felt that one downside (other than the unfortunate orphan instance problem) of these labels was the arbitrary preference of lenses over the other optics, but this PR seems to solve that wrinkle in a rather nice way. The label comparison trick is quite clever, I didn't think something like this was possible! The use of incoherent instances seems justified here, and it does seem sensible even outside the scope of overloaded labels. Seeing that |
By the way, shouldn't the comparison be between |
That is a great question and something that I forgot to document (I updated the PR description to discuss it). I'll describe the fundamental issue and my struggle with it, and since you're the library maintainer, I'll leave the final decision up to you. The question really boils down to: What are we defining to be the fundamental nature of a "prism" label as compared to a "lens" label? To highlight why this is a tough question, consider the following data type: data Foo = Foo
{ _Foo :: Int
, _bar :: Int
} Here, we have an obvious conflict because Then, I went about adding generic-lens support to my row-types library. Row-types supports open variants in addition to open records, so I added a It was at this point that I changed the definition of a "prism" label to be anything that comes after an At this point, you understand the struggle, so as I said at the start, I'll leave the question up to you: Is an |
Quite subtle indeed! Hm, I think I would prefer if I’m actually a little confused about how this works out, namely the following constraint: name' ~ AppendSymbol "_" name The instance head mentions only |
I do have some working examples, but I don't have enough knowledge of how type families (and data SumOfProducts =
RecA { foo :: Int, valA :: String }
| RecB { foo :: Int, valB :: Bool }
| RecC { foo :: Int }
deriving (Show, Eq, Generic)
testSumOfProducts =
(val ^. #foo ) == 3
&& (val & #foo +~ 10 ) == RecB 13 True
&& (val ^? #_RecB ) == Just (3, True)
&& (val ^? #_RecB . _1) == Just 3
&& (val ^? #_RecC ) == Nothing
where val = RecB 3 True Then, in GHCi, we can see that:
As for the label name issue, it's a little tough. Labels can only start with lowercase letters or an underscore, so there's no other symbol we can use. We could use unicode lowercase letters (p is for prism, so we could use ƥ or ꝕ, say) ... but this strikes me as not the best idea. Now that I'm considering this further, maybe an underscore followed by a uppercase letter is the right idea, and if labels are ever extended to allow uppercase letters, we could easily just drop the demand for a leading underscore, and everything will work great. The only real problem for me is in my row-types library, but perhaps I can use the same |
Having looked at the implementation in GHC, it seems like I'm leaning towards only allowing uppercase letters for prism labels, as it's much simpler that way, and as you said, the case conversion logic is probably better placed in the |
This PR extends the optional
IsLabel
instance to provide prisms for constructors in addition to lenses for fields. I debated whether to push this PR togeneric-lens-labels
or perhaps to just create a new repo (generic-optics-labels
?), but I felt that this was the best home for this code. Please let me know if you disagree.Field Access
Instead of writing
field @"foo"
to get a lens for the fieldfoo
for some data type, one merely needs to write#foo
. Furthermore, if the data type is part of a data instance, or otherwise has aHasField'
instance but noHasField
instance, the label#foo
will still work.Rather than using the
HasField
andHasField'
classes, one can use the combinedField
class, (which has the same structure asHasField
) and theField'
type synonym. (Note that I believe this addresses Issue #64)Constructor Access
Instead of writing
_Ctor @"Foo"
to get a prism for the variantFoo
of some data type, one merely needs to write#_Foo
. Note the need to include the underscore_
between the#
and the constructor name; this is because GHC does not currently support overloaded labels starting with capital letters. For example, the data typewill have prisms
#_OptionA
,#_OptionB
, and#_OptionC
.Likewise, rather than using the
AsConstructor
andAsConstructor'
classes, one can use the combinedConstructor
class, (which has the same structure asAsConstructor
) and theConstructor'
type synonym.Notes
Incoherent Instances
This PR uses incoherent instances to combine, e.g.,
HasField
andHasField'
into a single class so that labels can work for any field lens. This is not exactly elegant, but I didn't find a better solution.Orphan Instance
The
IsLabel
instance is now gigantic as it's covering alla -> b
. This can cause conflicts with other libraries, but I think it's worth it for the convenience you get here. As one of the maintainers of the extensible records package https://github.com/target/row-types, I already have overlappingHasField
andAsConstructor
instances for an upcominggeneric-lens-row-types
package, and I think it will be more convenient to use theData.Generics.Labels
here and have my library play nicely with it than to try to accommodate multiple differentIsLabel
instances.Label Comparison (and fields that start with underscores)
The
IsLabel
instance chooses between returning a prism vs returning a lens based on whether the first character in the symbol is an underscore. This is not very elegant, but it works, and it has some questionable repercussions. Notably, consider the following data type declaration:In this case, the label
#_Foo
would be for the prism based on theFoo
constructor, meaning that there is no generic lens for the_Foo
field. Furthermore, the label#_bar
would also be interpreted as a prism and could not be used as a lens to the_bar
field. Thus, this system does not mesh nicely with data types whose field names start with underscores.One could modify the approach in this PR to force labels to start with an underscore followed by a capital letter; this lets the label
#_bar
point to the_bar
field, but it does not help with the_Foo
field. I chose not to do this because 1) I like the simplicity of an underscore always indicating a prism, and 2) with overlapping instances, one can make usable lowercase names for prisms.Exposing This Module
Obviously, this work doesn't make a lot of sense without PR #68 also going through.