Reimplement `MapWithWithMove` in terms of `MapWithPatchingMove` #14

Ericson2314 · 2020-01-22T21:51:55Z

Per the change log, care is taken so this should not be a breaking change.

I had to to write explicit export lists to get around some obscure GHC bugs with reexport data types twice with different patterns nested for (..) imports, but I think it's nicer anyways as it makes comparability easier to check going forward.

Be very careful using newtypes and pattern synnonyms to break as little as possible.

…tch-desugar

Ericson2314 · 2020-02-06T03:12:26Z

Looks like a haddock or, less likely, GHC bug. Ugh!

…tch-desugar

- Do explicit export lists. This is a chore now, but makes managing Compatibility easier later. - Make `MapWithMove`'s `From` a proper newtype. This improves the compatibility and type error situation. - Work around GHC export list bug: the explicit export lists without big reexports this this for us, in fact.

This makes an instance, no new export needed.

- `mplus` -> `mappen` - better coverage

Ericson2314 · 2021-04-22T23:27:48Z

src/Data/Patch/MapWithMove.hs

-newtype PatchMapWithMove k v = PatchMapWithMove
-  { -- | Extract the internal representation of the 'PatchMapWithMove'
-    unPatchMapWithMove :: Map k (NodeInfo k v)
+newtype PatchMapWithMove k (v :: Type) = PatchMapWithMove'


PolyKinds was mischievously generalizing this over things for which PatchTarget (Proxy v) is undefined. Oh type families...

Ericson2314 · 2021-04-22T23:37:38Z

src/Data/Patch/MapWithPatchingMove.hs

+--
+-- From
+--
+
+-- | Describe how a key's new value should be produced
+data From k p
+   = From_Insert (PatchTarget p) -- ^ Insert the given value here
+   | From_Delete -- ^ Delete the existing value, if any, from here
+   | From_Move !k !p -- ^ Move the value here from the given key, and apply the given patch
+
+deriving instance (Show k, Show p, Show (PatchTarget p)) => Show (From k p)
+deriving instance (Read k, Read p, Read (PatchTarget p)) => Read (From k p)
+deriving instance (Eq k, Eq p, Eq (PatchTarget p)) => Eq (From k p)
+deriving instance (Ord k, Ord p, Ord (PatchTarget p)) => Ord (From k p)
+
+-- | Traverse the 'From' over the key, patch, and patch target. Because of
+-- the type families here, this doesn't it any bi- or tri-traversal class.
+bitraverseFrom
+  :: Applicative f
+  => (k0 -> f k1)
+  -> (p0 -> f p1)
+  -> (PatchTarget p0 -> f (PatchTarget p1))
+  -> From k0 p0 -> f (From k1 p1)
+bitraverseFrom fk fp fpt = \case
+  From_Insert pt -> From_Insert <$> fpt pt
+  From_Delete -> pure From_Delete
+  From_Move k p -> From_Move <$> fk k <*> fp p
+
+--
+-- To
+--
+
+-- | Describe where a key's old value will go.  If this is 'Just', that means
+-- the key's old value will be moved to the given other key; if it is 'Nothing',
+-- that means it will be deleted.
+type To = Maybe
+
+--
+-- Fixup
+--
+


This is just rearranging

Ericson2314 · 2021-04-22T23:38:05Z

src/Data/Patch/MapWithPatchingMove.hs

+--
+-- NodeInfo
+--
+
+-- | Holds the information about each key: where its new value should come from,
+-- and where its old value should go to
+data NodeInfo k p = NodeInfo
+  { _nodeInfo_from :: !(From k p)
+    -- ^ Where do we get the new value for this key?
+  , _nodeInfo_to :: !(To k)
+    -- ^ If the old value is being kept (i.e. moved rather than deleted or
+    -- replaced), where is it going?
+  }
+deriving instance (Show k, Show p, Show (PatchTarget p)) => Show (NodeInfo k p)
+deriving instance (Read k, Read p, Read (PatchTarget p)) => Read (NodeInfo k p)
+deriving instance (Eq k, Eq p, Eq (PatchTarget p)) => Eq (NodeInfo k p)
+deriving instance (Ord k, Ord p, Ord (PatchTarget p)) => Ord (NodeInfo k p)
+
+-- | Traverse the 'NodeInfo' over the key, patch, and patch target. Because of
+-- the type families here, this doesn't it any bi- or tri-traversal class.
+bitraverseNodeInfo
+  :: Applicative f
+  => (k0 -> f k1)
+  -> (p0 -> f p1)
+  -> (PatchTarget p0 -> f (PatchTarget p1))
+  -> NodeInfo k0 p0 -> f (NodeInfo k1 p1)
+bitraverseNodeInfo fk fp fpt (NodeInfo from to) = NodeInfo
+  <$> bitraverseFrom fk fp fpt from
+  <*> traverse fk to
+


This is just rearranging

Ericson2314 · 2021-04-22T23:38:14Z

src/Data/Patch/MapWithPatchingMove.hs

-- | Holds the information about each key: where its new value should come from,
-- and where its old value should go to
-data NodeInfo k p = NodeInfo
-  { _nodeInfo_from :: !(From k p)
-    -- ^ Where do we get the new value for this key?
-  , _nodeInfo_to :: !(To k)
-    -- ^ If the old value is being kept (i.e. moved rather than deleted or
-    -- replaced), where is it going?
-  }
-deriving instance (Show k, Show p, Show (PatchTarget p)) => Show (NodeInfo k p)
-deriving instance (Read k, Read p, Read (PatchTarget p)) => Read (NodeInfo k p)
-deriving instance (Eq k, Eq p, Eq (PatchTarget p)) => Eq (NodeInfo k p)
-deriving instance (Ord k, Ord p, Ord (PatchTarget p)) => Ord (NodeInfo k p)
-
-bitraverseNodeInfo
-  :: Applicative f
-  => (k0 -> f k1)
-  -> (p0 -> f p1)
-  -> (PatchTarget p0 -> f (PatchTarget p1))
-  -> NodeInfo k0 p0 -> f (NodeInfo k1 p1)
-bitraverseNodeInfo fk fp fpt (NodeInfo from to) = NodeInfo
-  <$> bitraverseFrom fk fp fpt from
-  <*> traverse fk to
-
-- | Describe how a key's new value should be produced
-data From k p
-   = From_Insert (PatchTarget p) -- ^ Insert the given value here
-   | From_Delete -- ^ Delete the existing value, if any, from here
-   | From_Move !k !p -- ^ Move the value here from the given key, and apply the given patch
-
-bitraverseFrom
-  :: Applicative f
-  => (k0 -> f k1)
-  -> (p0 -> f p1)
-  -> (PatchTarget p0 -> f (PatchTarget p1))
-  -> From k0 p0 -> f (From k1 p1)
-bitraverseFrom fk fp fpt = \case
-  From_Insert pt -> From_Insert <$> fpt pt
-  From_Delete -> pure From_Delete
-  From_Move k p -> From_Move <$> fk k <*> fp p
-
-deriving instance (Show k, Show p, Show (PatchTarget p)) => Show (From k p)
-deriving instance (Read k, Read p, Read (PatchTarget p)) => Read (From k p)
-deriving instance (Eq k, Eq p, Eq (PatchTarget p)) => Eq (From k p)
-deriving instance (Ord k, Ord p, Ord (PatchTarget p)) => Ord (From k p)
-
-- | Describe where a key's old value will go.  If this is 'Just', that means
-- the key's old value will be moved to the given other key; if it is 'Nothing',
-- that means it will be deleted.
-type To = Maybe
-


This is just rearranging, see below

Ericson2314 · 2021-04-22T23:39:47Z

.github/workflows/haskell.yml

+    - name: Build Docs
+      run: cabal haddock


Travis was doing this, so this is a good idea if we want to be able to drop travis.

…tch-desugar

ryantrinkle

Looks great!

Ericson2314 added 6 commits January 22, 2020 16:50

Reimplement MapWithWithMove in terms of MapWithPatchingMove

8cd0fc5

Be very careful using newtypes and pattern synnonyms to break as little as possible.

Merge branch 'develop' into patch-map-inner-patch-desugar

cfcd1cf

Make hlint stop complaining

4fb60c4

Merge branch 'no-fromJust' into patch-map-inner-patch-desugar

acd5b6a

Switch to using more newtypes for better back-compat

9828db7

Merge remote-tracking branch 'origin/develop' into patch-map-inner-pa…

444969d

…tch-desugar

Merge branch 'develop' into patch-map-inner-patch-desugar

9705224

Ericson2314 mentioned this pull request Feb 12, 2020

WIP: Combination of #1 and #2 #10

Open

Ericson2314 added 11 commits December 7, 2020 12:01

Merge remote-tracking branch 'origin/develop' into patch-map-inner-pa…

09fe276

…tch-desugar

Merge remote-tracking branch 'origin/develop' into patch-map-inner-pa…

220acfc

…tch-desugar

Appease hlint

e28525a

Appease hlint some more

05ee4d4

Add another makeWrapped for From

579ca70

This makes an instance, no new export needed.

Dedup traverse instance

29815c0

Fix haddock

4b8916d

Improve haddock

0ebc19f

- `mplus` -> `mappen` - better coverage

github workflows should build haddock too

73f2ce4

Fix haddock some more

32a1f2f

Ericson2314 force-pushed the patch-map-inner-patch-desugar branch from 20531ce to 32a1f2f Compare April 22, 2021 23:15

Ericson2314 commented Apr 22, 2021

View reviewed changes

Ericson2314 added 2 commits May 22, 2021 13:28

Merge remote-tracking branch 'origin/develop' into patch-map-inner-pa…

2d2c32f

…tch-desugar

Simplify Coercions

290829d

ryantrinkle approved these changes May 22, 2021

View reviewed changes

Ericson2314 merged commit 0019cce into develop May 22, 2021

Ericson2314 deleted the patch-map-inner-patch-desugar branch May 22, 2021 23:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement `MapWithWithMove` in terms of `MapWithPatchingMove` #14

Reimplement `MapWithWithMove` in terms of `MapWithPatchingMove` #14

Ericson2314 commented Jan 22, 2020 •

edited

Loading

Ericson2314 commented Feb 6, 2020

Ericson2314 Apr 22, 2021

Ericson2314 Apr 22, 2021 •

edited

Loading

Ericson2314 Apr 22, 2021

Ericson2314 Apr 22, 2021

Ericson2314 Apr 22, 2021

ryantrinkle left a comment

Reimplement MapWithWithMove in terms of MapWithPatchingMove #14

Reimplement MapWithWithMove in terms of MapWithPatchingMove #14

Conversation

Ericson2314 commented Jan 22, 2020 • edited Loading

Ericson2314 commented Feb 6, 2020

Ericson2314 Apr 22, 2021

Choose a reason for hiding this comment

Ericson2314 Apr 22, 2021 • edited Loading

Choose a reason for hiding this comment

Ericson2314 Apr 22, 2021

Choose a reason for hiding this comment

Ericson2314 Apr 22, 2021

Choose a reason for hiding this comment

Ericson2314 Apr 22, 2021

Choose a reason for hiding this comment

ryantrinkle left a comment

Choose a reason for hiding this comment

Reimplement `MapWithWithMove` in terms of `MapWithPatchingMove` #14

Reimplement `MapWithWithMove` in terms of `MapWithPatchingMove` #14

Ericson2314 commented Jan 22, 2020 •

edited

Loading

Ericson2314 Apr 22, 2021 •

edited

Loading