Add OverloadedLabels support for positional lenses
#156
Conversation
amesgen
force-pushed
the
positional-labels
branch
from
f7ef335
to
5eddcb1
Compare
| @@ -271,6 +271,14 @@ tests = TestList $ map mkHUnitTest | |||
| , (valLabel ^? #RecB . _1 ) ~=? Just 3 | |||
| , (valLabel ^? #RecB ) ~=? Just (3, True) | |||
| , (valLabel ^? #RecC ) ~=? Nothing | |||
|
|
|||
| , (valLabel ^. #1 ) ~=? 3 | |||
| 'FieldType | ||
| If (StartsWithDigit name) | ||
| 'PositionType | ||
| ( If (CmpSymbol "_@" name == 'LT && CmpSymbol "_[" name == 'GT) |
There was a problem hiding this comment.
It would be nice if these predicates also had names? I guess the reason you pulled out StartsWithDigit is so you can define it conditionally, but it would still make things more consistent to pull out the others too.
There was a problem hiding this comment.
Sounds good, added a commit doing that 👍
| @@ -144,3 +163,35 @@ instance ( Applicative f, Choice p, Constructor name s t a b | |||
| instance ( Applicative f, Choice p, Constructor name s t a b | |||
| ) => IsLabelHelper 'ConstrType name p f s t a b where | |||
| labelOutput = constructorPrism @name | |||
|
|
|||
| #if MIN_VERSION_base(4,18,0) | |||
There was a problem hiding this comment.
Why do we actually need this? A comment would help! Looking at the issue, it seems like before 9.6 it is just the case that labels can't start with numbers. If that's all, then the code here is perfectly correct before 9.6, it just won't ever be able to trigger because no label of the correct form can be written in the surface syntax. Or am I missing something?
There was a problem hiding this comment.
Currently, the implementation uses eg UnconsSymbol which is only available since GHC 9.2 (see the "Char kind" GHC proposal), as the implementation before GHC 9.2 is very convoluted, see eg the symbols package.
As this feature is only interesting on GHCs >=9.6, I just made it conditional based on that. In particular, for some reason, the current definition of DigitToNat does not compile on GHC 9.2, only on >=9.4, but it didn't seem necessary to investigate that due to the fact that we only need GHC >=9.6 support.
Changed the condition to #if MIN_VERSION_base(4,17,0) and added a comment 👍
There was a problem hiding this comment.
Aha, that makes sense. We could make the pre-9.6 case a TypeError so people get told why it doesn't work, but I don't think that's a big deal.
There was a problem hiding this comment.
Even then, you could leave StartsWithDigit and friends, and just have ParseNat do nothing pre 9.6. Not a big deal, it just can be nice to minimize the amount in the CPP.
amesgen
force-pushed
the
positional-labels
branch
from
5eddcb1
to
3475ce6
Compare
|
Oh, and could you add a changelog entry, please? |
amesgen
force-pushed
the
positional-labels
branch
from
3475ce6
to
2baf7a3
Compare
Closes #151