Skip to content

Conversation

@byorgey
Copy link
Collaborator

@byorgey byorgey commented May 23, 2019

Down nodes now count as annotations that must be counted through when following a path. A lens onto a specific node sees only the subtree rooted there, without accumulated down annotations being applied first. This makes edits more "local" and (arguably) more intuitive.

Copy link
Owner

@cchalmers cchalmers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick pass. I haven't tested them out yet, will try to soon.

-- | Push all down annotations to just above the leaves, acting on @u@ and
-- @a@ annotations along the way. (Mainly for benchmarking/debugging).
pushDown :: (Action d m, Action d a, Monoid' d) => NE i d u m a l -> NE i d u m a l
pushDown = go where
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this function make sense anymore? Down annotations should now be static for traversals to work?

-- {-# INLINE leafs #-}
Label i EmptyDUAL -> pure (Label i EmptyDUAL)
Label i (NE t) -> Label i . NE <$> go t
Down ls d' t -> Down ls d' <$> go t
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want the lower case version here so it rebuilds the name map properly.

Suggested change
Down ls d' t -> Down ls d' <$> go t
Down _ d' t -> down d' <$> go t

Concat i ts -> Concat i <$> traverse go ts
{-# INLINE leafs #-}

releaf
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this function is that useful anymore but if we do want to keep it the comment should be clearer that the old downs are still there.

Suggested change
releaf
-- | Rebuild the tree by replacing each leaf given the old leaf and the up and down annotations applied to it. Note that the old down annotations remain higher up the tree.
releaf

n -> f d <&> \d' -> down d' (NE n)
Label i EmptyDUAL -> Label i EmptyDUAL
Label i (NE t) -> Label i . NE $ go d t
Down ls d' t -> Down ls d' (go (d <> d') t)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Down ls d' t -> Down ls d' (go (d <> d') t)
Down _ d' t -> down d' (go (d <> d') t)

-- {-# INLINE matchingU #-}
Label i EmptyDUAL -> pure (Label i EmptyDUAL)
Label i (NE t) -> Label i . NE <$> go d t
Down ls d' t -> Down ls d' <$> go (d <> d') t
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Down ls d' t -> Down ls d' <$> go (d <> d') t
Down _ d' t -> down d' <$> go (d <> d') t

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

Successfully merging this pull request may close these issues.

3 participants