obsidiansystems / dependent-map

Dependently-typed finite maps (partial dependent products)
Other
63 stars 33 forks source link

Add alterF function. #22

Closed mankyKitty closed 6 years ago

mankyKitty commented 6 years ago

As per the Data.Map function of the same name, this is a targeted update of a value in the DMap with a function that returns some Functor. I have been working on some lens integration for DMap and this function proved useful and simple enough to add.

This should be O (log n) as per the alter function, but unsure if the Functor that is included could spice that up.

Should I bring across similar documentation to what exists on Data.Map for this function?

alterF docs

I just noticed that my editor automatically gobbled all the trailing whitespace in this commit, would you like me to put it all back? :sob:

ryantrinkle commented 6 years ago

@mankyKitty Awesome! Would you mind just separating the whitespace change into a separate commit? That will make it easier to review the substantive change.

mankyKitty commented 6 years ago

That's a bit better. :+1:

ryantrinkle commented 6 years ago

@mankyKitty Is there a commit that specifically shows just the new alterF functionality? It might be necessary to rebase this to squash the first two commits together. Note that it's fine to force-push to a PR branch.

mankyKitty commented 6 years ago

Gah, fixed that up, sorry I didn't realise I'd pushed it, I blame my muscle memory!

You're able to ignore any whitespace changes when viewing the diff on github, just in the "Diff Settings" in the top right when you're viewing the "Files Changed".

Nothing will need to be force pushed, there is a "squash and merge" option in the drop down menu for merging on the PR. :)

If you're not driving this with Github though, all of these points are moot. :p

ryantrinkle commented 6 years ago

Awesome, thanks!